This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 9f0101ec14 GH-40878: [JAVA] Fix flight-sql-jdbc-driver shading issues
(#40879)
9f0101ec14 is described below
commit 9f0101ec14336b2baad45d57320fb56c71d9321b
Author: Laurent Goujon <[email protected]>
AuthorDate: Fri Mar 29 18:29:21 2024 -0700
GH-40878: [JAVA] Fix flight-sql-jdbc-driver shading issues (#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: #40878
Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
---
java/flight/flight-sql-jdbc-driver/pom.xml | 51 ++++++--
.../arrow/driver/jdbc/ITDriverJarValidation.java | 141 +++++++++++++++++++++
2 files changed, 184 insertions(+), 8 deletions(-)
diff --git a/java/flight/flight-sql-jdbc-driver/pom.xml
b/java/flight/flight-sql-jdbc-driver/pom.xml
index 84ec1ff8c1..53d929afa7 100644
--- a/java/flight/flight-sql-jdbc-driver/pom.xml
+++ b/java/flight/flight-sql-jdbc-driver/pom.xml
@@ -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>
@@ -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>
@@ -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>
@@ -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>
@@ -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>
diff --git
a/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ITDriverJarValidation.java
b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ITDriverJarValidation.java
new file mode 100644
index 0000000000..fdb580d493
--- /dev/null
+++
b/java/flight/flight-sql-jdbc-driver/src/test/java/org/apache/arrow/driver/jdbc/ITDriverJarValidation.java
@@ -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");
+ }
+}
