DATAREST-461 - Ignore unmapped properties when applying PUT requests.

We now make sure that unmapped properties (e.g. by using @JsonIgnore) are ignored when applying PUT request payloads. To do so we now basically use the merging algorithm used for PATCH requests but make sure we fill up the request payload with explicit null values for all mapped but not contained properties.

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

@@ -128,4 +128,8 @@ class JsonPatchHandler {
 	<T> T applyMergePatch(InputStream source, T existingObject) throws Exception {
 		return reader.read(source, existingObject, mapper);
 	}
+
+	<T> T applyPut(ObjectNode source, T existingObject) {
+		return reader.readPut(source, existingObject, mapper);
+	}
 }

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<Object> 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<Object> 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);
+	}
 }

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> 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<String> 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> 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> T merge(ObjectNode root, T existingObject, ObjectMapper mapper) throws Exception {
+	private <T> 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<String> mappedProperties = getJacksonProperties(entity, mapper);
+
 		for (Iterator<Entry<String, JsonNode>> i = root.fields(); i.hasNext();) {
 
 			Entry<String, JsonNode> 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<String> 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<String> properties = new HashSet<String>();
+
+		for (BeanPropertyDefinition property : description.findProperties()) {
+			properties.add(property.getInternalName());
 		}
 
-		return null;
+		return properties;
 	}
 }

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;
+		}
+	}
+}