Skip to content

Commit

Permalink
apacheGH-40878: [JAVA] Fix flight-sql-jdbc-driver shading issues (apa…
Browse files Browse the repository at this point in the history
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
  • Loading branch information
laurentgo committed Mar 30, 2024
1 parent ce11e56 commit 9f0101e
Show file tree
Hide file tree
Showing 2 changed files with 184 additions and 8 deletions.
51 changes: 43 additions & 8 deletions java/flight/flight-sql-jdbc-driver/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,16 @@
<build>
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<enableAssertions>false</enableAssertions>
<systemPropertyVariables>
<arrow.test.dataRoot>${project.basedir}/../../../testing/data</arrow.test.dataRoot>
</systemPropertyVariables>
</configuration>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
Expand All @@ -167,12 +170,22 @@
</goals>
<configuration>
<shadedArtifactAttached>false</shadedArtifactAttached>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
<minimizeJar>false</minimizeJar>
<artifactSet>
<includes>
<include>*:*</include>
</includes>
<excludes>
<!-- Source annotations -->
<exclude>org.checkerframework:checker-qual</exclude>
<exclude>org.codehaus.mojo:animal-sniffer-annotations</exclude>
<exclude>javax.annotation:javax.annotation-api</exclude>
<exclude>com.google.android:annotations</exclude>
<exclude>com.google.errorprone:error_prone_annotations</exclude>
<exclude>com.google.code.findbugs:jsr305</exclude>
<exclude>com.google.j2objc:j2objc-annotations</exclude>
</excludes>
</artifactSet>
<relocations>
<relocation>
Expand All @@ -199,6 +212,14 @@
<pattern>io.</pattern>
<shadedPattern>cfjd.io.</shadedPattern>
</relocation>
<relocation>
<pattern>net.</pattern>
<shadedPattern>cfjd.net.</shadedPattern>
</relocation>
<relocation>
<pattern>mozilla.</pattern>
<shadedPattern>cfjd.mozilla.</shadedPattern>
</relocation>
<!-- Entries to relocate netty native libraries -->
<relocation>
<pattern>META-INF.native.libnetty_</pattern>
Expand All @@ -213,12 +234,25 @@
<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer"/>
</transformers>
<filters>
<filter>
<artifact>org.apache.arrow:arrow-vector</artifact>
<excludes>
<exclude>codegen/**</exclude>
</excludes>
</filter>
<filter>
<artifact>org.apache.calcite.avatica:*</artifact>
<excludes>
<exclude>META-INF/services/java.sql.Driver</exclude>
</excludes>
</filter>
<filter>
<artifact>org.eclipse.collections:*</artifact>
<excludes>
<exclude>about.html</exclude>
<exclude>LICENSE-*-1.0.txt</exclude>
</excludes>
</filter>
<filter>
<artifact>*:*</artifact>
<excludes>
Expand All @@ -227,6 +261,7 @@
<exclude>**/*.DSA</exclude>
<exclude>META-INF/native/libio_grpc_netty*</exclude>
<exclude>META-INF/native/io_grpc_netty_shaded*</exclude>
<exclude>**/*.proto</exclude>
</excludes>
</filter>
</filters>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.arrow.driver.jdbc;

import static org.junit.Assert.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.io.File;
import java.io.IOException;
import java.net.JarURLConnection;
import java.net.URL;
import java.util.Enumeration;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;

import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ErrorCollector;
import org.junit.rules.TestRule;
import org.junit.rules.Timeout;

import com.google.common.collect.ImmutableSet;

/**
* Check the content of the JDBC driver jar
*
* After shading everything should be either under org.apache.arrow.driver.jdbc.,
* org.slf4j., or cfjd. packages
*/
public class ITDriverJarValidation {
/**
* Use this property to provide path to the JDBC driver jar. Can be used to run the test from an IDE
*/
public static final String JDBC_DRIVER_PATH_OVERRIDE =
System.getProperty("arrow-flight-jdbc-driver.jar.override");

/**
* List of allowed prefixes a jar entry may match.
*/
public static final Set<String> ALLOWED_PREFIXES = ImmutableSet.of(
"org/apache/arrow/driver/jdbc/",
"cfjd/",
"org/slf4j/",
"META-INF/");

/**
* List of allowed files a jar entry may match.
*/
public static final Set<String> ALLOWED_FILES = ImmutableSet.of(
"arrow-git.properties",
"properties/flight.properties");

// This method is designed to work with Maven failsafe plugin and expects the
// JDBC driver jar to be present in the test classpath (instead of the individual classes)
private static JarFile getJdbcJarFile() throws IOException {
// Check if an override has been set
if (JDBC_DRIVER_PATH_OVERRIDE != null) {
return new JarFile(new File(JDBC_DRIVER_PATH_OVERRIDE));
}

// Check classpath to find the driver jar
URL driverClassURL = ITDriverJarValidation.class.getClassLoader()
.getResource("org/apache/arrow/driver/jdbc/ArrowFlightJdbcDriver.class");

assertNotNull(driverClassURL, "Driver jar was not detected in the classpath");
assertEquals("Driver jar was not detected in the classpath", "jar", driverClassURL.getProtocol());

JarURLConnection connection = (JarURLConnection) driverClassURL.openConnection();
return connection.getJarFile();
}

@ClassRule
public static final TestRule CLASS_TIMEOUT = Timeout.builder().withTimeout(2, TimeUnit.MINUTES).build();

@Rule
public ErrorCollector collector = new ErrorCollector();

@Test
public void validateShadedJar() throws IOException {
// Validate the content of the jar to enforce all 3rd party dependencies have
// been shaded
try (JarFile jar = getJdbcJarFile()) {
for (Enumeration<JarEntry> entries = jar.entries(); entries.hasMoreElements();) {
final JarEntry entry = entries.nextElement();
if (entry.isDirectory()) {
// Directories are ignored
continue;
}

try {
checkEntryAllowed(entry.getName());
} catch (AssertionError e) {
collector.addError(e);
}
}
}
}

/**
* Check if a jar entry is allowed.
*
* <p>
* A jar entry is allowed if either it is part of the allowed files or it
* matches one of the allowed prefixes
*
* @param name the jar entry name
* @throws AssertionException if the entry is not allowed
*/
private void checkEntryAllowed(String name) {
// Check if there's a matching file entry first
if (ALLOWED_FILES.contains(name)) {
return;
}

for (String prefix : ALLOWED_PREFIXES) {
if (name.startsWith(prefix)) {
return;
}
}

throw new AssertionError("'" + name + "' is not an allowed jar entry");
}
}

0 comments on commit 9f0101e

Please sign in to comment.