DATAREST-959 - Polishing.

Skip all merge logic if the source value is null. That frees all nested logic from handling with that case and us falling back to plain Jackson reading.

The array handling now also opts out if the source value is not a collection or array in the first place as it means we need to let Jackson override the value with the collection given to be deserialized.

Original pull request: #246.

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

@@ -23,7 +23,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -181,6 +180,10 @@ public class DomainObjectReader {
 			PersistentPropertyAccessor accessor = entity.getPropertyAccessor(target);
 			Object rawValue = accessor.getProperty(property);
 
+			if (rawValue == null) {
+				continue;
+			}
+
 			if (child.isArray()) {
 
 				if (handleArray(child, rawValue, mapper, property.getTypeInformation())) {
@@ -215,7 +218,7 @@ public class DomainObjectReader {
 					continue;
 				}
 
-				if (rawValue != null && property.isEntity()) {
+				if (property.isEntity()) {
 					i.remove();
 					doMerge(objectNode, rawValue, mapper);
 				}
@@ -240,7 +243,12 @@ public class DomainObjectReader {
 	private boolean handleArray(JsonNode node, Object source, ObjectMapper mapper, TypeInformation<?> collectionType)
 			throws Exception {
 
-		Collection<Object> collection = asCollection(source);
+		Collection<Object> collection = ifCollection(source);
+
+		if (collection == null) {
+			return false;
+		}
+
 		Iterator<Object> iterator = collection.iterator();
 		TypeInformation<?> componentType = iterator.hasNext() ? //
 				ClassTypeInformation.from(iterator.next().getClass()) : //
@@ -343,17 +351,16 @@ public class DomainObjectReader {
 	}
 
 	/**
-	 * Returns the given source instance as {@link Collection}.
+	 * Returns the given source instance as {@link Collection} or creates a new one for the given type.
 	 * 
 	 * @param source can be {@literal null}.
+	 * @param type must not be {@literal null} in case {@code source} is null.
 	 * @return
 	 */
 	@SuppressWarnings("unchecked")
-	private static Collection<Object> asCollection(Object source) {
+	private static Collection<Object> ifCollection(Object source) {
 
-		if (source == null) {
-			return new ArrayList<Object>();
-		}
+		Assert.notNull(source, "Source instance must not be null!");
 
 		if (source instanceof Collection) {
 			return (Collection<Object>) source;
@@ -363,7 +370,7 @@ public class DomainObjectReader {
 			return Arrays.asList((Object[]) source);
 		}
 
-		return Collections.singleton(source);
+		return null;
 	}
 
 	/**

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

@@ -26,10 +26,12 @@ import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -394,7 +396,7 @@ public class DomainObjectReaderUnitTests {
 	 * @see DATAREST-959
 	 */
 	@Test
-	public void writesArrayOverUndefinedValueForPut() throws Exception {
+	public void addsElementToPreviouslyEmptyCollection() throws Exception {
 
 		Parent source = new Parent();
 		source.inner = new Child();
@@ -408,6 +410,31 @@ public class DomainObjectReaderUnitTests {
 		assertThat(result.inner.items.get(0).some, is("value"));
 	}
 
+	/**
+	 * @see DATAREST-959
+	 */
+	@Test
+	@SuppressWarnings("unchecked")
+	public void turnsObjectIntoCollection() throws Exception {
+
+		Parent source = new Parent();
+		source.inner = new Child();
+		source.inner.object = new Item("value");
+
+		JsonNode node = new ObjectMapper()
+				.readTree("{ \"inner\" : { \"object\" : [ { \"some\" : \"value\" }, { \"some\" : \"otherValue\" } ] } }");
+
+		Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper());
+		assertThat(result.inner.object, is(instanceOf(Collection.class)));
+
+		Collection<?> collection = (Collection<?>) result.inner.object;
+		assertThat(collection.size(), is(2));
+
+		Iterator<Map<String, Object>> iterator = (Iterator<Map<String, Object>>) collection.iterator();
+		assertThat(iterator.next().get("some"), is((Object) "value"));
+		assertThat(iterator.next().get("some"), is((Object) "otherValue"));
+	}
+
 	@Test
 	public void testname() throws Exception {
 
@@ -515,6 +542,7 @@ public class DomainObjectReaderUnitTests {
 	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
 	static class Child {
 		List<Item> items;
+		Object object;
 	}
 
 	@JsonAutoDetect(fieldVisibility = Visibility.ANY)