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 489b15d44..5519c61be 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 @@ -20,14 +20,9 @@ import lombok.RequiredArgsConstructor; 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.*; import java.util.Map.Entry; -import java.util.Optional; +import java.util.stream.Collectors; import org.springframework.beans.PropertyAccessor; import org.springframework.beans.PropertyAccessorFactory; @@ -62,6 +57,7 @@ * @author Mark Paluch * @author Craig Andrews * @author Mathias Düsterhöft + * @author Stefan Reichert * @since 2.2 */ @RequiredArgsConstructor @@ -451,12 +447,19 @@ private Optional> mergeCollections(PersistentProperty prop : CollectionFactory.createApproximateCollection(targetCollection, sourceCollection.size()); Iterator sourceIterator = sourceCollection.iterator(); - Iterator targetIterator = targetCollection == null ? Collections.emptyIterator() - : targetCollection.iterator(); + // FIX DATAREST-1012: map the target collection by type to avoid incompatible type on heterogeneous + // polymorphic collections. This map is used to retrieve matching merge targets for the incoming elements + Map, Iterator> targetIteratorByTypeMap = createIteratorByTypeMap(targetCollection); while (sourceIterator.hasNext()) { Object sourceElement = sourceIterator.next(); + boolean sourceElementTypeExists = targetIteratorByTypeMap.containsKey(sourceElement.getClass()); + + // FIX DATAREST-1012: identify an object of matching type as merge target + Iterator targetIterator = sourceElementTypeExists + ? targetIteratorByTypeMap.get(sourceElement.getClass()) + : Collections.emptyListIterator(); Object targetElement = targetIterator.hasNext() ? targetIterator.next() : null; result.add(mergeForPut(sourceElement, targetElement, mapper)); @@ -479,6 +482,23 @@ private Optional> mergeCollections(PersistentProperty prop }); } + /** + * Returns a map containing target collection elements grouped by their type. The key is represented by the + * type's {@link Class} mapping a collection of the respective matching elements. + * + * @param targetCollection + * @return + */ + private Map, Iterator> createIteratorByTypeMap(Collection targetCollection) { + Map, Iterator> targetIteratorByTypeMap = new HashMap<>(); + if (targetCollection != null) { + targetCollection.stream() + .collect(Collectors.groupingBy(Object::getClass)) + .forEach((key, value) -> targetIteratorByTypeMap.put(key, value.iterator())); + } + return targetIteratorByTypeMap; + } + @SuppressWarnings("unchecked") private static Collection asCollection(Object source) { @@ -497,7 +517,6 @@ private static Collection asCollection(Object source) { * Returns the given source instance as {@link Collection} or creates a new one for the given type. * * @param source can be {@literal null}. - * @param type must not be {@literal null} in case {@code source} is null. * @return */ @SuppressWarnings("unchecked") 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 55fe8bd6b..c603790ec 100755 --- 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 @@ -16,6 +16,8 @@ package org.springframework.data.rest.webmvc.json; import static com.fasterxml.jackson.annotation.JsonProperty.Access.*; +import static com.fasterxml.jackson.annotation.JsonTypeInfo.As.*; +import static javax.persistence.CascadeType.*; import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; @@ -26,6 +28,10 @@ import lombok.RequiredArgsConstructor; import lombok.Value; +import javax.persistence.Basic; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.OneToMany; import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.*; @@ -55,6 +61,8 @@ import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationContext; @@ -64,6 +72,7 @@ import com.fasterxml.jackson.databind.PropertyNamingStrategy; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.node.ObjectNode; + import com.google.common.base.Charsets; /** @@ -73,11 +82,13 @@ * @author Craig Andrews * @author Mathias Düsterhöft * @author Ken Dombeck + * @author Stefan Reichert */ @RunWith(MockitoJUnitRunner.class) public class DomainObjectReaderUnitTests { - @Mock ResourceMappings mappings; + @Mock + ResourceMappings mappings; DomainObjectReader reader; PersistentEntities entities; @@ -102,6 +113,12 @@ public void setUp() { mappingContext.getPersistentEntity(SampleWithReference.class); mappingContext.getPersistentEntity(Note.class); mappingContext.getPersistentEntity(WithNullCollection.class); + mappingContext.getPersistentEntity(Basket.class); + mappingContext.getPersistentEntity(Fruit.class); + mappingContext.getPersistentEntity(Apple.class); + mappingContext.getPersistentEntity(Pear.class); + mappingContext.getPersistentEntity(Orange.class); + mappingContext.getPersistentEntity(Banana.class); mappingContext.afterPropertiesSet(); this.entities = new PersistentEntities(Collections.singleton(mappingContext)); @@ -516,6 +533,64 @@ public void mergesAssociationsAndKeepsMutableCollection() { assertThat(result.nested).isSameAs(originalCollection); } + @Test // DATAREST-1012 + public void writesPolymorphicArrayWithSwitchedItemForPut() throws Exception { + + Basket basket = new Basket(); + Apple apple = new Apple(); + Pear pear = new Pear(); + basket.fruits = new ArrayList<>(); + basket.fruits.add(new Banana()); + basket.fruits.add(apple); + basket.fruits.add(pear); + + JsonNode node = new ObjectMapper().readTree("{ \"fruits\" : [ " + + "{ \"@class\" : \"Pear\", \"color\" : \"green\" }," + "{ \"@class\" : \"Orange\", \"color\" : \"orange\" }," + + "{ \"@class\" : \"Apple\", \"color\" : \"red\" } ] }"); + + Basket result = reader.readPut((ObjectNode) node, basket, new ObjectMapper()); + + assertThat(result.fruits.size()).isEqualTo(3); + assertThat(result.fruits.get(0)).isSameAs(pear); + assertThat(result.fruits.get(0).color).isEqualTo("green"); + assertThat(result.fruits.get(1)).isInstanceOfAny(Orange.class); + assertThat(result.fruits.get(1).color).isEqualTo("orange"); + assertThat(result.fruits.get(2)).isSameAs(apple); + assertThat(result.fruits.get(2).color).isEqualTo("red"); + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Basket { + @Id @GeneratedValue(strategy = GenerationType.AUTO) Long id; + + @OneToMany(cascade = ALL, orphanRemoval = true) List fruits; + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = PROPERTY, property = "@class") + @JsonSubTypes({ @JsonSubTypes.Type(name = "Apple", value = Apple.class), + @JsonSubTypes.Type(name = "Pear", value = Pear.class), + @JsonSubTypes.Type(name = "Orange", value = Orange.class) }) + static class Fruit { + @Id @GeneratedValue(strategy = GenerationType.AUTO) Long id; + + @Basic + String color; + } + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Apple extends Fruit {} + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Pear extends Fruit {} + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Orange extends Fruit {} + + @JsonAutoDetect(fieldVisibility = Visibility.ANY) + static class Banana extends Fruit {} + + @Test // DATAREST-1030 public void patchWithReferenceToRelatedEntityIsResolvedCorrectly() throws Exception {