Duplicate serializations occur for inherited getters

[mikejhill]

It looks like #46 produced a bug for serialization when getter methods are inherited. Take the following class structure as an example:

class A {
    private int id;
    public int getId() {
        return this.id;
    }
}
class B extends A {}
class C extends B {}

Serializing an instance of class C will produce a result similar to the following: {"id": 0", "id": 0", "id": 0}.

Sorry, I'm not able to drill down to the exact cause for this, but it looks like the library is now traversing the class hierarchy multiple times. When serializing, the library searches each level of the class hierarchy for accessible properties. However, all inherited properties are included as well when searching for accessible properties. In the example above, this means that serialization of an instance of C writes getId() once when serializing properties for C, once when serializing properties for B, and finally once when serializing the properties of A for a total of three times.

My best guess is that this worked fine when just using declared fields (before #46) since inherited fields weren't checked.

Removing the additional class hierarchy traversal seems to fix the issue in my limited testing.

--- original
+++ new
@@ -317,44 +317,40 @@
     void writeObject(Object obj) throws IOException {
       jgen.writeStartObject();
 
-      Class cls = obj.getClass();
-      while(!cls.equals(Object.class)) {
-        List<AccessibleProperty> fields = getAccessibleProperties(cls);
-
-        for(AccessibleProperty property : fields) {
-          try {
-            //if the field has a serializer annotation on it, serialize with it
-            if(fieldAllowed(property, obj.getClass())) {
-              Object val = readField(obj, property);
-              if(!valueAllowed(property, val, obj.getClass())) {
-                continue;
-              }
-
-              String name = getFieldName(property);
-              jgen.writeFieldName(name);
-
-              JsonSerializer fieldSerializer = annotatedWithJsonSerialize(property);
-              if(fieldSerializer != null) {
-                fieldSerializer.serialize(val, jgen, serializerProvider);
-              } else if(customSerializersMap != null && val != null) {
-                JsonSerializer<Object> serializer = customSerializersMap.get(val.getClass());
-                if(serializer != null) {
-                  serializer.serialize(val, jgen, serializerProvider);
-                } else {
-                  new JsonWriter(jgen, result, currentMatch, currentPath, path, property, serializerProvider).write(name, val);
-                }
-              } else if(val instanceof JsonNode) {
-                // Let Jackson deal with these, they're special
-                serializerProvider.defaultSerializeValue(val, jgen);
+      List<AccessibleProperty> fields = getAccessibleProperties(obj.getClass());
+
+      for(AccessibleProperty property : fields) {
+        try {
+          //if the field has a serializer annotation on it, serialize with it
+          if(fieldAllowed(property, obj.getClass())) {
+            Object val = readField(obj, property);
+            if(!valueAllowed(property, val, obj.getClass())) {
+              continue;
+            }
+
+            String name = getFieldName(property);
+            jgen.writeFieldName(name);
+
+            JsonSerializer fieldSerializer = annotatedWithJsonSerialize(property);
+            if(fieldSerializer != null) {
+              fieldSerializer.serialize(val, jgen, serializerProvider);
+            } else if(customSerializersMap != null && val != null) {
+              JsonSerializer<Object> serializer = customSerializersMap.get(val.getClass());
+              if(serializer != null) {
+                serializer.serialize(val, jgen, serializerProvider);
               } else {
                 new JsonWriter(jgen, result, currentMatch, currentPath, path, property, serializerProvider).write(name, val);
               }
+            } else if(val instanceof JsonNode) {
+              // Let Jackson deal with these, they're special
+              serializerProvider.defaultSerializeValue(val, jgen);
+            } else {
+              new JsonWriter(jgen, result, currentMatch, currentPath, path, property, serializerProvider).write(name, val);
             }
-          } catch(IllegalArgumentException | IllegalAccessException e) {
-            throw new RuntimeException(e);
           }
-        }
-        cls = cls.getSuperclass();
+        } catch(IllegalArgumentException | IllegalAccessException e) {
+          throw new RuntimeException(e);
+        }
       }
 
       jgen.writeEndObject();

[monitorjbl]

So the unit tests I had in place were designed to operate on a Map, not the raw JSON. Because the map replaces any duplicates, the tests never had any issues. I've added a new Map implementation that will barf if given duplicate keys to prevent this from happening again.

I'll push out 1.0.1 tonight, this bug is bad enough that I don't want to sit on it.

[monitorjbl]

1.0.1 should be available now on Maven Central.