hudi-agent commented on code in PR #19784:
URL: https://github.com/apache/hudi/pull/19784#discussion_r3901123754
##########
hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -132,26 +132,30 @@ public static Stream<String>
getTopLevelClassesInClasspath(Class<?> clazz) {
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
String packageName = clazz.getPackage().getName();
String path = packageName.replace('.', '/');
- Enumeration<URL> resources = null;
try {
- resources = classLoader.getResources(path);
+ return Collections.list(classLoader.getResources(path)).stream()
+ .map(ReflectionUtils::toDirectory)
+ .filter(Objects::nonNull)
+ .flatMap(directory -> findClasses(directory, packageName).stream());
} catch (IOException e) {
log.error("Unable to fetch Resources in package {}", packageName, e);
+ return Stream.empty();
}
- List<File> directories = new ArrayList<>();
- while (Objects.requireNonNull(resources).hasMoreElements()) {
- URL resource = resources.nextElement();
- try {
- directories.add(new File(resource.toURI()));
- } catch (URISyntaxException e) {
- log.error("Unable to get URI for {}", resource, e);
- }
- }
- List<String> classes = new ArrayList<>();
- for (File directory : directories) {
- classes.addAll(findClasses(directory, packageName));
+ }
+
+ /**
+ * Converts a package resource {@link URL} to a {@link File} directory, or
{@code null} if the URI is malformed.
+ *
+ * @param resource the package resource URL
+ * @return the corresponding directory, or {@code null} on a malformed URI
+ */
+ private static File toDirectory(URL resource) {
+ try {
+ return new File(resource.toURI());
+ } catch (URISyntaxException | IllegalArgumentException e) {
Review Comment:
🤖 nit: `toDirectory` is a bit ambiguous since it specifically converts a
classpath resource URL — something like `resourceToDirectory` or
`urlToDirectory` might make the intent clearer at the call site.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-io/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -132,26 +132,30 @@ public static Stream<String>
getTopLevelClassesInClasspath(Class<?> clazz) {
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
String packageName = clazz.getPackage().getName();
String path = packageName.replace('.', '/');
- Enumeration<URL> resources = null;
try {
- resources = classLoader.getResources(path);
+ return Collections.list(classLoader.getResources(path)).stream()
+ .map(ReflectionUtils::toDirectory)
+ .filter(Objects::nonNull)
+ .flatMap(directory -> findClasses(directory, packageName).stream());
} catch (IOException e) {
Review Comment:
🤖 Worth noting the `testGetTopLevelClassesInClasspath` added in e667a5f6
only exercises the happy path — it discovers real classes through the actual
context ClassLoader, so the `IOException → Stream.empty()` branch you flagged
here still isn't covered. Reaching it would need a custom ClassLoader whose
`getResources` throws, as you suggested, to assert the method returns an empty
stream rather than propagating.
--
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]
