This is an automated email from the ASF dual-hosted git repository.
voonhous pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new 7b4ad9d6c962 fix(common): scan jar entries in
ReflectionUtils#getTopLevelClassesInClasspath (#19624)
7b4ad9d6c962 is described below
commit 7b4ad9d6c96284399bf9ed6d1ebdf4dccb9cc7e6
Author: deepakpanda93 <[email protected]>
AuthorDate: Wed Sep 9 21:25:11 2026 +0530
fix(common): scan jar entries in
ReflectionUtils#getTopLevelClassesInClasspath (#19624)
Follow-up to #19784 (HUDI-736), which simplified the method but did
not add jar-entry scanning.
A jar: classpath entry is non-hierarchical, so toDirectory logs the
IllegalArgumentException and drops it, and a scan run from inside a
shaded jar returns nothing. Every caller is a packaging bundle Main
class, which runs exactly that way; before #19784 the same case threw.
Class#getPackage is also null for arrays and primitives, which was
dereferenced without a check.
classNamesIn now dispatches on protocol. A jar: entry is read through
JarURLConnection with setUseCaches(false), so the scan opens its own
JarFile instead of closing one shared through the JVM-wide cache, and
the names are collected before the jar is closed because the returned
stream outlives the method. The entry prefix stays derived from the
package name: anchoring on getEntryName() would make the result JDK
dependent on a multi-release jar (the loader resolves the package to
META-INF/versions/N/<pkg>/ on JDK 9-23 but to <pkg>/ on 8 and 24+) and
would drop classes that exist only in the base directory. A connection
that is not a JarURLConnection is skipped, a class with no package
yields an empty stream, and findClasses skips a directory whose
listFiles() is null instead of throwing.
Only hudi-cli-bundle ships slf4j, so the other bundle Main classes
still fail in ReflectionUtils.<clinit> when run from the bare jar; the
scan works wherever slf4j is on the classpath.
TestReflectionUtils is 13 tests. Each guard is pinned by a test that
fails without it: the jar dispatch by an empty result, the null-package
guard and the listFiles guard by NullPointerException, the instanceof
check by ClassCastException, and setUseCaches(false) by
"IllegalStateException: zip file closed" on a JarFile another reader
holds open through the cache.
---
.../hudi/common/util/TestReflectionUtils.java | 236 +++++++++++++++++++--
.../apache/hudi/common/util/ReflectionUtils.java | 92 +++++++-
2 files changed, 307 insertions(+), 21 deletions(-)
diff --git
a/hudi-common/src/test/java/org/apache/hudi/common/util/TestReflectionUtils.java
b/hudi-common/src/test/java/org/apache/hudi/common/util/TestReflectionUtils.java
index 7640435ae4c5..be43ee3b2fa3 100644
---
a/hudi-common/src/test/java/org/apache/hudi/common/util/TestReflectionUtils.java
+++
b/hudi-common/src/test/java/org/apache/hudi/common/util/TestReflectionUtils.java
@@ -27,17 +27,34 @@ import org.apache.hudi.storage.StoragePath;
import org.apache.hudi.storage.StoragePathFilter;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
+import java.io.File;
import java.io.IOException;
+import java.net.JarURLConnection;
import java.net.URL;
+import java.net.URLClassLoader;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
+import java.util.List;
+import java.util.function.Supplier;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.jar.JarOutputStream;
+import java.util.stream.Collectors;
import static org.apache.hudi.common.util.ReflectionUtils.getMethod;
+import static
org.apache.hudi.common.util.ReflectionUtils.getTopLevelClassesInClasspath;
import static org.apache.hudi.common.util.ReflectionUtils.isSubClass;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -67,36 +84,229 @@ public class TestReflectionUtils {
}
@ParameterizedTest
- @ValueSource(strings = {"jar:file:/unused.jar!/org/apache/hudi/common/util",
"file:/invalid path"})
+ @ValueSource(strings = {"jar:file:/unused.jar!/org/apache/hudi/common/util",
"file:/invalid path",
+ "http://example.invalid/org/apache/hudi/common/util"})
void testGetTopLevelClassesInClasspathSkipsInvalidResources(String
invalidResource) {
- ClassLoader original = Thread.currentThread().getContextClassLoader();
- ClassLoader loader = new ClassLoader(original) {
+ ClassLoader loader = new
ClassLoader(Thread.currentThread().getContextClassLoader()) {
@Override
public Enumeration<URL> getResources(String name) throws IOException {
return Collections.enumeration(Arrays.asList(new URL(invalidResource),
TestReflectionUtils.class.getResource("")));
}
};
- try {
- Thread.currentThread().setContextClassLoader(loader);
-
assertTrue(ReflectionUtils.getTopLevelClassesInClasspath(TestReflectionUtils.class)
- .anyMatch(TestReflectionUtils.class.getName()::equals));
- } finally {
- Thread.currentThread().setContextClassLoader(original);
- }
+ assertTrue(withContextClassLoader(loader, () ->
+
ReflectionUtils.getTopLevelClassesInClasspath(TestReflectionUtils.class)
+ .anyMatch(TestReflectionUtils.class.getName()::equals)));
}
@Test
void testGetTopLevelClassesInClasspathHandlesIOException() {
- ClassLoader original = Thread.currentThread().getContextClassLoader();
- ClassLoader loader = new ClassLoader(original) {
+ ClassLoader loader = new
ClassLoader(Thread.currentThread().getContextClassLoader()) {
@Override
public Enumeration<URL> getResources(String name) throws IOException {
throw new IOException("Simulated failure enumerating resources");
}
};
+ assertFalse(withContextClassLoader(loader, () ->
+
ReflectionUtils.getTopLevelClassesInClasspath(TestReflectionUtils.class).findAny().isPresent()));
+ }
+
+ /**
+ * An exploded directory on the classpath, reached over the "file" protocol.
Built explicitly
+ * rather than scanning the test classpath, so the expected set is exact and
independent of
+ * whatever else surefire puts on the classpath.
+ */
+ @Test
+ void testGetTopLevelClassesInClasspathFromDirectory(@TempDir Path tempDir)
throws Exception {
+ String scanned = ReflectionUtils.class.getPackage().getName();
+ Path root = tempDir.resolve("classes");
+ Path pkgDir = root.resolve(scanned.replace('.', File.separatorChar));
+ Files.createDirectories(pkgDir.resolve("nested"));
+ Files.write(pkgDir.resolve("Alpha.class"), new byte[] {1});
+ Files.write(pkgDir.resolve("Beta.class"), new byte[] {1});
+ Files.write(pkgDir.resolve("nested").resolve("Gamma.class"), new byte[]
{1});
+ Files.write(pkgDir.resolve("notaclass.txt"), new byte[] {1});
+
+ List<String> classes = withContextClassLoaderOver(() ->
+
getTopLevelClassesInClasspath(ReflectionUtils.class).collect(Collectors.toList()),
root);
+
+ assertEquals(
+ Arrays.asList(scanned + ".Alpha", scanned + ".Beta", scanned +
".nested.Gamma"),
+ classes.stream().sorted().collect(Collectors.toList()),
+ "a directory entry must yield the classes of the package and its
subpackages, and nothing else");
+ }
+
+ /**
+ * Classes packaged in a jar are reached over the "jar" protocol, whose URLs
are not hierarchical
+ * and so cannot be turned into a {@link File}. Every caller of this method
is a bundle
+ * Main class, which is exactly the packaged case.
+ * <p>
+ * The jar is built under the scanned class's own package, and the loader is
given no parent, so
+ * the only resource found for that package is the one written here.
+ */
+ @Test
+ void testGetTopLevelClassesInClasspathFromJar(@TempDir Path tempDir) throws
Exception {
+ String scanned = ReflectionUtils.class.getPackage().getName();
+ String dir = scanned.replace('.', '/') + "/";
+ Path jar = tempDir.resolve("classes.jar");
+ writeJar(jar,
+ dir,
+ dir + "Alpha.class",
+ dir + "Beta.class",
+ dir + "nested/",
+ dir + "nested/Gamma.class",
+ dir + "notaclass.txt",
+ "org/example/other/Delta.class");
+
+ List<String> classes = withContextClassLoaderOver(() ->
+
getTopLevelClassesInClasspath(ReflectionUtils.class).collect(Collectors.toList()),
jar);
+
+ assertEquals(
+ Arrays.asList(scanned + ".Alpha", scanned + ".Beta", scanned +
".nested.Gamma"),
+ classes.stream().sorted().collect(Collectors.toList()),
+ "a jar entry must yield the classes of the package and its
subpackages, and nothing else");
+ }
+
+ @Test
+ void testGetTopLevelClassesInClasspathForClassesWithoutAPackage() {
+ // Arrays and primitives have no package, which used to dereference null.
+ assertEquals(0, getTopLevelClassesInClasspath(String[].class).count());
+ assertEquals(0, getTopLevelClassesInClasspath(int.class).count());
+ }
+
+ /**
+ * A jar: URL whose stream handler returns a plain URLConnection rather than
a JarURLConnection.
+ * Without the type check the cast throws ClassCastException, which the
IOException catch does not
+ * cover, so a single such entry would fail the whole scan.
+ */
+ @Test
+ void testGetTopLevelClassesInClasspathSkipsAJarUrlThatIsNotAJarConnection()
throws Exception {
+ URLStreamHandler plainHandler = new URLStreamHandler() {
+ @Override
+ protected URLConnection openConnection(URL url) {
+ return new URLConnection(url) {
+ @Override
+ public void connect() {
+ // Never connected: getTopLevelClassesInClasspath only inspects
the connection's type.
+ }
+ };
+ }
+ };
+ URL notAJarConnection =
+ new URL(null, "jar:file:/unused.jar!/org/apache/hudi/common/util",
plainHandler);
+ ClassLoader loader = new
ClassLoader(Thread.currentThread().getContextClassLoader()) {
+ @Override
+ public Enumeration<URL> getResources(String name) {
+ return Collections.enumeration(Arrays.asList(notAJarConnection,
TestReflectionUtils.class.getResource("")));
+ }
+ };
+
+ assertTrue(withContextClassLoader(loader, () ->
+
ReflectionUtils.getTopLevelClassesInClasspath(TestReflectionUtils.class)
+ .anyMatch(TestReflectionUtils.class.getName()::equals)),
+ "an entry whose connection is not a JarURLConnection must be skipped,
not fail the scan");
+ }
+
+ /**
+ * The scan must leave a jar it did not open alone. Without
setUseCaches(false) it closes the
+ * JVM-wide cached JarFile, and the next read by whoever opened it first
fails with
+ * "IllegalStateException: zip file closed".
+ */
+ @Test
+ void
testGetTopLevelClassesInClasspathFromJarLeavesASharedJarFileUsable(@TempDir
Path tempDir) throws Exception {
+ String scanned = ReflectionUtils.class.getPackage().getName();
+ String dir = scanned.replace('.', '/') + "/";
+ Path jar = tempDir.resolve("shared.jar");
+ writeJar(jar, dir, dir + "Alpha.class");
+
+ // Open it through the shared cache first, the way another reader on the
same JVM would.
+ URL entry = new URL("jar:" + jar.toUri().toURL() + "!/" + dir);
+ JarURLConnection shared = (JarURLConnection) entry.openConnection();
+ shared.setUseCaches(true);
+ JarFile sharedJar = shared.getJarFile();
+ try {
+ withContextClassLoaderOver(() ->
getTopLevelClassesInClasspath(ReflectionUtils.class).count(), jar);
+ assertDoesNotThrow(() -> sharedJar.getEntry(dir + "Alpha.class"),
+ "the scan must not close a JarFile held open through the JVM-wide
cache");
+ } finally {
+ sharedJar.close();
+ }
+ }
+
+ /**
+ * A package path that is a regular file rather than a directory.
File#listFiles returns null
+ * there, which used to escape as a NullPointerException.
+ */
+ @Test
+ void testGetTopLevelClassesInClasspathWhenThePackagePathIsAFile(@TempDir
Path tempDir) throws Exception {
+ String scanned = ReflectionUtils.class.getPackage().getName();
+ Path root = tempDir.resolve("classes");
+ Path pkgPath = root.resolve(scanned.replace('.', File.separatorChar));
+ Files.createDirectories(pkgPath.getParent());
+ Files.write(pkgPath, new byte[] {1});
+
+ long count = withContextClassLoaderOver(() ->
getTopLevelClassesInClasspath(ReflectionUtils.class).count(), root);
+ assertEquals(0, count, "an unreadable package entry must be skipped, not
throw");
+ }
+
+ /**
+ * The package resolving to more than one classpath entry, which is the
shape surefire produces.
+ * Every entry has to contribute, whichever protocol it uses.
+ */
+ @Test
+ void testGetTopLevelClassesInClasspathUnionsEveryClasspathEntry(@TempDir
Path tempDir) throws Exception {
+ String scanned = ReflectionUtils.class.getPackage().getName();
+ String dir = scanned.replace('.', '/') + "/";
+ Path jar = tempDir.resolve("classes.jar");
+ writeJar(jar, dir, dir + "FromJar.class");
+ Path root = tempDir.resolve("classes");
+ Path pkgDir = root.resolve(scanned.replace('.', File.separatorChar));
+ Files.createDirectories(pkgDir);
+ Files.write(pkgDir.resolve("FromDirectory.class"), new byte[] {1});
+
+ List<String> classes = withContextClassLoaderOver(() ->
+
getTopLevelClassesInClasspath(ReflectionUtils.class).collect(Collectors.toList()),
jar, root);
+
+ assertEquals(
+ Arrays.asList(scanned + ".FromDirectory", scanned + ".FromJar"),
+ classes.stream().sorted().collect(Collectors.toList()),
+ "a jar entry and a directory entry for the same package must both
contribute");
+ }
+
+ /** Writes a jar holding the given entry names; names ending in "/" become
directory entries. */
+ private static void writeJar(Path jar, String... entryNames) throws
IOException {
+ try (JarOutputStream out = new
JarOutputStream(Files.newOutputStream(jar))) {
+ for (String entryName : entryNames) {
+ out.putNextEntry(new JarEntry(entryName));
+ if (!entryName.endsWith("/")) {
+ // Content is irrelevant: the scan reads entry names, never the
bytecode.
+ out.write(new byte[] {1, 2, 3});
+ }
+ out.closeEntry();
+ }
+ }
+ }
+
+ /**
+ * Runs the supplier with the thread context class loader reading only from
the given classpath
+ * roots, each of which may be a jar or an exploded directory. The loader is
given no parent so
+ * the scan sees nothing else.
+ */
+ private static <T> T withContextClassLoaderOver(Supplier<T> supplier,
Path... roots) throws IOException {
+ URL[] urls = new URL[roots.length];
+ for (int i = 0; i < roots.length; i++) {
+ urls[i] = roots[i].toUri().toURL();
+ }
+ try (URLClassLoader loader = new URLClassLoader(urls, null)) {
+ return withContextClassLoader(loader, supplier);
+ }
+ }
+
+ /** Runs the supplier with the given thread context class loader, restoring
the previous one after. */
+ private static <T> T withContextClassLoader(ClassLoader loader, Supplier<T>
supplier) {
+ ClassLoader original = Thread.currentThread().getContextClassLoader();
try {
Thread.currentThread().setContextClassLoader(loader);
-
assertFalse(ReflectionUtils.getTopLevelClassesInClasspath(TestReflectionUtils.class).findAny().isPresent());
+ return supplier.get();
} finally {
Thread.currentThread().setContextClassLoader(original);
}
diff --git
a/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
b/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
index 819394207ba2..9b873c2420ea 100644
--- a/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
+++ b/hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java
@@ -26,15 +26,19 @@ import java.io.File;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
+import java.net.JarURLConnection;
import java.net.URISyntaxException;
import java.net.URL;
+import java.net.URLConnection;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
/**
@@ -44,6 +48,7 @@ import java.util.stream.Stream;
public class ReflectionUtils {
private static final Map<String, Class<?>> CLAZZ_CACHE = new
ConcurrentHashMap<>();
+ private static final String CLASS_FILE_SUFFIX = ".class";
public static Class<?> getClass(String clazzName) {
return CLAZZ_CACHE.computeIfAbsent(clazzName, c -> {
@@ -130,19 +135,83 @@ public class ReflectionUtils {
*/
public static Stream<String> getTopLevelClassesInClasspath(Class<?> clazz) {
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
- String packageName = clazz.getPackage().getName();
+ // Arrays and primitives have no package, and Class#getPackage is also
null when the class was
+ // loaded by a loader that defines no package for it.
+ Package pkg = clazz.getPackage();
+ if (pkg == null) {
+ return Stream.empty();
+ }
+ String packageName = pkg.getName();
String path = packageName.replace('.', '/');
try {
return Collections.list(classLoader.getResources(path)).stream()
- .map(ReflectionUtils::toDirectory)
- .filter(Objects::nonNull)
- .flatMap(directory -> findClasses(directory, packageName).stream());
+ .flatMap(resource -> classNamesIn(resource, packageName));
} catch (IOException e) {
log.error("Unable to fetch Resources in package {}", packageName, e);
return Stream.empty();
}
}
+ /**
+ * Class names under a single classpath entry for the package, whether that
entry is an exploded
+ * directory or a jar.
+ *
+ * <p>A jar entry cannot go through {@link #toDirectory}: a {@code jar:} URL
is non-hierarchical,
+ * so {@code new File(uri)} throws and the entry would be dropped. Every
bundle {@code Main} class
+ * runs from inside a shaded jar, so that path has to be read through the
jar connection instead.
+ *
+ * @param resource a classpath entry holding the package
+ * @param packageName the package being scanned
+ * @return class names found under that entry, empty if it cannot be read
+ */
+ private static Stream<String> classNamesIn(URL resource, String packageName)
{
+ if ("jar".equals(resource.getProtocol())) {
+ return classNamesInJar(resource, packageName);
+ }
+ File directory = toDirectory(resource);
+ return directory == null ? Stream.empty() : findClasses(directory,
packageName).stream();
+ }
+
+ /**
+ * Class names under the package inside a jar, read through the jar
connection.
+ *
+ * @param resource a {@code jar:} classpath entry holding the package
+ * @param packageName the package being scanned
+ * @return class names found in that jar, empty if the jar cannot be read
+ */
+ private static Stream<String> classNamesInJar(URL resource, String
packageName) {
+ try {
+ URLConnection connection = resource.openConnection();
+ if (!(connection instanceof JarURLConnection)) {
+ // A jar: URL served by a non-JDK stream handler. Skip it rather than
let the cast throw,
+ // since this method exists to stop such an entry from failing the
whole scan.
+ log.warn("Skipping classpath entry {}, {} is not a JarURLConnection",
resource, connection.getClass());
+ return Stream.empty();
+ }
+ JarURLConnection jarConnection = (JarURLConnection) connection;
+ // Without this the JarFile is cached and shared JVM-wide, and closing
it below would leave
+ // any reader that opened the same jar first with
"IllegalStateException: zip file closed".
+ jarConnection.setUseCaches(false);
+ // Derived from the package rather than from
JarURLConnection#getEntryName. On a multi-release
+ // jar the loader resolves the package to META-INF/versions/N/<pkg>/ on
JDK 9-23 but to <pkg>/
+ // on 8 and 24+, so anchoring to the entry name would make the result
JDK dependent and would
+ // drop classes that exist only in the base directory.
+ String entryPrefix = packageName.replace('.', '/') + '/';
+ try (JarFile jar = jarConnection.getJarFile()) {
+ // Collected before the jar is closed, since the returned stream
outlives this method.
+ return jar.stream()
+ .map(JarEntry::getName)
+ .filter(name -> name.startsWith(entryPrefix) &&
name.endsWith(CLASS_FILE_SUFFIX))
+ .map(name -> name.substring(0, name.length() -
CLASS_FILE_SUFFIX.length()).replace('/', '.'))
+ .collect(Collectors.toList())
+ .stream();
+ }
+ } catch (IOException e) {
+ log.error("Unable to read jar for {}", resource, e);
+ return Stream.empty();
+ }
+ }
+
/**
* Converts a package resource {@link URL} to a {@link File} directory, or
{@code null} if the URI is malformed or does not represent a file.
*
@@ -171,11 +240,18 @@ public class ReflectionUtils {
return classes;
}
File[] files = directory.listFiles();
- for (File file : Objects.requireNonNull(files)) {
+ if (files == null) {
+ // Null for an unreadable directory, or for a package path that is a
regular file. Skipping it
+ // keeps one bad classpath entry from failing the whole scan, as the jar
branch above does.
+ log.warn("Unable to list {}, skipping it", directory);
+ return classes;
+ }
+ for (File file : files) {
if (file.isDirectory()) {
classes.addAll(findClasses(file, packageName + "." + file.getName()));
- } else if (file.getName().endsWith(".class")) {
- classes.add(packageName + '.' + file.getName().substring(0,
file.getName().length() - 6));
+ } else if (file.getName().endsWith(CLASS_FILE_SUFFIX)) {
+ classes.add(packageName + '.'
+ + file.getName().substring(0, file.getName().length() -
CLASS_FILE_SUFFIX.length()));
}
}
return classes;
