Add check if primitive and wrapper types are assignable and remove condition for valueClass

Author: theigl

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

@@ -30,6 +30,7 @@
 import com.esotericsoftware.kryo.io.Output;
 import com.esotericsoftware.kryo.io.OutputChunked;
 import com.esotericsoftware.kryo.util.ObjectMap;
+import com.esotericsoftware.kryo.util.Util;
 
 /** Serializes objects using direct field assignment, providing both forward and backward compatibility. This means fields can be
  * added or removed without invalidating previously serialized bytes. Renaming or changing the type of a field is not supported.
@@ -90,13 +91,7 @@ public void write (Kryo kryo, Output output, T object) {
 				try {
 					if (object != null) {
 						Object value = cachedField.field.get(object);
-						if (value != null) {
-							final Class<?> fieldType = cachedField.field.getType();
-							if (fieldType.isPrimitive())
-								valueClass = fieldType;
-							else
-								valueClass = value.getClass();
-						}
+						if (value != null) valueClass = value.getClass();
 					}
 				} catch (IllegalAccessException ex) {
 				}
@@ -168,7 +163,7 @@ public T read (Kryo kryo, Input input, Class<? extends T> type) {
 				}
 
 				// Ensure the type in the data is compatible with the field type.
-				if (cachedField.valueClass != null && !cachedField.valueClass.isAssignableFrom(valueClass)) {
+				if (cachedField.valueClass != null && !Util.isAssignableTo(valueClass, cachedField.valueClass)) {
 					String message = "Read type is incompatible with the field type: " + className(valueClass) + " -> "
 						+ className(cachedField.valueClass) + " (" + getType().getName() + "#" + cachedField + ")";
 					if (!chunked) throw new KryoException(message);

src/com/esotericsoftware/kryo/util/Util.java

@@ -27,6 +27,8 @@
 import com.esotericsoftware.kryo.util.Generics.GenericType;
 
 import java.lang.reflect.Type;
+import java.util.HashMap;
+import java.util.Map;
 
 /** A few utility methods, mostly for private use.
  * @author Nathan Sweet */
@@ -53,6 +55,18 @@ public class Util {
 	// Maximum reasonable array length. See: https://stackoverflow.com/questions/3038392/do-java-arrays-have-a-maximum-size
 	public static final int maxArraySize = Integer.MAX_VALUE - 8;
 
+	private static final Map<Class<?>, Class<?>> primitiveWrappers = new HashMap<>();
+	static {
+		primitiveWrappers.put(boolean.class, Boolean.class);
+		primitiveWrappers.put(byte.class, Byte.class);
+		primitiveWrappers.put(char.class, Character.class);
+		primitiveWrappers.put(double.class, Double.class);
+		primitiveWrappers.put(float.class, Float.class);
+		primitiveWrappers.put(int.class, Integer.class);
+		primitiveWrappers.put(long.class, Long.class);
+		primitiveWrappers.put(short.class, Short.class);
+	}
+
 	public static boolean isClassAvailable (String className) {
 		try {
 			Class.forName(className);
@@ -207,6 +221,18 @@ public static Class getElementClass (Class arrayClass) {
 		return elementClass;
 	}
 
+	public static boolean isAssignableTo (Class<?> from, Class<?> to) {
+		if (to.isAssignableFrom(from)) return true;
+		if (from.isPrimitive()) return isPrimitiveWrapperOf(to, from);
+		if (to.isPrimitive()) return isPrimitiveWrapperOf(from, to);
+		return false;
+	}
+
+	private static boolean isPrimitiveWrapperOf (Class<?> targetClass, Class<?> primitive) {
+		if (!primitive.isPrimitive()) throw new IllegalArgumentException("First argument has to be primitive type");
+		return primitiveWrappers.get(primitive) == targetClass;
+	}
+
 	public static boolean isAscii (String value) {
 		for (int i = 0, n = value.length(); i < n; i++)
 			if (value.charAt(i) > 127) return false;

test/com/esotericsoftware/kryo/serializers/CompatibleFieldSerializerTest.java

@@ -243,6 +243,28 @@ private void testChangeFieldType(int length, boolean chunked) {
 		assertNull(o.value);
 	}
 
+	@Test
+	public void testChangePrimitiveAndWrapperFieldTypes () {
+		testChangePrimitiveAndWrapperFieldTypes(26, true);
+		testChangePrimitiveAndWrapperFieldTypes(22, false);
+	}
+
+	private void testChangePrimitiveAndWrapperFieldTypes (int length, boolean chunked) {
+		CompatibleFieldSerializer<ClassWithPrimitiveAndWrapper> serializer = new CompatibleFieldSerializer<>(kryo, ClassWithPrimitiveAndWrapper.class);
+		serializer.getCompatibleFieldSerializerConfig().setChunkedEncoding(chunked);
+		kryo.setReferences(false);
+		kryo.register(ClassWithPrimitiveAndWrapper.class, serializer);
+
+		roundTrip(length, new ClassWithPrimitiveAndWrapper(1, 1L));
+
+		serializer.getField("primitive").setValueClass(Long.class);
+		serializer.getField("wrapper").setValueClass(long.class);
+
+		ClassWithPrimitiveAndWrapper o = (ClassWithPrimitiveAndWrapper)kryo.readClassAndObject(input);
+		assertEquals(1, o.primitive);
+		assertEquals(1L, o.wrapper, 0);
+	}
+
 	@Test
 	public void testRemovedFieldFromClassWithManyFields () {
 		testRemovedFieldFromClassWithManyFields(198, false, false, true);
@@ -566,6 +588,32 @@ public boolean equals (Object obj) {
 		}
 	}
 
+	public static class ClassWithPrimitiveAndWrapper {
+		long primitive;
+		Long wrapper;
+
+		public ClassWithPrimitiveAndWrapper () {
+		}
+
+		public ClassWithPrimitiveAndWrapper (long primitive, Long wrapper) {
+			this.primitive = primitive;
+			this.wrapper = wrapper;
+		}
+
+		@Override
+		public boolean equals (Object o) {
+			if (this == o) return true;
+			if (o == null || getClass() != o.getClass()) return false;
+			final ClassWithPrimitiveAndWrapper that = (ClassWithPrimitiveAndWrapper)o;
+			return primitive == that.primitive && Objects.equals(wrapper, that.wrapper);
+		}
+
+		@Override
+		public int hashCode () {
+			return Objects.hash(primitive, wrapper);
+		}
+	}
+
 	public static class ClassWithSuperTypeFields {
 		private Object value;
 		private Iterable<?> list;

test/com/esotericsoftware/kryo/util/UtilTest.java

@@ -0,0 +1,17 @@
+package com.esotericsoftware.kryo.util;
+
+import junit.framework.TestCase;
+
+public class UtilTest extends TestCase {
+
+    public void testIsAssignableTo() {
+        assertTrue(Util.isAssignableTo(Long.class, long.class));
+        assertTrue(Util.isAssignableTo(long.class, Long.class));
+        assertTrue(Util.isAssignableTo(Long.class, Long.class));
+        assertTrue(Util.isAssignableTo(long.class, long.class));
+
+        assertFalse(Util.isAssignableTo(String.class, Long.class));
+        assertFalse(Util.isAssignableTo(String.class, long.class));
+    }
+
+}