Copilot commented on code in PR #7661:
URL: https://github.com/apache/ignite-3/pull/7661#discussion_r2852774411
##########
migration-tools/modules/migration-tools-commons/src/main/java/org/apache/ignite/migrationtools/types/TypeInspector.java:
##########
@@ -95,33 +95,39 @@ public static List<InspectedField> inspectType(Class<?>
type) {
} else if (isPrimitiveType(rootType)) {
return
Collections.singletonList(InspectedField.forUnnamed(rootTypeName,
InspectedFieldType.PRIMITIVE));
} else {
- Field[] fields = rootType.getDeclaredFields();
- List<InspectedField> ret = new ArrayList<>(fields.length);
- for (Field field : fields) {
- if (shouldPersistField(field)) {
- @Nullable QuerySqlField annotation =
field.getAnnotation(QuerySqlField.class);
- boolean hasAnnotation = annotation != null;
-
- Class<?> origFieldType = field.getType();
- Class<?> wrappedFieldType =
ClassUtils.primitiveToWrapper(origFieldType);
-
- boolean nullable = !origFieldType.isPrimitive();
-
- InspectedFieldType inspectedFieldType =
isPrimitiveType(wrappedFieldType)
- ? InspectedFieldType.POJO_ATTRIBUTE
- : InspectedFieldType.NESTED_POJO_ATTRIBUTE;
-
- InspectedField inspectedField = InspectedField.forNamed(
- field.getName(),
- wrappedFieldType.getName(),
- inspectedFieldType,
- nullable,
- hasAnnotation
- );
-
- ret.add(inspectedField);
+ Class<?> currentType = type;
+ ArrayList<InspectedField> ret = new ArrayList<>();
+ do {
+ Field[] fields = currentType.getDeclaredFields();
+ ret.ensureCapacity(ret.size() + fields.length);
+ for (Field field : fields) {
+ if (shouldPersistField(field)) {
+ @Nullable QuerySqlField annotation =
field.getAnnotation(QuerySqlField.class);
+ boolean hasAnnotation = annotation != null;
+
+ Class<?> origFieldType = field.getType();
+ Class<?> wrappedFieldType =
ClassUtils.primitiveToWrapper(origFieldType);
+
+ boolean nullable = !origFieldType.isPrimitive();
+
+ InspectedFieldType inspectedFieldType =
isPrimitiveType(wrappedFieldType)
+ ? InspectedFieldType.POJO_ATTRIBUTE
+ : InspectedFieldType.NESTED_POJO_ATTRIBUTE;
+
+ InspectedField inspectedField =
InspectedField.forNamed(
+ field.getName(),
+ wrappedFieldType.getName(),
+ inspectedFieldType,
+ nullable,
+ hasAnnotation
+ );
+
+ ret.add(inspectedField);
+ }
}
- }
+
+ currentType = currentType.getSuperclass();
+ } while (currentType != Object.class && currentType != null);
Review Comment:
`Class#getDeclaredFields()` does not guarantee a stable ordering across
JVMs, and `inspectType` now concatenates results from multiple hierarchy
levels. Returning the fields in reflection order can make downstream behavior
and tests flaky; consider sorting the collected fields deterministically (e.g.,
by declaring-class depth and then field name, or just by field name) before
returning.
##########
migration-tools/modules/migration-tools-commons/src/test/java/org/apache/ignite/migrationtools/types/TypeInspectorTest.java:
##########
@@ -111,4 +113,18 @@ void testNestedPojoAttribute() {
assertThat(inspectedTypes).containsExactly(expected);
}
+
+ @Test
+ void testSuperClass() {
+ List<InspectedField> inspectedTypes =
inspectType(IdentifiedPojo.class);
+
+ InspectedField[] expected = new InspectedField[] {
+ forNamed("name", String.class.getName(),
InspectedFieldType.POJO_ATTRIBUTE, true, false),
+ forNamed("amount", Integer.class.getName(),
InspectedFieldType.POJO_ATTRIBUTE, false, false),
+ forNamed("decimalAmount", Double.class.getName(),
InspectedFieldType.POJO_ATTRIBUTE, true, false),
+ forNamed("id", UUID.class.getName(),
InspectedFieldType.POJO_ATTRIBUTE, true, false)
+ };
+
+ assertThat(inspectedTypes).containsExactly(expected);
+ }
Review Comment:
These assertions use `containsExactly`, but the order of fields returned by
reflection (`getDeclaredFields`) is unspecified and can vary across JVMs. To
avoid flaky tests, consider making the assertions order-insensitive (or sorting
the inspected fields before asserting).
--
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]
