Support for same named fields in class hierarchy

Added an option/strategy to influence how CachedFields are
recognized in FieldSerieliazer and CompatibleFieldSerializer. It is
an improvement for FieldSerializer (influences getField/removeField
methods) and nearly a must have in CompatibleFieldSerialzer in case
your class hierarchy contains same named fields (e.g. class A has a
field named "a", and class B extends A and also contains a field
named "a"). Per default, both mentioned serializers behave like
before (DEFAULT strategy used). To change it, use
`kryo.getFieldSerializerConfig().setCachedFieldNameStrategy(
FieldSerializer.CachedFieldNameStrategy.EXTENDED);`. At this moment,
class simple name is added to CachedField name. This helps in
CompatibleFieldSerializer write the right value to right class
field. Without EXTENDED, all same named values are written just to
the first found field.

Resolves: #424
See also: #187

Author: phamrak

README.md

@@ -479,7 +479,7 @@ A serialization library needs special knowledge on how to create new instances,
 
 The Serializer class has a `copy` method that does the work. These methods can be ignored when implementing application specific serializers if the copying functionality will not be used. All serializers provided with Kryo support copying. Multiple references to the same object and circular references are handled by the framework automatically.
 
-Similar to the `read()` Serializer method, `kryo.reference()` must be called before Kryo can be used to copy child objects. See [Serializers](#Serializers) for more information.
+Similar to the `read()` Serializer method, `kryo.reference()` must be called before Kryo can be used to copy child objects. See [Serializers](#serializers) for more information.
 
 Similar to KryoSerializable, classes can implement KryoCopyable to do their own copying:
 
@@ -561,7 +561,20 @@ VersionFieldSerializer extends FieldSerializer and allows fields to have a `@Sin
 
 TaggedFieldSerializer extends FieldSerializer to only serialize fields that have a `@Tag(int)` annotation, providing backward compatibility so new fields can be added. And it also provides forward compatibility by `setIgnoreUnknownTags(true)`, thus any unknown field tags will be ignored. TaggedFieldSerializer has two advantages over VersionFieldSerializer: 1) fields can be renamed and 2) fields marked with the `@Deprecated` annotation will be ignored when reading old bytes and won't be written to new bytes. Deprecation effectively removes the field from serialization, though the field and `@Tag` annotation must remain in the class. Deprecated fields can optionally be made private and/or renamed so they don't clutter the class (eg, `ignored`, `ignored2`). For these reasons, TaggedFieldSerializer generally provides more flexibility for classes to evolve. The downside is that it has a small amount of additional overhead compared to VersionFieldSerializer (an additional varint per field).
 
-CompatibleFieldSerializer extends FieldSerializer to provide both forward and backward compatibility, meaning fields can be added or removed without invalidating previously serialized bytes. Changing the type of a field is not supported. Like FieldSerializer, it can serialize most classes without needing annotations. The forward and backward compatibility comes at a cost: the first time the class is encountered in the serialized bytes, a simple schema is written containing the field name strings. Also, during serialization and deserialization buffers are allocated to perform chunked encoding. This is what enables CompatibleFieldSerializer to skip bytes for fields it does not know about. When Kryo is configured to use references, there can be a [problem](https://github.com/EsotericSoftware/kryo/issues/286#issuecomment-74870545) with CompatibleFieldSerializer if a field is removed.
+CompatibleFieldSerializer extends FieldSerializer to provide both forward and backward compatibility, meaning fields can be added or removed without invalidating previously serialized bytes. Changing the type of a field is not supported. Like FieldSerializer, it can serialize most classes without needing annotations. The forward and backward compatibility comes at a cost: the first time the class is encountered in the serialized bytes, a simple schema is written containing the field name strings. Also, during serialization and deserialization buffers are allocated to perform chunked encoding. This is what enables CompatibleFieldSerializer to skip bytes for fields it does not know about. When Kryo is configured to use references, there can be a [problem](https://github.com/EsotericSoftware/kryo/issues/286#issuecomment-74870545) with CompatibleFieldSerializer if a field is removed. In case your class inheritance hierarchy contains same named fields, use the `CachedFieldNameStrategy.EXTENDED` strategy.
+
+```java
+class A {
+	String a;
+}
+
+class B extends A {
+	String a;
+}
+...
+// use `EXTENDED` name strategy, otherwise serialized object can't be deserialized correctly. Attention, `EXTENDED` strategy increases the serialized footprint.
+kryo.getFieldSerializerConfig().setCachedFieldNameStrategy(FieldSerializer.CachedFieldNameStrategy.EXTENDED);
+```
 
 Additional serializers can easily be developed for forward and backward compatibility, such as a serializer that uses an external, hand written schema.
 

src/com/esotericsoftware/kryo/Kryo.java

@@ -462,7 +462,7 @@ public Registration register (Registration registration) {
 		Registration existing = getRegistration(registration.getId());
 		if (DEBUG && existing != null && existing.getType() != registration.getType()) {
 			debug("An existing registration with a different type already uses ID: " + registration.getId()
-				+ "\nExisting registration: " + existing + "\nUnable to set registration: " + registration);
+				+ "\nExisting registration: " + existing + "\nis now overwritten with: " + registration);
 		}
 
 		return classResolver.register(registration);

src/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializer.java

@@ -58,7 +58,7 @@ public void write (Kryo kryo, Output output, T object) {
 			if (TRACE) trace("kryo", "Write " + fields.length + " field names.");
 			output.writeVarInt(fields.length, true);
 			for (int i = 0, n = fields.length; i < n; i++)
-				output.writeString(fields[i].field.getName());
+				output.writeString(getCachedFieldName(fields[i]));
 		}
 
 		OutputChunked outputChunked = new OutputChunked(output, 1024);
@@ -88,7 +88,7 @@ public T read (Kryo kryo, Input input, Class<T> type) {
 					for (int i = 0; i < length; i++) {
 						String schemaName = names[i];
 						for (int ii = 0, nn = allFields.length; ii < nn; ii++) {
-							if (allFields[ii].field.getName().equals(schemaName)) {
+							if (getCachedFieldName(allFields[ii]).equals(schemaName)) {
 								fields[i] = allFields[ii];
 								continue outer;
 							}
@@ -108,7 +108,7 @@ public T read (Kryo kryo, Input input, Class<T> type) {
 
 						while (low <= high) {
 							mid = (low + high) >>> 1;
-							String midVal = allFields[mid].field.getName();
+							String midVal = getCachedFieldName(allFields[mid]);
 							compare = schemaName.compareTo(midVal);
 
 							if (compare < 0) {
@@ -137,7 +137,7 @@ else if (compare > 0) {
 				// Generic type used to instantiate this field could have
 				// been changed in the meantime. Therefore take the most
 				// up-to-date definition of a field
-				cachedField = getField(cachedField.field.getName());
+				cachedField = getField(getCachedFieldName(cachedField));
 			}
 			if (cachedField == null) {
 				if (TRACE) trace("kryo", "Skip obsolete field.");

src/com/esotericsoftware/kryo/serializers/FieldSerializer.java

@@ -410,7 +410,7 @@ private CachedFieldFactory getUnsafeFieldFactory () {
 
 	public int compare (CachedField o1, CachedField o2) {
 		// Fields are sorted by alpha so the order of the data is known.
-		return o1.field.getName().compareTo(o2.field.getName());
+		return getCachedFieldName(o1).compareTo(getCachedFieldName(o2));
 	}
 
 	/** Sets the default value for {@link CachedField#setCanBeNull(boolean)}. Calling this method resets the {@link #getFields()
@@ -542,15 +542,19 @@ protected T create (Kryo kryo, Input input, Class<T> type) {
 	/** Allows specific fields to be optimized. */
 	public CachedField getField (String fieldName) {
 		for (CachedField cachedField : fields)
-			if (cachedField.field.getName().equals(fieldName)) return cachedField;
+			if (getCachedFieldName(cachedField).equals(fieldName)) return cachedField;
 		throw new IllegalArgumentException("Field \"" + fieldName + "\" not found on class: " + type.getName());
 	}
 
+	protected String getCachedFieldName(CachedField cachedField) {
+		return config.getCachedFieldNameStrategy().getName(cachedField);
+	}
+
 	/** Removes a field so that it won't be serialized. */
 	public void removeField (String fieldName) {
 		for (int i = 0; i < fields.length; i++) {
 			CachedField cachedField = fields[i];
-			if (cachedField.field.getName().equals(fieldName)) {
+			if (getCachedFieldName(cachedField).equals(fieldName)) {
 				CachedField[] newFields = new CachedField[fields.length - 1];
 				System.arraycopy(fields, 0, newFields, 0, i);
 				System.arraycopy(fields, i + 1, newFields, i, newFields.length - i);
@@ -562,7 +566,7 @@ public void removeField (String fieldName) {
 
 		for (int i = 0; i < transientFields.length; i++) {
 			CachedField cachedField = transientFields[i];
-			if (cachedField.field.getName().equals(fieldName)) {
+			if (getCachedFieldName(cachedField).equals(fieldName)) {
 				CachedField[] newFields = new CachedField[transientFields.length - 1];
 				System.arraycopy(transientFields, 0, newFields, 0, i);
 				System.arraycopy(transientFields, i + 1, newFields, i, newFields.length - i);
@@ -721,6 +725,25 @@ public static interface CachedFieldFactory {
 		public CachedField createCachedField (Class fieldClass, Field field, FieldSerializer ser);
 	}
 
+	public interface CachedFieldNameStrategy {
+
+		CachedFieldNameStrategy DEFAULT = new CachedFieldNameStrategy() {
+			@Override
+			public String getName(CachedField cachedField) {
+				return cachedField.field.getName();
+			}
+		};
+
+		CachedFieldNameStrategy EXTENDED = new CachedFieldNameStrategy() {
+			@Override
+			public String getName(CachedField cachedField) {
+				return cachedField.field.getDeclaringClass().getSimpleName() + "." + cachedField.field.getName();
+			}
+		};
+
+		String getName(CachedField cachedField);
+	}
+
 	/** Indicates a field should be ignored when its declaring class is registered unless the {@link Kryo#getContext() context} has
 	 * a value set for the specified key. This can be useful when a field must be serialized for one purpose, but not for another.
 	 * Eg, a class for a networked application could have a field that should not be serialized and sent to clients, but should be

src/com/esotericsoftware/kryo/serializers/FieldSerializerConfig.java

@@ -38,6 +38,8 @@ public class FieldSerializerConfig implements Cloneable {
     /** If set, transient fields will be serialized */
     private boolean serializeTransient = false;
 
+    private FieldSerializer.CachedFieldNameStrategy cachedFieldNameStrategy = FieldSerializer.CachedFieldNameStrategy.DEFAULT;
+
     {
         useAsm = !FieldSerializer.unsafeAvailable;
         if (TRACE) trace("kryo.FieldSerializerConfig", "useAsm: " + useAsm);
@@ -136,4 +138,13 @@ public boolean isCopyTransient() {
     public boolean isSerializeTransient() {
         return serializeTransient;
     }
+
+    public FieldSerializer.CachedFieldNameStrategy getCachedFieldNameStrategy() {
+        return cachedFieldNameStrategy;
+    }
+
+    public void setCachedFieldNameStrategy(FieldSerializer.CachedFieldNameStrategy cachedFieldNameStrategy) {
+        this.cachedFieldNameStrategy = cachedFieldNameStrategy;
+        if (TRACE) trace("kryo.FieldSerializerConfig", "CachedFieldNameStrategy: " + cachedFieldNameStrategy);
+    }
 }

test/com/esotericsoftware/kryo/CompatibleFieldSerializerTest.java

@@ -15,13 +15,14 @@
  * SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
  * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
  * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
-
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
 package com.esotericsoftware.kryo;
 
 import java.io.FileNotFoundException;
 
 import com.esotericsoftware.kryo.serializers.CompatibleFieldSerializer;
+import com.esotericsoftware.kryo.serializers.FieldSerializer;
 
 /** @author Nathan Sweet <[email protected]> */
 public class CompatibleFieldSerializerTest extends KryoTestCase {
@@ -71,6 +72,21 @@ public void testRemovedField () throws FileNotFoundException {
 		assertEquals(object1, object2);
 	}
 
+	public void testExtendedClass() throws FileNotFoundException {
+		ExtendedTestClass extendedObject = new ExtendedTestClass();
+
+		// this test would fail with DEFAULT field name strategy
+		kryo.getFieldSerializerConfig().setCachedFieldNameStrategy(FieldSerializer.CachedFieldNameStrategy.EXTENDED);
+
+		CompatibleFieldSerializer serializer = new CompatibleFieldSerializer(kryo, ExtendedTestClass.class);
+		kryo.register(ExtendedTestClass.class, serializer);
+		roundTrip(286, 286, extendedObject);
+
+		ExtendedTestClass object2 = (ExtendedTestClass) kryo.readClassAndObject(input);
+		assertEquals(extendedObject, object2);
+	}
+
+
 	static public class TestClass {
 		public String text = "something";
 		public int moo = 120;
@@ -97,7 +113,36 @@ public boolean equals (Object obj) {
 		}
 	}
 
+	static public class ExtendedTestClass extends TestClass {
+		// keep the same names of attributes like TestClass
+		public String text = "extendedSomething";
+		public int moo = 127;
+		public long moo2 = 5555;
+		public TestClass child;
+		public int zzz = 222;
+		public AnotherClass other;
+
+		public boolean equals (Object obj) {
+			if (this == obj) return true;
+			if (obj == null) return false;
+			if (getClass() != obj.getClass()) return false;
+			ExtendedTestClass other = (ExtendedTestClass) obj;
+
+			if (!super.equals(obj)) return false;
+			if (child == null) {
+				if (other.child != null) return false;
+			} else if (!child.equals(other.child)) return false;
+			if (moo != other.moo) return false;
+			if (moo2 != other.moo2) return false;
+			if (text == null) {
+				if (other.text != null) return false;
+			} else if (!text.equals(other.text)) return false;
+			if (zzz != other.zzz) return false;
+			return true;
+		}
+	}
+
 	static public class AnotherClass {
 		String value;
 	}
-}
+}

test/com/esotericsoftware/kryo/FieldSerializerInheritanceTest.java

@@ -0,0 +1,148 @@
+/* Copyright (c) 2008, Nathan Sweet
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
+ * - Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided with the distribution.
+ * - Neither the name of Esoteric Software nor the names of its contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
+ * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
+ * SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
+package com.esotericsoftware.kryo;
+
+import com.esotericsoftware.kryo.serializers.FieldSerializer;
+import org.junit.Assert;
+
+/**
+ * Created by phamrak on 8.6.2016.
+ */
+public class FieldSerializerInheritanceTest extends KryoTestCase {
+    public void testDefaultStrategyForDefaultClass() {
+        TestDefault testDefault = new TestDefault();
+        testDefault.a = "someDefaultValue";
+        kryo.setDefaultSerializer(FieldSerializer.class);
+        kryo.register(TestDefault.class);
+
+        roundTrip(17, 17, testDefault);
+
+        FieldSerializer serializer = (FieldSerializer) kryo.getSerializer(TestDefault.class);
+        assertNotNull(serializer.getField("a"));
+        serializer.removeField("a");
+        assertFieldRemoved(serializer, "a");
+    }
+
+    public void testDefaultStrategyForExtendedClass() {
+        TestExtended testExtended = new TestExtended();
+        ((TestDefault) testExtended).a = "someDefaultValue";
+        testExtended.a = "someExtendedValue";
+        kryo.setDefaultSerializer(FieldSerializer.class);
+        kryo.register(TestExtended.class);
+
+        roundTrip(34, 34, testExtended);
+
+        FieldSerializer serializer = (FieldSerializer) kryo.getSerializer(TestExtended.class);
+
+        // the "a" field needs to be removed 2x, once for TestDefault.a and once for TestExtended.a. You
+        // can't remove the second one without removing the first one (in DEFAULT field name strategy)
+        assertNotNull(serializer.getField("a"));
+        serializer.removeField("a");
+        assertNotNull(serializer.getField("a"));
+        serializer.removeField("a");
+        assertFieldRemoved(serializer, "a");
+    }
+
+    public void testExtendedStrategyForExtendedClass() {
+        TestExtended testExtended = new TestExtended();
+        ((TestDefault) testExtended).a = "someDefaultValue";
+        testExtended.a = "someExtendedValue";
+        kryo.getFieldSerializerConfig().setCachedFieldNameStrategy(FieldSerializer.CachedFieldNameStrategy.EXTENDED);
+        kryo.setDefaultSerializer(FieldSerializer.class);
+        kryo.register(TestExtended.class);
+
+        roundTrip(34, 34, testExtended);
+
+        FieldSerializer serializer = (FieldSerializer) kryo.getSerializer(TestExtended.class);
+
+        // Simple class name is part of field name in EXTENDED field name strategy.
+        assertNotNull(serializer.getField("TestDefault.a"));
+        serializer.removeField("TestDefault.a");
+        assertFieldRemoved(serializer, "TestDefault.a");
+        assertNotNull(serializer.getField("TestExtended.a"));
+        serializer.removeField("TestExtended.a");
+        assertFieldRemoved(serializer, "TestExtended.a");
+    }
+
+    protected void assertFieldRemoved(FieldSerializer serializer, String fieldName) {
+        try {
+            assertNull(serializer.getField(fieldName));
+            Assert.fail("Expected IllegalArgumentException to be thrown for serializer.getField(" + fieldName + ")");
+        } catch (IllegalArgumentException iae) {
+            assertTrue(true);
+        }
+    }
+
+    static public class TestDefault {
+        private String a;
+
+        public String getA() {
+            return a;
+        }
+
+        public void setA(String a) {
+            this.a = a;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            TestDefault that = (TestDefault) o;
+
+            return a != null ? a.equals(that.a) : that.a == null;
+
+        }
+
+        @Override
+        public int hashCode() {
+            return a != null ? a.hashCode() : 0;
+        }
+    }
+
+    static public class TestExtended extends TestDefault {
+        private String a;
+
+        public String getA() {
+            return a;
+        }
+
+        public void setA(String a) {
+            this.a = a;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            if (!super.equals(o)) return false;
+
+            TestExtended that = (TestExtended) o;
+            return a != null ? a.equals(that.a) : that.a == null;
+        }
+
+        @Override
+        public int hashCode() {
+            return a != null ? a.hashCode() : 0;
+        }
+    }
+}

test/com/esotericsoftware/kryo/FieldSerializerTest.java

@@ -621,7 +621,7 @@ public void testMultipleTimesAnnotatedMapFields () {
 
 		assertFalse("Exception was expected", true);
 	}
-	
+
 	static public class DefaultTypes {
 		// Primitives.
 		public boolean booleanField;