From 01c1ec2e5bd01c31c990aa284df3cdfc374fd96c Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 15 Nov 2018 15:26:36 +0100 Subject: [PATCH] DATAMONGO-2135 - Default to intermediate List for properties typed to Collection. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../core/convert/MappingMongoConverter.java | 22 +++++++++----- .../MappingMongoConverterUnitTests.java | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index c4475fce0..839690ad6 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/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 items = targetType.getType().isArray() ? new ArrayList() : 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 objects = bulkReadAndConvertDBRefs((List)(List) sourceValue, componentType, path, + List objects = bulkReadAndConvertDBRefs((List) (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. diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index ac1d55e46..99418b896 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/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 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 items = new ArrayList(); + } }