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 c79603fe0..176ff8665 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 @@ -128,4 +128,8 @@ class JsonPatchHandler { T applyMergePatch(InputStream source, T existingObject) throws Exception { return reader.read(source, existingObject, mapper); } + + T applyPut(ObjectNode source, T existingObject) { + return reader.readPut(source, existingObject, mapper); + } } 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 5247a8b79..6ffb8156b 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 @@ -41,7 +41,9 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.ModelAndViewContainer; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; /** * Custom {@link HandlerMethodArgumentResolver} to create {@link PersistentEntityResource} instances. @@ -136,35 +138,39 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha * Reads the given {@link ServerHttpRequest} into an object of the type of the given {@link RootResourceInformation}, * potentially applying the content to an object of the given id. * - * @param information - * @param request - * @param converter - * @param id + * @param information must not be {@literal null}. + * @param request must not be {@literal null}. + * @param converter must not be {@literal null}. + * @param id must not be {@literal null}. * @return - * @throws IOException */ private Object read(RootResourceInformation information, IncomingRequest request, HttpMessageConverter converter, Serializable id) { + Object objectToUpdate = getObjectToUpdate(id, information); + + // JSON + PATCH request if (request.isPatchRequest() && converter instanceof MappingJackson2HttpMessageConverter) { - if (id == null) { + if (objectToUpdate == null) { new ResourceNotFoundException(); } - RepositoryInvoker invoker = information.getInvoker(); - Object existingObject = invoker.invokeFindOne(id); - - if (existingObject == null) { - throw new ResourceNotFoundException(); - } - ObjectMapper mapper = ((MappingJackson2HttpMessageConverter) converter).getObjectMapper(); - Object result = readPatch(request, mapper, existingObject); + Object result = readPatch(request, mapper, objectToUpdate); return result; + + // JSON + PUT request + } else if (converter instanceof MappingJackson2HttpMessageConverter) { + + ObjectMapper mapper = ((MappingJackson2HttpMessageConverter) converter).getObjectMapper(); + + return objectToUpdate == null ? read(request, converter, information) : readPutForUpdate(request, mapper, + objectToUpdate); } + // Catch all return read(request, converter, information); } @@ -178,6 +184,20 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha } } + private Object readPutForUpdate(IncomingRequest request, ObjectMapper mapper, Object existingObject) { + + try { + + JsonPatchHandler handler = new JsonPatchHandler(mapper, reader); + JsonNode jsonNode = mapper.readTree(request.getBody()); + + return handler.applyPut((ObjectNode) jsonNode, existingObject); + + } catch (Exception o_O) { + throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, existingObject.getClass()), o_O); + } + } + private Object read(IncomingRequest request, HttpMessageConverter converter, RootResourceInformation information) { @@ -187,4 +207,21 @@ public class PersistentEntityResourceHandlerMethodArgumentResolver implements Ha throw new HttpMessageNotReadableException(String.format(ERROR_MESSAGE, information.getDomainType()), o_O); } } + + /** + * Returns the object to be updated identified by the given id using the given {@link RootResourceInformation}. + * + * @param id can be {@literal null}. + * @param information must not be {@literal null}. + * @return + */ + private static Object getObjectToUpdate(Serializable id, RootResourceInformation information) { + + if (id == null) { + return null; + } + + RepositoryInvoker invoker = information.getInvoker(); + return invoker.invokeFindOne(id); + } } 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 18803bd29..a449d1719 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 @@ -16,12 +16,16 @@ package org.springframework.data.rest.webmvc.json; import java.io.InputStream; +import java.util.Collection; +import java.util.HashSet; import java.util.Iterator; import java.util.Map.Entry; +import java.util.Set; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.SimplePropertyHandler; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.rest.core.mapping.ResourceMappings; import org.springframework.data.rest.webmvc.mapping.AssociationLinks; @@ -31,7 +35,6 @@ import org.springframework.util.Assert; import com.fasterxml.jackson.databind.BeanDescription; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.introspect.BasicClassIntrospector; import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; import com.fasterxml.jackson.databind.introspect.ClassIntrospector; @@ -82,9 +85,56 @@ public class DomainObjectReader { Assert.notNull(mapper, "ObjectMapper must not be null!"); try { - return merge((ObjectNode) mapper.readTree(source), target, mapper); - } catch (Exception e) { - throw new HttpMessageNotReadableException("Could not read payload!", e); + return doMerge((ObjectNode) mapper.readTree(source), target, mapper); + } catch (Exception o_O) { + throw new HttpMessageNotReadableException("Could not read payload!", o_O); + } + } + + /** + * Reads the given source node onto the given target object and applies PUT semantics, i.e. explicitly + * + * @param source must not be {@literal null}. + * @param target must not be {@literal null}. + * @param mapper + * @return + */ + public T readPut(final ObjectNode source, T target, final ObjectMapper mapper) { + + Assert.notNull(source, "ObjectNode must not be null!"); + Assert.notNull(target, "Existing object instance must not be null!"); + Assert.notNull(mapper, "ObjectMapper must not be null!"); + + final PersistentEntity entity = entities.getPersistentEntity(target.getClass()); + final Collection properties = getJacksonProperties(entity, mapper); + + entity.doWithProperties(new SimplePropertyHandler() { + + /* + * (non-Javadoc) + * @see org.springframework.data.mapping.SimplePropertyHandler#doWithPersistentProperty(org.springframework.data.mapping.PersistentProperty) + */ + @Override + public void doWithPersistentProperty(PersistentProperty property) { + + boolean isMappedProperty = properties.contains(property.getName()); + boolean noValueInSource = !source.has(property.getName()); + + if (isMappedProperty && noValueInSource) { + source.putNull(property.getName()); + } + } + }); + + return merge(source, target, mapper); + } + + public T merge(ObjectNode source, T target, ObjectMapper mapper) { + + try { + return doMerge(source, target, mapper); + } catch (Exception o_O) { + throw new HttpMessageNotReadableException("Could not read payload!", o_O); } } @@ -92,79 +142,73 @@ public class DomainObjectReader { * Merges the given {@link ObjectNode} onto the given object. * * @param root must not be {@literal null}. - * @param existingObject - * @param mapper + * @param target must not be {@literal null}. + * @param mapper must not be {@literal null}. * @return * @throws Exception */ - public T merge(ObjectNode root, T existingObject, ObjectMapper mapper) throws Exception { + private T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exception { Assert.notNull(root, "Root ObjectNode must not be null!"); - Assert.notNull(existingObject, "Existing object instance must not be null!"); + Assert.notNull(target, "Target object instance must not be null!"); Assert.notNull(mapper, "ObjectMapper must not be null!"); + PersistentEntity entity = entities.getPersistentEntity(target.getClass()); + Collection mappedProperties = getJacksonProperties(entity, mapper); + for (Iterator> i = root.fields(); i.hasNext();) { Entry entry = i.next(); JsonNode child = entry.getValue(); if (child.isArray()) { - // We ignore arrays so they get instantiated fresh every time - } else if (child.isObject()) { + continue; + } - PersistentProperty property = findProperty(existingObject, entry.getKey(), mapper); + PersistentProperty property = entity.getPersistentProperty(entry.getKey()); - if (property == null || associationLinks.isLinkableAssociation(property)) { + if (property == null || !mappedProperties.contains(property.getName())) { + i.remove(); + continue; + } + + if (child.isObject()) { + + if (associationLinks.isLinkableAssociation(property)) { continue; } - PersistentEntity entity = entities.getPersistentEntity(existingObject.getClass()); - PersistentPropertyAccessor accessor = entity.getPropertyAccessor(existingObject); + PersistentPropertyAccessor accessor = entity.getPropertyAccessor(target); Object nested = accessor.getProperty(property); if (nested != null) { - - // Only remove the JsonNode if the object already exists. Otherwise it will be instantiated when the parent - // gets deserialized. - - i.remove(); - merge((ObjectNode) child, nested, mapper); + doMerge((ObjectNode) child, nested, mapper); } + } } - ObjectReader jsonReader = mapper.readerForUpdating(existingObject); - jsonReader.readValue(root); - - return existingObject; + return mapper.readerForUpdating(target).readValue(root); } /** - * Finds the {@link PersistentProperty} for the JSON field of the given name on the given object. + * Returns the names of all mapped properties for the given {@link PersistentEntity}. * - * @param object must not be {@literal null}. - * @param fieldName must not be {@literal null} or empty. + * @param entity must not be {@literal null}. * @param mapper must not be {@literal null}. - * @return the {@link PersistentProperty} for the JSON field of the given name on the given object or {@literal null} - * if either the given source object is no persistent entity or the property cannot be found. + * @return the collection of mapped properties. */ - private PersistentProperty findProperty(Object object, String fieldName, ObjectMapper mapper) { - - PersistentEntity entity = entities.getPersistentEntity(object.getClass()); - - if (entity == null) { - return null; - } + private Collection getJacksonProperties(PersistentEntity entity, ObjectMapper mapper) { BeanDescription description = introspector.forDeserialization(mapper.getDeserializationConfig(), - mapper.constructType(object.getClass()), mapper.getDeserializationConfig()); + mapper.constructType(entity.getType()), mapper.getDeserializationConfig()); - for (BeanPropertyDefinition definition : description.findProperties()) { - if (definition.getName().equals(fieldName)) { - return entity.getPersistentProperty(definition.getInternalName()); - } + Set properties = new HashSet(); + + for (BeanPropertyDefinition property : description.findProperties()) { + properties.add(property.getInternalName()); } - return null; + return properties; } } 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 new file mode 100644 index 000000000..56753ef05 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/DomainObjectReaderUnitTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2015 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.rest.webmvc.json; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import java.util.Collections; + +import org.junit.Before; +import org.junit.Test; +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 com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; + +/** + * Unit tests for {@link DomainObjectReader}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class DomainObjectReaderUnitTests { + + @Mock ResourceMappings mappings; + + DomainObjectReader reader; + + @Before + public void setUp() { + + MongoMappingContext mappingContext = new MongoMappingContext(); + mappingContext.setInitialEntitySet(Collections.singleton(SampleUser.class)); + mappingContext.afterPropertiesSet(); + + PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext)); + + this.reader = new DomainObjectReader(entities, mappings); + } + + /** + * @see DATAREST-461 + */ + @Test + public void doesNotConsiderIgnoredProperties() throws Exception { + + SampleUser user = new SampleUser("firstname", "password"); + JsonNode node = new ObjectMapper().readTree("{}"); + + SampleUser result = reader.readPut((ObjectNode) node, user, new ObjectMapper()); + + assertThat(result.name, is(nullValue())); + assertThat(result.password, is("password")); + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class SampleUser { + + String name; + @JsonIgnore String password; + + public SampleUser(String name, String password) { + this.name = name; + this.password = password; + } + } +}