DATAMONGO-2135 - Default to intermediate List for properties typed to Collection.

We now defensively create a List rather than a LinkedHashSet (which Spring's CollectionFactory.createCollection(…) defaults to) to make sure we're not accidentally dropping values that are considered equal according to their Java class definition.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

@@ -28,7 +28,6 @@ import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -531,7 +530,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 				: new BasicDBObject();
 		addCustomTypeKeyIfNecessary(ClassTypeInformation.from(prop.getRawType()), obj, propDbObj);
 
-		MongoPersistentEntity<?> entity = isSubtype(prop.getType(), obj.getClass())
+		MongoPersistentEntity<?> entity = isSubTypeOf(obj.getClass(), prop.getType())
 				? mappingContext.getPersistentEntity(obj.getClass())
 				: mappingContext.getPersistentEntity(type);
 
@@ -539,10 +538,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		accessor.put(prop, propDbObj);
 	}
 
-	private boolean isSubtype(Class<?> left, Class<?> right) {
-		return left.isAssignableFrom(right) && !left.equals(right);
-	}
-
 	/**
 	 * Returns given object as {@link Collection}. Will return the {@link Collection} as is if the source is a
 	 * {@link Collection} already, will convert an array into a {@link Collection} or simply create a single element
@@ -910,7 +905,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		TypeInformation<?> componentType = targetType.getComponentType();
 		Class<?> rawComponentType = componentType == null ? null : componentType.getType();
 
-		collectionType = Collection.class.isAssignableFrom(collectionType) ? collectionType : List.class;
+		collectionType = isSubTypeOf(collectionType, Collection.class) ? collectionType : List.class;
 		Collection<Object> items = targetType.getType().isArray() ? new ArrayList<Object>()
 				: CollectionFactory.createCollection(collectionType, rawComponentType, sourceValue.size());
 
@@ -920,7 +915,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 
 		if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) {
 
-			List<Object> objects = bulkReadAndConvertDBRefs((List<DBRef>)(List) sourceValue, componentType, path,
+			List<Object> objects = bulkReadAndConvertDBRefs((List<DBRef>) (List) sourceValue, componentType, path,
 					rawComponentType);
 			return getPotentiallyConvertedSimpleRead(objects, targetType.getType());
 		}
@@ -1360,6 +1355,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		return true;
 	}
 
+	/**
+	 * Returns whether the given type is a sub type of the given reference, i.e. assignable but not the exact same type.
+	 * 
+	 * @param type must not be {@literal null}.
+	 * @param reference must not be {@literal null}.
+	 * @return
+	 */
+	private static boolean isSubTypeOf(Class<?> type, Class<?> reference) {
+		return !type.equals(reference) && reference.isAssignableFrom(type);
+	}
+
 	/**
 	 * Marker class used to indicate we have a non root document object here that might be used within an update - so we
 	 * need to preserve type hints for potential nested elements but need to remove it on top level.

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java

@@ -21,6 +21,7 @@ import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 import static org.springframework.data.mongodb.core.DBObjectTestUtils.*;
 
+import lombok.EqualsAndHashCode;
 import lombok.RequiredArgsConstructor;
 
 import java.math.BigDecimal;
@@ -1862,6 +1863,23 @@ public class MappingMongoConverterUnitTests {
 		assertThat(target.get("_class"), is(nullValue()));
 	}
 
+	@Test // DATAMONGO-2135
+	public void addsEqualObjectsToCollection() {
+
+		DBObject itemDocument = new BasicDBObject("itemKey", "123");
+
+		BasicDBList items = new BasicDBList();
+		items.add(itemDocument);
+		items.add(itemDocument);
+		items.add(itemDocument);
+
+		DBObject orderDocument = new BasicDBObject("items", items);
+
+		Order order = converter.read(Order.class, orderDocument);
+
+		assertThat(order.items, hasSize(3));
+	}
+
 	static class GenericType<T> {
 		T content;
 	}
@@ -2232,4 +2250,15 @@ public class MappingMongoConverterUnitTests {
 	static class DocWithInterfacedEnum {
 		SomeInterface property;
 	}
+
+	// DATAMONGO-2135
+
+	@EqualsAndHashCode // equality check by fields
+	static class SomeItem {
+		String itemKey;
+	}
+
+	static class Order {
+		Collection<SomeItem> items = new ArrayList<SomeItem>();
+	}
 }