diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java index 9182abeae..8b847eea8 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/JsonPatchHandler.java +++ b/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 applyPatch(InputStream source, T target) throws Exception { - CollectionType listOfOperationsType = mapper.getTypeFactory().constructCollectionType(List.class, - JsonPatchOperation.class); - List readValue = mapper.readValue(source, listOfOperationsType); + List 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 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. * diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/PersistentEntityResourceHandlerMethodArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/PersistentEntityResourceHandlerMethodArgumentResolver.java index 6ffb8156b..8605e709d 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/PersistentEntityResourceHandlerMethodArgumentResolver.java +++ b/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); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java index ba3eec964..44b1b1bde 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/DomainObjectReader.java +++ b/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) 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 source, ObjectNode node, ObjectMapper mapper) throws Exception { + + if (source == null) { + return; + } + + Iterator> fields = node.fields(); + + while (fields.hasNext()) { + + Entry 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}. * diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java index b6ebf7f0d..0fab114e8 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/config/JsonPatchHandlerUnitTests.java +++ b/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()); + } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java index b2c0ddf4d..a66c129a7 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java +++ b/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 relatedUsers; public SampleUser(String name, String password) { this.name = name;