voonhous commented on code in PR #19624:
URL: https://github.com/apache/hudi/pull/19624#discussion_r3968081779
##########
hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -130,19 +135,91 @@ public static Object loadClass(String clazz, Object...
constructorArgs) {
*/
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);
+ // The loader resolves the package to this entry, which on a
multi-release jar is
+ // META-INF/versions/N/<pkg>/ rather than <pkg>/. Anchoring to it keeps
those classes visible
+ // and keeps the reported names rooted at packageName, the way
findClasses does.
+ String entryPrefix = jarConnection.getEntryName();
Review Comment:
**minor:** This prefix (my round-1 ask) makes the result JDK-dependent on a
multi-release jar: the loader resolves the package to `!/<pkg>` on JDK 8 and
24+ but to `!/META-INF/versions/N/<pkg>/` on 9-23, so one jar scans to `[Alpha,
BaseOnly]` on 8 and `[Alpha, VersionedOnly]` on 11/17, dropping base-only
classes (Azure surefire is 11). Reverting to `packageName.replace('.', '/') +
'/'` gives `[Alpha, BaseOnly]` on every JDK and leaves all 12 tests green, so
nothing pins this either. Not blocking, and no Hudi bundle is multi-release:
could we revert to the package-derived prefix (the null guard and slash
normalisation go with it), or, if you prefer to keep it, pin it with a test
that feeds a `!/META-INF/versions/9/<pkg>/` URL through the anonymous-loader
pattern and reword the comment above, which only holds on 9-23?
##########
hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -130,19 +135,91 @@ public static Object loadClass(String clazz, Object...
constructorArgs) {
*/
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)) {
Review Comment:
**minor:** This guard is unreached: dropping it leaves all 12 tests green,
so the body's "each fix is pinned by a test that fails without it" does not
hold for it. It is reachable without a `URLStreamHandlerFactory`: `new
URL(null, "jar:file:/x.jar!/org/apache/hudi/common/util", handler)` with an
anonymous `URLStreamHandler` whose `openConnection` returns a plain
`URLConnection`, fed through the anonymous-`ClassLoader` pattern
`SkipsInvalidResources` already uses (about 8 lines; passes here,
`ClassCastException` with the guard removed). Not blocking: could that test be
added?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
