DATAREST-977 - Fixed reading of complex enums on collection expansion for PATCH.

When merging collections on PATCH we now don't use the first collection item's type for all elements but inspect the values for each existing element found. When it comes to appending elements to the collection, wen now just stick to the declared component type as type hint for reading the provided value.

Author: odrotbohm

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java

@@ -340,12 +340,7 @@ private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, T
 			return false;
 		}
 
-		Iterator<Object> iterator = collection.iterator();
-		TypeInformation<?> componentType = iterator.hasNext() ? //
-				ClassTypeInformation.from(iterator.next().getClass()) : //
-				collectionType.getComponentType();
-
-		return handleArrayNode((ArrayNode) node, collection, mapper, componentType);
+		return handleArrayNode((ArrayNode) node, collection, mapper, collectionType.getComponentType());
 	}
 
 	/**
@@ -373,16 +368,15 @@ private boolean handleArrayNode(ArrayNode array, Collection<Object> collection,
 
 			if (!value.hasNext()) {
 
-				Class<?> type = componentType == null ? Object.class : componentType.getType();
-				collection.add(mapper.treeToValue(jsonNode, type));
+				collection.add(mapper.treeToValue(jsonNode, getTypeToMap(null, componentType).getType()));
 
 				continue;
 			}
 
 			Object next = value.next();
 
 			if (ArrayNode.class.isInstance(jsonNode)) {
-				return handleArray(jsonNode, next, mapper, componentType);
+				return handleArray(jsonNode, next, mapper, getTypeToMap(value, componentType));
 			}
 
 			if (ObjectNode.class.isInstance(jsonNode)) {
@@ -417,7 +411,7 @@ private void doMergeNestedMap(Map<Object, Object> source, ObjectNode node, Objec
 
 		Iterator<Entry<String, JsonNode>> fields = node.fields();
 		Class<?> keyType = typeOrObject(type.getComponentType());
-		Class<?> valueType = typeOrObject(type.getMapValueType());
+		TypeInformation<?> valueType = type.getMapValueType();
 
 		while (fields.hasNext()) {
 
@@ -427,19 +421,19 @@ private void doMergeNestedMap(Map<Object, Object> source, ObjectNode node, Objec
 
 			Object mappedKey = mapper.readValue(quote(key), keyType);
 			Object sourceValue = source.get(mappedKey);
+			TypeInformation<?> typeToMap = getTypeToMap(sourceValue, valueType);
 
 			if (value instanceof ObjectNode && sourceValue != null) {
 
 				doMerge((ObjectNode) value, sourceValue, mapper);
 
 			} else if (value instanceof ArrayNode && sourceValue != null) {
 
-				handleArray(value, sourceValue, mapper, type);
+				handleArray(value, sourceValue, mapper, getTypeToMap(sourceValue, typeToMap));
 
 			} else {
 
-				Class<?> typeToRead = sourceValue != null ? sourceValue.getClass() : valueType;
-				source.put(mappedKey, mapper.treeToValue(value, typeToRead));
+				source.put(mappedKey, mapper.treeToValue(value, typeToMap.getType()));
 			}
 
 			fields.remove();
@@ -554,4 +548,30 @@ private static String quote(String source) {
 	private static Class<?> typeOrObject(TypeInformation<?> type) {
 		return type == null ? Object.class : type.getType();
 	}
+
+	/**
+	 * Returns the type to read for the given value and default type. The type will be defaulted to {@link Object} if
+	 * missing. If the given value's type is different from the given default (i.e. more concrete) the value's type will
+	 * be used.
+	 * 
+	 * @param value can be {@literal null}.
+	 * @param type can be {@literal null}.
+	 * @return
+	 */
+	private static TypeInformation<?> getTypeToMap(Object value, TypeInformation<?> type) {
+
+		if (type == null) {
+			type = ClassTypeInformation.OBJECT;
+		}
+
+		if (value == null) {
+			return type;
+		}
+
+		if (Enum.class.isInstance(value)) {
+			return ClassTypeInformation.from(((Enum<?>) value).getDeclaringClass());
+		}
+
+		return value.getClass().equals(type.getType()) ? type : ClassTypeInformation.from(value.getClass());
+	}
 }

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java

@@ -15,7 +15,7 @@
  */
 package org.springframework.data.rest.webmvc.json;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
@@ -93,6 +93,7 @@ public void setUp() {
 		mappingContext.getPersistentEntity(Parent.class);
 		mappingContext.getPersistentEntity(Product.class);
 		mappingContext.getPersistentEntity(TransientReadOnlyProperty.class);
+		mappingContext.getPersistentEntity(CollectionOfEnumWithMethods.class);
 		mappingContext.afterPropertiesSet();
 
 		PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext));
@@ -451,6 +452,20 @@ public void handlesTransientPropertyWithoutFieldProperly() throws Exception {
 		reader.readPut((ObjectNode) node, new TransientReadOnlyProperty(), mapper);
 	}
 
+	@Test // DATAREST-977
+	public void readsCollectionOfComplexEnum() throws Exception {
+
+		CollectionOfEnumWithMethods sample = new CollectionOfEnumWithMethods();
+		sample.enums.add(SampleEnum.FIRST);
+
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree("{ \"enums\" : [ \"SECOND\", \"FIRST\" ] }");
+
+		CollectionOfEnumWithMethods result = reader.merge((ObjectNode) node, sample, mapper);
+
+		assertThat(result.enums, contains(SampleEnum.SECOND, SampleEnum.FIRST));
+	}
+
 	@SuppressWarnings("unchecked")
 	private static <T> T as(Object source, Class<T> type) {
 
@@ -585,4 +600,33 @@ public String getName() {
 
 		public void setName(String name) {}
 	}
+
+	// DATAREST-977
+
+	interface EnumInterface {
+		String getFoo();
+	}
+
+	static enum SampleEnum implements EnumInterface {
+
+		FIRST {
+
+			@Override
+			public String getFoo() {
+				return "first";
+			}
+
+		},
+		SECOND {
+
+			public String getFoo() {
+				return "second";
+			}
+		};
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class CollectionOfEnumWithMethods {
+		List<SampleEnum> enums = new ArrayList<SampleEnum>();
+	}
 }