Spring/spring-data-rest
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAREST-944 - DomainObjectMerger now also merges associations for PUT.

Browse Source 
We now explicitly merge associations skipping the linkable ones. We also try to reuse the existing collections and maps if they're mutable falling back to a completely new one if not.

Extracted PropertyHandler and AssociationHandler implementations.
This commit is contained in:
Oliver Gierke
2017-01-26 07:39:54 +01:00
parent 5494f1f6a5
commit fe75811812
2 changed files with 172 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -15,6 +15,7 @@
*/
package org.springframework.data.rest.webmvc.json;
import lombok.Getter;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
@@ -30,11 +31,12 @@ 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.Association;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.SimpleAssociationHandler;
import org.springframework.data.mapping.SimplePropertyHandler;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
@@ -127,7 +129,7 @@ public class DomainObjectReader {
* @param mapper must not be {@literal null}.
* @return
*/
private <T> T mergeForPut(T source, T target, final ObjectMapper mapper) {
<T> T mergeForPut(T source, T target, final ObjectMapper mapper) {
Assert.notNull(mapper, "ObjectMapper must not be null!");
@@ -137,7 +139,7 @@ public class DomainObjectReader {
Class<? extends Object> type = target.getClass();
final PersistentEntity<?, ?> entity = entities.getPersistentEntity(type);
PersistentEntity<?, ?> entity = entities.getPersistentEntity(type);
if (entity == null) {
return source;
@@ -145,52 +147,14 @@ public class DomainObjectReader {
Assert.notNull(entity, "No PersistentEntity found for ".concat(type.getName()).concat("!"));
final MappedProperties properties = MappedProperties.fromJacksonProperties(entity, mapper);
MergingPropertyHandler propertyHandler = new MergingPropertyHandler(source, target, 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() {
/*
* (non-Javadoc)
* @see org.springframework.data.mapping.SimplePropertyHandler#doWithPersistentProperty(org.springframework.data.mapping.PersistentProperty)
*/
@Override
public void doWithPersistentProperty(PersistentProperty<?> property) {
if (property.isIdProperty() || property.isVersionProperty() || !property.isWritable()) {
return;
}
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);
}
});
entity.doWithProperties(propertyHandler);
entity.doWithAssociations(new LinkedAssociationSkippingAssociationHandler(associationLinks, propertyHandler));
// Need to copy unmapped properties as the PersistentProperty model currently does not contain any transient
// properties
copyRemainingProperties(properties, source, target);
copyRemainingProperties(propertyHandler.getProperties(), source, target);
return target;
}
@@ -460,7 +424,20 @@ public class DomainObjectReader {
result.put(entry.getKey(), mergeForPut(entry.getValue(), targetValue, mapper));
}
return result;
if (targetMap == null) {
return result;
}
try {
targetMap.clear();
targetMap.putAll(result);
return targetMap;
} catch (UnsupportedOperationException o_O) {
return result;
}
}
private Collection<Object> mergeCollections(PersistentProperty<?> property, Object source, Object target,
@@ -489,7 +466,20 @@ public class DomainObjectReader {
result.add(mergeForPut(sourceElement, targetElement, mapper));
}
return result;
if (targetCollection == null) {
return result;
}
try {
targetCollection.clear();
targetCollection.addAll(result);
return targetCollection;
} catch (UnsupportedOperationException o_O) {
return result;
}
}
@SuppressWarnings("unchecked")
@@ -574,4 +564,99 @@ public class DomainObjectReader {
return value.getClass().equals(type.getType()) ? type : ClassTypeInformation.from(value.getClass());
}
/**
* {@link SimpleAssociationHandler} that skips linkable associations and forwards handling for all other ones to the
* delegate {@link SimplePropertyHandler}.
*
* @author Oliver Gierke
*/
@RequiredArgsConstructor
private final class LinkedAssociationSkippingAssociationHandler implements SimpleAssociationHandler {
private final @NonNull Associations associations;
private final @NonNull SimplePropertyHandler delegate;
/*
* (non-Javadoc)
* @see org.springframework.data.mapping.SimpleAssociationHandler#doWithAssociation(org.springframework.data.mapping.Association)
*/
@Override
public void doWithAssociation(Association<? extends PersistentProperty<?>> association) {
if (associationLinks.isLinkableAssociation(association)) {
return;
}
delegate.doWithPersistentProperty(association.getInverse());
}
}
/**
* {@link SimplePropertyHandler} to merge the states of the given objects.
*
* @author Oliver Gierke
*/
private class MergingPropertyHandler implements SimplePropertyHandler {
private final @Getter MappedProperties properties;
private final PersistentPropertyAccessor targetAccessor;
private final PersistentPropertyAccessor sourceAccessor;
private final ObjectMapper mapper;
/**
* Creates a new {@link MergingPropertyHandler} for the given source, target, {@link PersistentEntity} and
* {@link ObjectMapper}.
*
* @param source must not be {@literal null}.
* @param target must not be {@literal null}.
* @param entity must not be {@literal null}.
* @param mapper must not be {@literal null}.
*/
public MergingPropertyHandler(Object source, Object target, PersistentEntity<?, ?> entity, ObjectMapper mapper) {
Assert.notNull(source, "Source instance must not be null!");
Assert.notNull(target, "Target instance must not be null!");
Assert.notNull(entity, "PersistentEntity must not be null!");
Assert.notNull(mapper, "ObjectMapper must not be null!");
this.properties = MappedProperties.fromJacksonProperties(entity, mapper);
this.targetAccessor = new ConvertingPropertyAccessor(entity.getPropertyAccessor(target),
new DefaultConversionService());
this.sourceAccessor = entity.getPropertyAccessor(source);
this.mapper = mapper;
}
/*
* (non-Javadoc)
* @see org.springframework.data.mapping.SimplePropertyHandler#doWithPersistentProperty(org.springframework.data.mapping.PersistentProperty)
*/
@Override
public void doWithPersistentProperty(PersistentProperty<?> property) {
if (property.isIdProperty() || property.isVersionProperty() || !property.isWritable()) {
return;
}
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;
}
targetAccessor.setProperty(property, result);
}
}
}

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

@@ -22,6 +22,7 @@ import static org.mockito.Mockito.*;
import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import lombok.Value;
import java.io.ByteArrayInputStream;
import java.util.ArrayList;
@@ -45,6 +46,7 @@ import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.data.annotation.CreatedDate;
import org.springframework.data.annotation.Id;
import org.springframework.data.annotation.ReadOnlyProperty;
import org.springframework.data.annotation.Reference;
import org.springframework.data.annotation.Transient;
import org.springframework.data.annotation.Version;
import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
@@ -94,6 +96,7 @@ public class DomainObjectReaderUnitTests {
mappingContext.getPersistentEntity(Product.class);
mappingContext.getPersistentEntity(TransientReadOnlyProperty.class);
mappingContext.getPersistentEntity(CollectionOfEnumWithMethods.class);
mappingContext.getPersistentEntity(SampleWithReference.class);
mappingContext.afterPropertiesSet();
PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext));
@@ -466,6 +469,33 @@ public class DomainObjectReaderUnitTests {
assertThat(result.enums, contains(SampleEnum.SECOND, SampleEnum.FIRST));
}
@Test // DATAREST-944
public void mergesAssociations() {
List<Nested> originalCollection = Arrays.asList(new Nested(2, 3));
SampleWithReference source = new SampleWithReference(Arrays.asList(new Nested(1, 2), new Nested(2, 3)));
SampleWithReference target = new SampleWithReference(originalCollection);
SampleWithReference result = reader.mergeForPut(source, target, new ObjectMapper());
assertThat(result.nested, is(source.nested));
assertThat(result.nested == originalCollection, is(false));
}
@Test // DATAREST-944
public void mergesAssociationsAndKeepsMutableCollection() {
ArrayList<Nested> originalCollection = new ArrayList<Nested>(Arrays.asList(new Nested(2, 3)));
SampleWithReference source = new SampleWithReference(
new ArrayList<Nested>(Arrays.asList(new Nested(1, 2), new Nested(2, 3))));
SampleWithReference target = new SampleWithReference(originalCollection);
SampleWithReference result = reader.mergeForPut(source, target, new ObjectMapper());
assertThat(result.nested, is(source.nested));
assertThat(result.nested == originalCollection, is(true));
}
@SuppressWarnings("unchecked")
private static <T> T as(Object source, Class<T> type) {
@@ -629,4 +659,14 @@ public class DomainObjectReaderUnitTests {
static class CollectionOfEnumWithMethods {
List<SampleEnum> enums = new ArrayList<SampleEnum>();
}
@Value
static class SampleWithReference {
@Reference List<Nested> nested;
}
@Value
static class Nested {
int x, y;
}
}