reuvenlax commented on code in PR #32757:
URL: https://github.com/apache/beam/pull/32757#discussion_r1799816927


##########
sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ReflectUtils.java:
##########
@@ -88,14 +89,23 @@ public static List<Method> getMethods(Class<?> clazz) {
     return DECLARED_METHODS.computeIfAbsent(
         clazz,
         c -> {
-          return Arrays.stream(c.getDeclaredMethods())
-              .filter(
-                  m -> !m.isBridge()) // Covariant overloads insert bridge 
functions, which we must
-              // ignore.
-              .filter(m -> !Modifier.isPrivate(m.getModifiers()))
-              .filter(m -> !Modifier.isProtected(m.getModifiers()))
-              .filter(m -> !Modifier.isStatic(m.getModifiers()))
-              .collect(Collectors.toList());
+          List<Method> methods = Lists.newArrayList();
+          do {
+            if (c.getPackage() != null && 
c.getPackage().getName().startsWith("java.")) {
+              break; // skip java built-in classes
+            }
+            Arrays.stream(c.getDeclaredMethods())
+                .filter(
+                    m ->
+                        !m.isBridge()) // Covariant overloads insert bridge 
functions, which we must
+                // ignore.
+                .filter(m -> !Modifier.isPrivate(m.getModifiers()))
+                .filter(m -> !Modifier.isProtected(m.getModifiers()))
+                .filter(m -> !Modifier.isStatic(m.getModifiers()))
+                .forEach(methods::add);
+            c = c.getSuperclass();

Review Comment:
   This was an existing bug that was uncovered by the new unit tests I added in 
this PR. JavaFieldSchema (for POJOs) finds all fields in superclasses and adds 
them to the schema, but we didn't do that for JavaBeans. I changed this to make 
things consistent and fix the broken tests.
   
   BTW related to your other comments - we should probably mark all of these. 
helper classes as Internal. They were never meant for external users to use 
them, and this change (among others) is at least a functionality change
   



-- 
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]

Reply via email to