DATAREST-956 - Polishing.

Some tiny refactorings in DomainObjectReader. We're now using TypeInformation instead of Class to preserve more generics information when it comes to deeper nesting.

Moved some code around in the unit tests.

Original pull request: #245.

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

@@ -33,6 +33,8 @@ import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.SimplePropertyHandler;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.rest.webmvc.mapping.Associations;
+import org.springframework.data.util.ClassTypeInformation;
+import org.springframework.data.util.TypeInformation;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.util.Assert;
 
@@ -177,9 +179,7 @@ public class DomainObjectReader {
 
 			if (child.isArray()) {
 
-				boolean nestedObjectFound = handleArrayNode((ArrayNode) child, asCollection(rawValue), property.getComponentType(), mapper);
-
-				if (nestedObjectFound) {
+				if (handleArray(child, rawValue, mapper, property.getTypeInformation())) {
 					i.remove();
 				}
 
@@ -201,7 +201,7 @@ public class DomainObjectReader {
 						continue;
 					}
 
-					doMergeNestedMap((Map<String, Object>) rawValue, objectNode, mapper);
+					doMergeNestedMap((Map<String, Object>) rawValue, objectNode, mapper, property.getTypeInformation());
 
 					// Remove potentially emptied Map as values have been handled recursively
 					if (!objectNode.fieldNames().hasNext()) {
@@ -221,40 +221,65 @@ public class DomainObjectReader {
 		return mapper.readerForUpdating(target).readValue(root);
 	}
 
+	/**
+	 * Handles the given {@link JsonNode} by treating it as {@link ArrayNode} and the given source value as
+	 * {@link Collection}-like value. Looks up the actual type to handle from the potentially available first element,
+	 * falling back to component type lookup on the given type.
+	 * 
+	 * @param node must not be {@literal null}.
+	 * @param source must not be {@literal null}.
+	 * @param mapper must not be {@literal null}.
+	 * @param collectionType must not be {@literal null}.
+	 * @return
+	 * @throws Exception
+	 */
+	private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation<?> collectionType)
+			throws Exception {
+
+		Collection<Object> collection = asCollection(source);
+		Iterator<Object> iterator = collection.iterator();
+		TypeInformation<?> componentType = iterator.hasNext() ? //
+				ClassTypeInformation.from(iterator.next().getClass()) : //
+				collectionType.getComponentType();
+
+		return handleArrayNode((ArrayNode) node, collection, mapper, componentType);
+	}
+
 	/**
 	 * Applies the diff handling to {@link ArrayNode}s, potentially recursing into nested ones.
 	 * 
 	 * @param array the source {@link ArrayNode}m, must not be {@literal null}.
 	 * @param collection the actual collection values, must not be {@literal null}.
-	 * @param componentType the item type of the collection
-	 * @param mapper the {@link ObjectMapper} to use, must not be {@literal null}.  @return whether an object merge has been applied to the {@link ArrayNode}.
+	 * @param mapper the {@link ObjectMapper} to use, must not be {@literal null}.
+	 * @param componentType the item type of the collection, can be {@literal null}.
+	 * @return whether an object merge has been applied to the {@link ArrayNode}.
 	 */
-	private boolean handleArrayNode(ArrayNode array, Collection<Object> collection, Class<?> componentType, ObjectMapper mapper)
-			throws Exception {
+	private boolean handleArrayNode(ArrayNode array, Collection<Object> collection, ObjectMapper mapper,
+			TypeInformation<?> componentType) throws Exception {
 
 		Assert.notNull(array, "ArrayNode must not be null!");
 		Assert.notNull(collection, "Source collection must not be null!");
 		Assert.notNull(mapper, "ObjectMapper must not be null!");
 
-		//we need an iterator for the original collection - we might modify it but we want to keep iterating over the original collection
+		// We need an iterator for the original collection.
+		// We might modify it but we want to keep iterating over the original collection.
 		Iterator<Object> value = new ArrayList<Object>(collection).iterator();
 		boolean nestedObjectFound = false;
 
 		for (JsonNode jsonNode : array) {
 
 			if (!value.hasNext()) {
-				if (componentType != null) {
-					collection.add(mapper.treeToValue(jsonNode, componentType));
-					continue;
-				}
+
+				Class<?> type = componentType == null ? Object.class : componentType.getType();
+				collection.add(mapper.treeToValue(jsonNode, type));
+
+				continue;
 			}
 
 			Object next = value.next();
 
 			if (ArrayNode.class.isInstance(jsonNode)) {
-				Collection<Object> nestedCollection = asCollection(next);
-				Iterator<Object> iterator = nestedCollection.iterator();
-				return handleArrayNode(array, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper);
+				return handleArray(jsonNode, next, mapper, componentType);
 			}
 
 			if (ObjectNode.class.isInstance(jsonNode)) {
@@ -264,8 +289,8 @@ public class DomainObjectReader {
 			}
 		}
 
+		// there are more items in the collection than contained in the JSON node - remove it.
 		while (value.hasNext()) {
-			//there are more items in the collection than contained in the json node - remove it.
 			collection.remove(value.next());
 		}
 
@@ -280,7 +305,8 @@ public class DomainObjectReader {
 	 * @param mapper must not be {@literal null}.
 	 * @throws Exception
 	 */
-	private void doMergeNestedMap(Map<String, Object> source, ObjectNode node, ObjectMapper mapper) throws Exception {
+	private void doMergeNestedMap(Map<String, Object> source, ObjectNode node, ObjectMapper mapper,
+			TypeInformation<?> type) throws Exception {
 
 		if (source == null) {
 			return;
@@ -295,15 +321,17 @@ public class DomainObjectReader {
 			Object sourceValue = source.get(entry.getKey());
 
 			if (child instanceof ObjectNode && sourceValue != null) {
+
 				doMerge((ObjectNode) child, sourceValue, mapper);
+
 			} else if (child instanceof ArrayNode && sourceValue != null) {
-				Collection<Object> nestedCollection = asCollection(sourceValue);
-				Iterator<Object> iterator = nestedCollection.iterator();
-				handleArrayNode((ArrayNode) child, nestedCollection, iterator.hasNext() ? iterator.next().getClass() : null, mapper);
+
+				handleArray(child, sourceValue, mapper, type);
+
 			} else {
 
-				Class<?> target = sourceValue == null ? Object.class : sourceValue.getClass();
-				source.put(entry.getKey(), mapper.treeToValue(child, target));
+				Class<?> valueType = sourceValue == null ? Object.class : sourceValue.getClass();
+				source.put(entry.getKey(), mapper.treeToValue(child, valueType));
 			}
 
 			fields.remove();

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

@@ -15,13 +15,12 @@
  */
 package org.springframework.data.rest.webmvc.json;
 
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.notNullValue;
-import static org.hamcrest.CoreMatchers.nullValue;
-import static org.hamcrest.CoreMatchers.sameInstance;
-import static org.junit.Assert.assertThat;
-import static org.mockito.Mockito.mock;
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+
+import lombok.AllArgsConstructor;
+import lombok.NoArgsConstructor;
 
 import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
@@ -59,10 +58,6 @@ import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.base.Charsets;
 
-import javassist.runtime.Inner;
-import lombok.AllArgsConstructor;
-import lombok.NoArgsConstructor;
-
 /**
  * Unit tests for {@link DomainObjectReader}.
  * 
@@ -310,40 +305,6 @@ public class DomainObjectReaderUnitTests {
 		assertThat(result.inner, is(sameInstance(inner)));
 	}
 
-	@SuppressWarnings("unchecked")
-	private static <T> T as(Object source, Class<T> type) {
-
-		assertThat(source, is(instanceOf(type)));
-		return (T) source;
-	}
-
-	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
-	static class SampleUser {
-
-		String name;
-		@JsonIgnore String password;
-		Map<String, SampleUser> relatedUsers;
-
-		public SampleUser(String name, String password) {
-			this.name = name;
-			this.password = password;
-		}
-	}
-
-	/**
-	 * @see DATAREST-556
-	 */
-	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
-	static class Person {
-
-		String firstName, lastName;
-
-		public Person(String firstName, String lastName) {
-			this.firstName = firstName;
-			this.lastName = lastName;
-		}
-	}
-
 	/**
 	 * @see DATAREST-937
 	 */
@@ -395,10 +356,8 @@ public class DomainObjectReaderUnitTests {
 		Parent source = new Parent();
 		source.inner = inner;
 
-		JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ " +
-				"{ \"some\" : \"value1\" }," +
-				"{ \"some\" : \"value2\" }," +
-				"{ \"some\" : \"value3\" } ] } }");
+		JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ " + "{ \"some\" : \"value1\" },"
+				+ "{ \"some\" : \"value2\" }," + "{ \"some\" : \"value3\" } ] } }");
 
 		Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper());
 
@@ -431,6 +390,49 @@ public class DomainObjectReaderUnitTests {
 		assertThat(result.inner.items.get(0).some, is("value"));
 	}
 
+	@Test
+	public void testname() throws Exception {
+
+		ObjectMapper mapper = new ObjectMapper();
+		JsonNode node = mapper.readTree("\"asd\"");
+
+		assertThat(mapper.treeToValue(node, Object.class), is((Object) "asd"));
+	}
+
+	@SuppressWarnings("unchecked")
+	private static <T> T as(Object source, Class<T> type) {
+
+		assertThat(source, is(instanceOf(type)));
+		return (T) source;
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class SampleUser {
+
+		String name;
+		@JsonIgnore String password;
+		Map<String, SampleUser> relatedUsers;
+
+		public SampleUser(String name, String password) {
+			this.name = name;
+			this.password = password;
+		}
+	}
+
+	/**
+	 * @see DATAREST-556
+	 */
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Person {
+
+		String firstName, lastName;
+
+		public Person(String firstName, String lastName) {
+			this.firstName = firstName;
+			this.lastName = lastName;
+		}
+	}
+
 	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
 	static class TypeWithGenericMap {