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 4382ca9f2..aafc19d6a 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 @@ -22,20 +22,28 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import org.springframework.beans.PropertyAccessor; +import org.springframework.beans.PropertyAccessorFactory; +import org.springframework.core.CollectionFactory; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.support.DefaultConversionService; 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.mapping.model.ConvertingPropertyAccessor; import org.springframework.data.rest.webmvc.mapping.Associations; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -88,6 +96,7 @@ public class DomainObjectReader { * @param mapper * @return */ + @SuppressWarnings("unchecked") public T readPut(final ObjectNode source, T target, final ObjectMapper mapper) { Assert.notNull(source, "ObjectNode must not be null!"); @@ -100,8 +109,50 @@ public class DomainObjectReader { Assert.notNull(entity, "No PersistentEntity found for ".concat(type.getName()).concat("!")); + try { + + Object intermediate = mapper.readerFor(target.getClass()).readValue(source); + return (T) mergeForPut(intermediate, target, mapper); + + } catch (Exception o_O) { + throw new HttpMessageNotReadableException("Could not read payload!", o_O); + } + } + + /** + * Merges the state of given source object onto the target one preserving PUT semantics. + * + * @param source can be {@literal null}. + * @param target can be {@literal null}. + * @param mapper must not be {@literal null}. + * @return + */ + private T mergeForPut(T source, T target, final ObjectMapper mapper) { + + Assert.notNull(mapper, "ObjectMapper must not be null!"); + + if (target == null || source == null) { + return source; + } + + Class type = target.getClass(); + + final PersistentEntity entity = entities.getPersistentEntity(type); + + if (entity == null) { + return source; + } + + Assert.notNull(entity, "No PersistentEntity found for ".concat(type.getName()).concat("!")); + final MappedProperties properties = MappedProperties.fromJacksonProperties(entity, mapper); + ConversionService conversionService = new DefaultConversionService(); + final PersistentPropertyAccessor targetAccessor = entity.getPropertyAccessor(target); + final ConvertingPropertyAccessor convertingAccessor = new ConvertingPropertyAccessor(targetAccessor, + conversionService); + final PersistentPropertyAccessor sourceAccessor = entity.getPropertyAccessor(source); + entity.doWithProperties(new SimplePropertyHandler() { /* @@ -115,18 +166,50 @@ public class DomainObjectReader { return; } - String mappedName = properties.getMappedName(property); - - boolean isMappedProperty = mappedName != null; - boolean noValueInSource = !source.has(mappedName); - - if (isMappedProperty && noValueInSource) { - source.putNull(mappedName); + if (!properties.isMappedProperty(property)) { + return; } + + Object sourceValue = sourceAccessor.getProperty(property); + Object targetValue = targetAccessor.getProperty(property); + Object result = null; + + if (property.isMap()) { + result = mergeMaps(property, sourceValue, targetValue, mapper); + } else if (property.isCollectionLike()) { + result = mergeCollections(property, sourceValue, targetValue, mapper); + } else if (property.isEntity()) { + result = mergeForPut(sourceValue, targetValue, mapper); + } else { + result = sourceValue; + } + + convertingAccessor.setProperty(property, result); } }); - return merge(source, target, mapper); + // Need to copy unmapped properties as the PersistentProperty model currently does not contain any transient + // properties + copyRemainingProperties(properties, source, target); + + return target; + } + + /** + * Copies the unmapped properties of the given {@link MappedProperties} from the source object to the target instance. + * + * @param properties must not be {@literal null}. + * @param source must not be {@literal null}. + * @param target must not be {@literal null}. + */ + private static void copyRemainingProperties(MappedProperties properties, Object source, Object target) { + + PropertyAccessor sourceAccessor = PropertyAccessorFactory.forDirectFieldAccess(source); + PropertyAccessor targetAccessor = PropertyAccessorFactory.forDirectFieldAccess(target); + + for (String property : properties.getSpringDataUnmappedProperties()) { + targetAccessor.setPropertyValue(property, sourceAccessor.getPropertyValue(property)); + } } public T merge(ObjectNode source, T target, ObjectMapper mapper) { @@ -148,7 +231,7 @@ public class DomainObjectReader { * @throws Exception */ @SuppressWarnings("unchecked") - private T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exception { + T doMerge(ObjectNode root, T target, ObjectMapper mapper) throws Exception { Assert.notNull(root, "Root ObjectNode must not be null!"); Assert.notNull(target, "Target object instance must not be null!"); @@ -351,6 +434,72 @@ public class DomainObjectReader { } } + @SuppressWarnings("unchecked") + private Map mergeMaps(PersistentProperty property, Object source, Object target, + ObjectMapper mapper) { + + Map sourceMap = (Map) source; + + if (sourceMap == null) { + return null; + } + + Map targetMap = (Map) target; + Map result = targetMap == null ? CollectionFactory.createMap(Map.class, sourceMap.size()) + : CollectionFactory.createApproximateMap(targetMap, sourceMap.size()); + + for (Entry entry : sourceMap.entrySet()) { + + Object targetValue = targetMap == null ? null : targetMap.get(entry.getKey()); + result.put(entry.getKey(), mergeForPut(entry.getValue(), targetValue, mapper)); + } + + return result; + } + + private Collection mergeCollections(PersistentProperty property, Object source, Object target, + ObjectMapper mapper) { + + Collection sourceCollection = asCollection(source); + + if (sourceCollection == null) { + return null; + } + + Collection targetCollection = asCollection(target); + Collection result = targetCollection == null + ? CollectionFactory.createCollection(Collection.class, sourceCollection.size()) + : CollectionFactory.createApproximateCollection(targetCollection, sourceCollection.size()); + + Iterator sourceIterator = sourceCollection.iterator(); + Iterator targetIterator = targetCollection == null ? Collections.emptyIterator() + : targetCollection.iterator(); + + while (sourceIterator.hasNext()) { + + Object sourceElement = sourceIterator.next(); + Object targetElement = targetIterator.hasNext() ? targetIterator.next() : null; + + result.add(mergeForPut(sourceElement, targetElement, mapper)); + } + + return result; + } + + @SuppressWarnings("unchecked") + private static Collection asCollection(Object source) { + + if (source == null) { + return null; + } else if (source instanceof Collection) { + return (Collection) source; + } else if (source.getClass().isArray()) { + return Arrays.asList(ObjectUtils.toObjectArray(source)); + } else { + return Collections.singleton(source); + } + } + /** * Returns the given source instance as {@link Collection} or creates a new one for the given type. * diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java index 4b31ee499..3492eb38d 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java @@ -15,8 +15,13 @@ */ package org.springframework.data.rest.webmvc.json; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -38,8 +43,9 @@ class MappedProperties { private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector(); - private final Map, String> propertyToFieldName; + private final Map, BeanPropertyDefinition> propertyToFieldName; private final Map> fieldNameToProperty; + private final Set unmappedProperties; /** * Creates a new {@link MappedProperties} instance for the given {@link PersistentEntity} and {@link BeanDescription}. @@ -52,16 +58,19 @@ class MappedProperties { Assert.notNull(entity, "Entity must not be null!"); Assert.notNull(description, "BeanDescription must not be null!"); - this.propertyToFieldName = new HashMap, String>(); + this.propertyToFieldName = new HashMap, BeanPropertyDefinition>(); this.fieldNameToProperty = new HashMap>(); + this.unmappedProperties = new HashSet(); for (BeanPropertyDefinition property : description.findProperties()) { PersistentProperty persistentProperty = entity.getPersistentProperty(property.getInternalName()); if (persistentProperty != null) { - propertyToFieldName.put(persistentProperty, property.getName()); + propertyToFieldName.put(persistentProperty, property); fieldNameToProperty.put(property.getName(), persistentProperty); + } else { + unmappedProperties.add(property); } } } @@ -89,7 +98,7 @@ class MappedProperties { Assert.notNull(property, "PersistentProperty must not be null!"); - return propertyToFieldName.get(property); + return propertyToFieldName.get(property).getName(); } /** @@ -113,4 +122,37 @@ class MappedProperties { return fieldNameToProperty.get(fieldName); } + + /** + * Returns all properties only known to Jackson. + * + * @return the names of all properties that are not known to Spring Data but appear in the Jackson metamodel. + */ + public Iterable getSpringDataUnmappedProperties() { + + if (unmappedProperties.isEmpty()) { + return Collections.emptySet(); + } + + List result = new ArrayList(unmappedProperties.size()); + + for (BeanPropertyDefinition definitions : unmappedProperties) { + result.add(definitions.getInternalName()); + } + + return result; + } + + /** + * Returns whether the given {@link PersistentProperty} is mapped, i.e. known to both Jackson and Spring Data. + * + * @param property must not be {@literal null}. + * @return + */ + public boolean isMappedProperty(PersistentProperty property) { + + Assert.notNull(property, "PersistentProperty must not be null!"); + + return propertyToFieldName.containsKey(property); + } } 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 aa1cc8c36..29bc7135b 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 @@ -68,6 +68,7 @@ import com.google.common.base.Charsets; * @author Oliver Gierke * @author Craig Andrews * @author Mathias Düsterhöft + * @author Ken Dombeck */ @RunWith(MockitoJUnitRunner.class) public class DomainObjectReaderUnitTests { @@ -271,7 +272,7 @@ public class DomainObjectReaderUnitTests { JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"name\" : \"new inner name\" } }"); - Outer result = reader.merge((ObjectNode) node, outer, new ObjectMapper()); + Outer result = reader.doMerge((ObjectNode) node, outer, new ObjectMapper()); assertThat(result, is(sameInstance(outer))); assertThat(result.prop, is("else")); @@ -390,13 +391,41 @@ public class DomainObjectReaderUnitTests { assertThat(iterator.next().get("some"), is((Object) "otherValue")); } - @Test - public void testname() throws Exception { + @Test // DATAREST-965 + public void writesObjectWithRemovedItemsForPut() throws Exception { - ObjectMapper mapper = new ObjectMapper(); - JsonNode node = mapper.readTree("\"asd\""); + Child inner = new Child(); + inner.items = new ArrayList(); + inner.items.add(new Item("test1")); + inner.items.add(new Item("test2")); - assertThat(mapper.treeToValue(node, Object.class), is((Object) "asd")); + Parent source = new Parent(); + source.inner = inner; + + JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"object\" : \"value\" } }"); + + Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); + + assertThat(result.inner.items, is(nullValue())); + assertThat((String) result.inner.object, is("value")); + } + + @Test // DATAREST-965 + public void writesArrayWithRemovedObjectForPut() throws Exception { + + Child inner = new Child(); + inner.object = "value"; + + Parent source = new Parent(); + source.inner = inner; + + JsonNode node = new ObjectMapper().readTree("{ \"inner\" : { \"items\" : [ { \"some\" : \"value\" } ] } }"); + + Parent result = reader.readPut((ObjectNode) node, source, new ObjectMapper()); + + assertThat(result.inner.items.size(), is(1)); + assertThat(result.inner.items.get(0).some, is("value")); + assertThat(result.inner.object, is(nullValue())); } @Test // DATAREST-986 @@ -427,9 +456,12 @@ public class DomainObjectReaderUnitTests { Map relatedUsers; public SampleUser(String name, String password) { + this.name = name; this.password = password; } + + protected SampleUser() {} } // DATAREST-556 @@ -442,6 +474,8 @@ public class DomainObjectReaderUnitTests { this.firstName = firstName; this.lastName = lastName; } + + protected Person() {} } @JsonAutoDetect(fieldVisibility = Visibility.ANY)