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.
@@ -21,8 +21,8 @@
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -42,9 +42,9 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.introspect.BasicClassIntrospector;
-import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
 import com.fasterxml.jackson.databind.introspect.ClassIntrospector;
+import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
 /**
@@ -208,7 +208,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()) {
@@ -317,33 +317,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();
 
-			if (child instanceof ObjectNode && sourceValue != null) {
+			Object mappedKey = mapper.readValue(quote(key), keyType);
+			Object sourceValue = source.get(mappedKey);
 
-				doMerge((ObjectNode) child, sourceValue, mapper);
+			if (value instanceof ObjectNode && sourceValue != null) {
 
-			} else if (child instanceof ArrayNode && sourceValue != null) {
+				doMerge((ObjectNode) value, sourceValue, mapper);
 
-				handleArray(child, sourceValue, mapper, type);
+			} else if (value instanceof ArrayNode && sourceValue != null) {
+
+				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();
@@ -374,13 +379,30 @@ private static Collection<Object> ifCollection(Object source) {
 	}
 
 	/**
-	 * Simple value object to capture a mapping of Jackson mapped field names and {@link PersistentProperty} instances.
+	 * 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();
+	}
+
+	/**
+	 * Simple value object to capture a mapping of Jackson mapped field names and {@link PersistentProperty} instances.
 	 *
 	 * @author Oliver Gierke
 	 */
-	@SuppressWarnings("unchecked")
 	static class MappedProperties {
 
 		private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector();

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));
@@ -444,6 +447,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) {
 
@@ -551,4 +567,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;
+	}
 }