DATAREST-1012 - added test to reproduce issue DATAREST-1012, fix Domain…

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

@@ -24,6 +24,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -62,6 +63,7 @@
  * @author Mark Paluch
  * @author Craig Andrews
  * @author Mathias Düsterhöft
+ * @author Stefan Reichert
  * @since 2.2
  */
 @RequiredArgsConstructor
@@ -448,26 +450,26 @@ private Optional<Collection<Object>> mergeCollections(PersistentProperty<?> prop
 
 			Collection<Object> sourceCollection = asCollection(it);
 			Collection<Object> targetCollection = asCollection(target.orElse(null));
+
 			Collection<Object> result = targetCollection == null
 					? CollectionFactory.createCollection(Collection.class, sourceCollection.size())
 					: CollectionFactory.createApproximateCollection(targetCollection, sourceCollection.size());
 
 			Iterator<Object> sourceIterator = sourceCollection.iterator();
-			Iterator<Object> targetIterator = targetCollection == null ? Collections.emptyIterator()
-					: targetCollection.iterator();
+
+			Map<Class<?>, Iterator<Object>> targetIteratorByTypeMap = createTargetIteratorByTypeMap(targetCollection);
 
 			while (sourceIterator.hasNext()) {
 
 				Object sourceElement = sourceIterator.next();
+				boolean sourceElementTypeExists = targetIteratorByTypeMap.containsKey(sourceElement.getClass());
+				Iterator<Object> targetIterator = sourceElementTypeExists ? targetIteratorByTypeMap.get(sourceElement.getClass())
+						: Collections.emptyListIterator();
 				Object targetElement = targetIterator.hasNext() ? targetIterator.next() : null;
 
 				result.add(mergeForPut(sourceElement, targetElement, mapper));
 			}
 
-			if (targetCollection == null) {
-				return result;
-			}
-
 			try {
 
 				targetCollection.clear();
@@ -480,6 +482,30 @@ private Optional<Collection<Object>> mergeCollections(PersistentProperty<?> prop
 			}
 		});
 	}
+
+	private Map<Class<?>, Iterator<Object>> createTargetIteratorByTypeMap(Collection<Object> targetCollection) {
+		if (targetCollection == null) {
+			return Collections.emptyMap();
+		}
+		// group the target collection by concrete type
+		Map<Class<?>, Collection<Object>> targetCollectionByTypeMap = new HashMap<Class<?>, Collection<Object>>();
+		for (Object object : targetCollection) {
+			if (!targetCollectionByTypeMap.containsKey(object.getClass())) {
+				ArrayList<Object> targetCollectionByType = new ArrayList<Object>();
+				targetCollectionByTypeMap.put(object.getClass(), targetCollectionByType);
+			}
+			targetCollectionByTypeMap.get(object.getClass()).add(object);
+		}
+
+		// convert the homogeneous collections to iterators
+		Map<Class<?>, Iterator<Object>> targetIteratorByTypeMap = new HashMap<Class<?>, Iterator<Object>>();
+		for (Entry<Class<?>, Collection<Object>> targetCollectionByTypeEntry : targetCollectionByTypeMap.entrySet()) {
+			Class<?> type = targetCollectionByTypeEntry.getKey();
+			Collection<Object> collection = targetCollectionByTypeEntry.getValue();
+			targetIteratorByTypeMap.put(type, collection.iterator());
+		}
+		return targetIteratorByTypeMap;
+	}
 
 	@SuppressWarnings("unchecked")
 	private static Collection<Object> asCollection(Object source) {

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

@@ -39,6 +39,12 @@
 import java.util.Locale;
 import java.util.Map;
 
+import javax.persistence.Basic;
+import javax.persistence.CascadeType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.GenerationType;
+import javax.persistence.OneToMany;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -59,6 +65,8 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.PropertyNamingStrategy;
@@ -72,6 +80,7 @@
  * @author Craig Andrews
  * @author Mathias Düsterhöft
  * @author Ken Dombeck
+ * @author Stefan Reichert
  */
 @RunWith(MockitoJUnitRunner.class)
 public class DomainObjectReaderUnitTests {
@@ -98,6 +107,11 @@ public void setUp() {
 		mappingContext.getPersistentEntity(TransientReadOnlyProperty.class);
 		mappingContext.getPersistentEntity(CollectionOfEnumWithMethods.class);
 		mappingContext.getPersistentEntity(SampleWithReference.class);
+		mappingContext.getPersistentEntity(Basket.class);
+		mappingContext.getPersistentEntity(Fruit.class);
+		mappingContext.getPersistentEntity(Apple.class);
+		mappingContext.getPersistentEntity(Pear.class);
+		mappingContext.getPersistentEntity(Orange.class);
 		mappingContext.afterPropertiesSet();
 
 		PersistentEntities entities = new PersistentEntities(Collections.singleton(mappingContext));
@@ -534,6 +548,29 @@ public Person(String firstName, String lastName) {
 		protected Person() {}
 	}
 
+	@Test // DATAREST-1012
+	public void writesPolymorphicArrayWithSwitchedItemForPut() throws Exception {
+
+		Basket basket = new Basket();
+		basket.fruits = new ArrayList<Fruit>();
+		basket.fruits.add(new Apple());
+		basket.fruits.add(new 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(), is(3));
+		assertThat(result.fruits.get(0).color, is("green"));
+		assertThat(result.fruits.get(0) instanceof Pear, is(true));
+		assertThat(result.fruits.get(1).color, is("orange"));
+		assertThat(result.fruits.get(1) instanceof Orange, is(true));
+		assertThat(result.fruits.get(2).color, is("red"));
+		assertThat(result.fruits.get(2) instanceof Apple, is(true));
+	}
+
 	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
 	static class TypeWithGenericMap {
 
@@ -608,6 +645,33 @@ static class Item {
 		String some;
 	}
 
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	static class Basket {
+		@Id @GeneratedValue(strategy = GenerationType.AUTO) Long id;
+
+		@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) List<Fruit> fruits;
+	}
+
+	@JsonAutoDetect(fieldVisibility = Visibility.ANY)
+	@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.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 Product {
 		Map<Locale, LocalizedValue> map = new HashMap<Locale, LocalizedValue>();