DATAREST-986 - DomainObjectReader now merges complex nested maps correctly.

We now use a Map property's generic type information to make sure we convert both the key and the value into the declared types. Previously we just used Object if the source value to map was null. Object is still used as fallback for raw maps though.

Author: odrotbohm

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -204,7 +204,7 @@ private <T> T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exc
 						continue;
 					}
 
-					doMergeNestedMap((Map<String, Object>) rawValue, objectNode, mapper, property.getTypeInformation());
+					doMergeNestedMap((Map<Object, Object>) rawValue, objectNode, mapper, property.getTypeInformation());
 
 					// Remove potentially emptied Map as values have been handled recursively
 					if (!objectNode.fieldNames().hasNext()) {
@@ -313,33 +313,38 @@ private boolean handleArrayNode(ArrayNode array, Collection<Object> collection,
 	 * @param mapper must not be {@literal null}.
 	 * @throws Exception
 	 */
-	private void doMergeNestedMap(Map<String, Object> source, ObjectNode node, ObjectMapper mapper,
+	private void doMergeNestedMap(Map<Object, Object> source, ObjectNode node, ObjectMapper mapper,
 			TypeInformation<?> type) throws Exception {
 
 		if (source == null) {
 			return;
 		}
 
 		Iterator<Entry<String, JsonNode>> fields = node.fields();
+		Class<?> keyType = typeOrObject(type.getComponentType());
+		Class<?> valueType = typeOrObject(type.getMapValueType());
 
 		while (fields.hasNext()) {
 
 			Entry<String, JsonNode> entry = fields.next();
-			JsonNode child = entry.getValue();
-			Object sourceValue = source.get(entry.getKey());
+			JsonNode value = entry.getValue();
+			String key = entry.getKey();
+
+			Object mappedKey = mapper.readValue(quote(key), keyType);
+			Object sourceValue = source.get(mappedKey);
 
-			if (child instanceof ObjectNode && sourceValue != null) {
+			if (value instanceof ObjectNode && sourceValue != null) {
 
-				doMerge((ObjectNode) child, sourceValue, mapper);
+				doMerge((ObjectNode) value, sourceValue, mapper);
 
-			} else if (child instanceof ArrayNode && sourceValue != null) {
+			} else if (value instanceof ArrayNode && sourceValue != null) {
 
-				handleArray(child, sourceValue, mapper, type);
+				handleArray(value, sourceValue, mapper, type);
 
 			} else {
 
-				Class<?> valueType = sourceValue == null ? Object.class : sourceValue.getClass();
-				source.put(entry.getKey(), mapper.treeToValue(child, valueType));
+				Class<?> typeToRead = sourceValue != null ? sourceValue.getClass() : valueType;
+				source.put(mappedKey, mapper.treeToValue(value, typeToRead));
 			}
 
 			fields.remove();
@@ -368,4 +373,24 @@ private static Collection<Object> ifCollection(Object source) {
 
 		return null;
 	}
+
+	/**
+	 * Surrounds the given source {@link String} with quotes so that they represent a valid JSON String.
+	 * 
+	 * @param source can be {@literal null}.
+	 * @return
+	 */
+	private static String quote(String source) {
+		return source == null ? null : "\"".concat(source).concat("\"");
+	}
+
+	/**
+	 * Returns the raw type of the given {@link TypeInformation} or {@link Object} as fallback.
+	 * 
+	 * @param type can be {@literal null}.
+	 * @return
+	 */
+	private static Class<?> typeOrObject(TypeInformation<?> type) {
+		return type == null ? Object.class : type.getType();
+	}
 }

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

@@ -20,6 +20,7 @@
 import static org.mockito.Mockito.*;
 
 import lombok.AllArgsConstructor;
+import lombok.EqualsAndHashCode;
 import lombok.NoArgsConstructor;
 
 import java.io.ByteArrayInputStream;
@@ -33,6 +34,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import org.junit.Before;
@@ -88,6 +90,7 @@ public void setUp() {
 		mappingContext.getPersistentEntity(Inner.class);
 		mappingContext.getPersistentEntity(Outer.class);
 		mappingContext.getPersistentEntity(Parent.class);
+		mappingContext.getPersistentEntity(Product.class);
 		mappingContext.afterPropertiesSet();
 
 		PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext));
@@ -396,6 +399,19 @@ public void testname() throws Exception {
 		assertThat(mapper.treeToValue(node, Object.class), is((Object) "asd"));
 	}
 
+	@Test // DATAREST-986
+	public void readsComplexMap() throws Exception {
+
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree(
+				"{ \"map\" : { \"en\" : { \"value\" : \"eventual\" }, \"de\" : { \"value\" : \"schlussendlich\" } } }");
+
+		Product result = reader.readPut((ObjectNode) node, new Product(), mapper);
+
+		assertThat(result.map.get(Locale.ENGLISH), is(new LocalizedValue("eventual")));
+		assertThat(result.map.get(Locale.GERMAN), is(new LocalizedValue("schlussendlich")));
+	}
+
 	@SuppressWarnings("unchecked")
 	private static <T> T as(Object source, Class<T> type) {
 
@@ -501,4 +517,17 @@ static class Child {
 	static class Item {
 		String some;
 	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Product {
+		Map<Locale, LocalizedValue> map = new HashMap<Locale, LocalizedValue>();
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	@NoArgsConstructor
+	@AllArgsConstructor
+	@EqualsAndHashCode
+	static class LocalizedValue {
+		String value;
+	}
 }