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

@@ -248,7 +248,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	private ParameterValueProvider<MongoPersistentProperty> getParameterProvider(MongoPersistentEntity<?> entity,
 			DocumentAccessor source, SpELExpressionEvaluator evaluator, ObjectPath path) {
 
-
 		AssociationAwareMongoDbPropertyValueProvider provider = new AssociationAwareMongoDbPropertyValueProvider(source,
 				evaluator, path);
 		PersistentEntityParameterValueProvider<MongoPersistentProperty> parameterProvider = new PersistentEntityParameterValueProvider<>(
@@ -287,8 +286,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 
 		// Make sure id property is set before all other properties
 
-		Object rawId = readAndPopulateIdentifier(accessor, documentAccessor, entity,
-				path, evaluator);
+		Object rawId = readAndPopulateIdentifier(accessor, documentAccessor, entity, path, evaluator);
 		ObjectPath currentPath = path.push(accessor.getBean(), entity, rawId);
 
 		MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(documentAccessor, evaluator,
@@ -620,7 +618,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 			return;
 		}
 
-		MongoPersistentEntity<?> entity = isSubtype(prop.getType(), obj.getClass())
+		MongoPersistentEntity<?> entity = isSubTypeOf(obj.getClass(), prop.getType())
 				? mappingContext.getRequiredPersistentEntity(obj.getClass())
 				: mappingContext.getRequiredPersistentEntity(type);
 
@@ -632,10 +630,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		accessor.put(prop, document);
 	}
 
-	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
@@ -1012,7 +1006,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		Assert.notNull(path, "Object path must not be null!");
 
 		Class<?> collectionType = targetType.getType();
-		collectionType = Collection.class.isAssignableFrom(collectionType) //
+		collectionType = isSubTypeOf(collectionType, Collection.class) //
 				? collectionType //
 				: List.class;
 
@@ -1630,6 +1624,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

@@ -26,6 +26,7 @@ import static org.junit.Assert.fail;
 import static org.mockito.Mockito.*;
 import static org.springframework.data.mongodb.core.DocumentTestUtils.*;
 
+import lombok.EqualsAndHashCode;
 import lombok.RequiredArgsConstructor;
 
 import java.math.BigDecimal;
@@ -1913,6 +1914,18 @@ public class MappingMongoConverterUnitTests {
 		assertThat(target).doesNotContainKeys("_class");
 	}
 
+	@Test // DATAMONGO-2135
+	public void addsEqualObjectsToCollection() {
+
+		org.bson.Document itemDocument = new org.bson.Document("itemKey", "123");
+		org.bson.Document orderDocument = new org.bson.Document("items",
+				Arrays.asList(itemDocument, itemDocument, itemDocument));
+
+		Order order = converter.read(Order.class, orderDocument);
+
+		assertThat(order.items).hasSize(3);
+	}
+
 	static class GenericType<T> {
 		T content;
 	}
@@ -2341,4 +2354,15 @@ public class MappingMongoConverterUnitTests {
 		final @Id String id;
 		String value;
 	}
+
+	// DATAMONGO-2135
+
+	@EqualsAndHashCode // equality check by fields
+	static class SomeItem {
+		String itemKey;
+	}
+
+	static class Order {
+		Collection<SomeItem> items = new ArrayList<>();
+	}
 }