DATAREST-605 - Fixed processing of Maps in DomainObjectReader.

We now post-process Map values recursively to make sure they get merged correctly if they're complex objects in turn.

Improved the exception message for invalid payloads for JSON Patch requests to indicate the expected payload as the root Jackson exception does not contain any hints to what's actually expected.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java

@@ -20,7 +20,9 @@ import java.lang.reflect.Field;
 import java.util.List;
 
 import org.springframework.data.rest.webmvc.IncomingRequest;
+import org.springframework.data.rest.webmvc.RestMediaTypes;
 import org.springframework.data.rest.webmvc.json.DomainObjectReader;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 import org.springframework.util.Assert;
 import org.springframework.util.ReflectionUtils;
 
@@ -102,9 +104,7 @@ class JsonPatchHandler {
 
 	<T> T applyPatch(InputStream source, T target) throws Exception {
 
-		CollectionType listOfOperationsType = mapper.getTypeFactory().constructCollectionType(List.class,
-				JsonPatchOperation.class);
-		List<JsonPatchOperation> readValue = mapper.readValue(source, listOfOperationsType);
+		List<JsonPatchOperation> readValue = getPatchOperations(source);
 
 		JsonNode existingAsNode = mapper.readTree(sourceMapper.writeValueAsBytes(target));
 		JsonNode patchedNode = existingAsNode;
@@ -115,8 +115,8 @@ class JsonPatchHandler {
 
 				// Replace remove operation with replace operation and a value of null.
 				JsonPointer path = (JsonPointer) ReflectionUtils.getField(PATH_FIELD, operation);
-				patchedNode = isCollectionElementReference(path) ? operation.apply(patchedNode) : new ReplaceOperation(path,
-						NullNode.getInstance()).apply(patchedNode);
+				patchedNode = isCollectionElementReference(path) ? operation.apply(patchedNode)
+						: new ReplaceOperation(path, NullNode.getInstance()).apply(patchedNode);
 
 			} else {
 				patchedNode = operation.apply(patchedNode);
@@ -134,6 +134,26 @@ class JsonPatchHandler {
 		return reader.readPut(source, existingObject, mapper);
 	}
 
+	/**
+	 * Returns all {@link JsonPatchOperation}s to be applied.
+	 * 
+	 * @param source must not be {@literal null}.
+	 * @return
+	 * @throws HttpMessageNotReadableException in case the payload can't be read.
+	 */
+	private List<JsonPatchOperation> getPatchOperations(InputStream source) {
+
+		CollectionType listOfOperationsType = mapper.getTypeFactory().constructCollectionType(List.class,
+				JsonPatchOperation.class);
+
+		try {
+			return mapper.readValue(source, listOfOperationsType);
+		} catch (Exception o_O) {
+			throw new HttpMessageNotReadableException(
+					String.format("Could not read PATCH operations! Expected %s!", RestMediaTypes.JSON_PATCH_JSON), o_O);
+		}
+	}
+
 	/**
 	 * Returns whether the trailing element of the given {@link JsonPointer} is a pointer into an array or collection.
 	 * 

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/PersistentEntityResourceHandlerMethodArgumentResolver.java

@@ -166,8 +166,8 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
 
 			ObjectMapper mapper = ((MappingJackson2HttpMessageConverter) converter).getObjectMapper();
 
-			return objectToUpdate == null ? read(request, converter, information) : readPutForUpdate(request, mapper,
-					objectToUpdate);
+			return objectToUpdate == null ? read(request, converter, information)
+					: readPutForUpdate(request, mapper, objectToUpdate);
 		}
 
 		// Catch all
@@ -177,9 +177,16 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha
 	private Object readPatch(IncomingRequest request, ObjectMapper mapper, Object existingObject) {
 
 		try {
+
 			JsonPatchHandler handler = new JsonPatchHandler(mapper, reader);
 			return handler.apply(request, existingObject);
+
 		} catch (Exception o_O) {
+
+			if (o_O instanceof HttpMessageNotReadableException) {
+				throw (HttpMessageNotReadableException) o_O;
+			}
+
 			throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, existingObject.getClass()), o_O);
 		}
 	}

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

@@ -184,16 +184,63 @@ public class DomainObjectReader {
 				PersistentPropertyAccessor accessor = entity.getPropertyAccessor(target);
 				Object nested = accessor.getProperty(property);
 
-				if (nested != null && property.isEntity()) {
-					doMerge((ObjectNode) child, nested, mapper);
+				ObjectNode objectNode = (ObjectNode) child;
+
+				if (property.isMap()) {
+
+					// Keep empty Map to wipe it as expected
+					if (!objectNode.fieldNames().hasNext()) {
+						continue;
+					}
+
+					doMergeNestedMap((Map<String, Object>) nested, objectNode, mapper);
+
+					// Remove potentially emptied Map as values have been handled recursively
+					if (!objectNode.fieldNames().hasNext()) {
+						i.remove();
+					}
+
+					continue;
 				}
 
+				if (nested != null && property.isEntity()) {
+					doMerge(objectNode, nested, mapper);
+				}
 			}
 		}
 
 		return mapper.readerForUpdating(target).readValue(root);
 	}
 
+	/**
+	 * Merges nested {@link Map} values for the given source {@link Map}, the {@link ObjectNode} and {@link ObjectMapper}.
+	 * 
+	 * @param source can be {@literal null}.
+	 * @param node must not be {@literal null}.
+	 * @param mapper must not be {@literal null}.
+	 * @throws Exception
+	 */
+	private void doMergeNestedMap(Map<String, Object> source, ObjectNode node, ObjectMapper mapper) throws Exception {
+
+		if (source == null) {
+			return;
+		}
+
+		Iterator<Entry<String, JsonNode>> fields = node.fields();
+
+		while (fields.hasNext()) {
+
+			Entry<String, JsonNode> entry = fields.next();
+			JsonNode child = entry.getValue();
+			Object sourceValue = source.get(entry.getKey());
+
+			if (child instanceof ObjectNode && sourceValue != null) {
+				doMerge((ObjectNode) child, sourceValue, mapper);
+				fields.remove();
+			}
+		}
+	}
+
 	/**
 	 * Returns the {@link MappedProperties} for the given {@link PersistentEntity}.
 	 * 

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java

@@ -22,16 +22,20 @@ import static org.springframework.data.rest.webmvc.util.TestUtils.*;
 import java.util.Arrays;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.springframework.data.mapping.context.PersistentEntities;
 import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
 import org.springframework.data.rest.core.mapping.ResourceMappings;
+import org.springframework.data.rest.webmvc.RestMediaTypes;
 import org.springframework.data.rest.webmvc.json.DomainObjectReader;
 import org.springframework.data.rest.webmvc.mongodb.Address;
 import org.springframework.data.rest.webmvc.mongodb.User;
+import org.springframework.http.converter.HttpMessageNotReadableException;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 
@@ -47,6 +51,7 @@ public class JsonPatchHandlerUnitTests {
 	User user;
 
 	@Mock ResourceMappings mappings;
+	public @Rule ExpectedException exception = ExpectedException.none();
 
 	@Before
 	public void setUp() {
@@ -118,4 +123,16 @@ public class JsonPatchHandlerUnitTests {
 		assertThat(user.colleagues, hasSize(1));
 		assertThat(user.colleagues.get(0).firstname, is(christoph.firstname));
 	}
+
+	/**
+	 * @see DATAREST-609
+	 */
+	@Test
+	public void hintsToMediaTypeIfBodyCantBeRead() throws Exception {
+
+		exception.expect(HttpMessageNotReadableException.class);
+		exception.expectMessage(RestMediaTypes.JSON_PATCH_JSON.toString());
+
+		handler.applyPatch(asStream("{ \"foo\" : \"bar\" }"), new User());
+	}
 }

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

@@ -19,6 +19,7 @@ import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
 import java.util.Collections;
+import java.util.Map;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -94,11 +95,31 @@ public class DomainObjectReaderUnitTests {
 		assertThat(result.lastName, is("Beauford"));
 	}
 
+	/**
+	 * @see DATAREST-605
+	 */
+	@Test
+	public void mergesMapCorrectly() throws Exception {
+
+		SampleUser user = new SampleUser("firstname", "password");
+		user.relatedUsers = Collections.singletonMap("parent", new SampleUser("firstname", "password"));
+
+		JsonNode node = new ObjectMapper()
+				.readTree("{ \"relatedUsers\" : { \"parent\" : { \"password\" : \"sneeky\", \"name\" : \"Oliver\" } } }");
+
+		SampleUser result = reader.readPut((ObjectNode) node, user, new ObjectMapper());
+
+		// Assert that the nested Map values also consider ignored properties
+		assertThat(result.relatedUsers.get("parent").password, is("password"));
+		assertThat(result.relatedUsers.get("parent").name, is("Oliver"));
+	}
+
 	@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;