deepakpanda93 commented on PR #19624: URL: https://github.com/apache/hudi/pull/19624#issuecomment-5598801670
Thanks @voonhous — rebased onto master and scoped down to exactly those two, and I ran the thing you flagged as unverified. **The two fixes that remain** 1. `jar:` URLs. `classNamesIn` now dispatches on protocol: `jar` goes through `JarURLConnection`, anything else keeps the `toDirectory` + `findClasses` path exactly as #19784 merged it. Nothing in the directory branch is touched. 2. `getPackage()` null guard, so `getTopLevelClassesInClasspath(String[].class)` returns an empty stream instead of throwing. Everything #19784 already covers is dropped — the `IOException` handling and the surrounding restructure are gone from the diff because they are already on master. It went from +212/-38 to +165/-4, and the only production change is the protocol dispatch plus the two new helpers. **On the two tests #19784 added — your reading is right, and I ran it** Both pass against this implementation. The `jar:file:/unused.jar!/org/apache/hudi/common/util` case is the interesting one: it no longer falls through `toDirectory`, it now enters `classNamesInJar`, where `getJarFile()` on a jar that does not exist throws `IOException`, which is caught and returns an empty stream. Same outcome, different route. `file:/invalid path` still fails in `toDirectory` as before. `testGetTopLevelClassesInClasspathHandlesIOException` is untouched by the change. `TestReflectionUtils` is 8/8 (7 methods, the parameterized one counting twice); `checkstyle:check` and `apache-rat:check` clean on `hudi-io`. **Both fixes verified by negative control**, not just by the tests passing: | fix reverted | result | | --- | --- | | null guard removed | `...ForClassesWithoutAPackage` errors with `NullPointerException: Cannot invoke "java.lang.Package.getName()" because the return value of "java.lang.Class.getPackage()" is null` | | jar dispatch removed | `...FromJar` fails | So each test detects the bug it is there for, rather than passing for unrelated reasons. The directory and jar fixtures are kept, as you suggested. I dropped two of my earlier tests that #19784's `SkipsInvalidResources` already covers (a real-classpath scan and a package-not-on-the-classpath case) to keep the diff to what is actually new. Thanks also for closing #17675 in favour of this one. -- 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]
