Fokko commented on a change in pull request #874:
URL: https://github.com/apache/avro/pull/874#discussion_r422469102



##########
File path: lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
##########
@@ -737,6 +737,9 @@ protected Schema createSchema(Type type, Map<String, 
Schema> names) {
 
               AvroName annotatedName = field.getAnnotation(AvroName.class); // 
Rename fields
               String fieldName = (annotatedName != null) ? 
annotatedName.value() : field.getName();
+              if ("this$0".equals(fieldName)) {

Review comment:
       Can you split the string out to a static private final variable?

##########
File path: 
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java
##########
@@ -130,4 +130,34 @@ private void validateSchema(Meta meta) {
       assertEquals("Invalid field " + field.name(), field.defaultVal(), 
testCases.get(field.name()));
     }
   }
+
+  public class Definition {
+    public Map<String, String> tokens;
+  }
+
+  @Test
+  public void testInnerClasses() {
+    testStaticInnerClasses();
+    testNonStaticInnerClasses();
+  }
+
+  public void testNonStaticInnerClasses() {

Review comment:
       I'd prefer to use the annotation: `@Test(expected = 
AvroTypeException.class)`
   What do you think?
   
   

##########
File path: 
lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java
##########
@@ -130,4 +130,34 @@ private void validateSchema(Meta meta) {
       assertEquals("Invalid field " + field.name(), field.defaultVal(), 
testCases.get(field.name()));
     }
   }
+
+  public class Definition {
+    public Map<String, String> tokens;
+  }
+
+  @Test
+  public void testInnerClasses() {
+    testStaticInnerClasses();
+    testNonStaticInnerClasses();
+  }
+
+  public void testNonStaticInnerClasses() {
+    boolean successful = false;
+    try {
+      ReflectData.get().getSchema(Definition.class);
+    } catch (AvroTypeException ex) {
+      if (ex.getMessage().contains("must be a static inner class")) {
+        successful = true;
+      }
+    }
+    assertTrue(successful);
+  }
+
+  public void testStaticInnerClasses() {

Review comment:
       Same here, I'd love to use the annotation, and remove the fail("") part. 
If it throws an exception, it will fail anyway




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to