From bcba123e322271eb0dfb78658f412fc89f6adffd Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 12 Apr 2017 16:53:42 +0200 Subject: [PATCH] DATAMONGO-1666 - Consider collection type in bulk DBRef fetching. We now consider the property's collection type after bulk-fetching DBRefs before returning the actual result value. The issue got only visible if bulk fetching is possible and constructor creation is used. Setting the property value on through an property accessor works fine because the property accessor checks all values for assignability and potentially converts values to their target type. That's different for constructor creation. Original Pull Request: #457 --- .../core/convert/MappingMongoConverter.java | 14 +++-- .../DbRefMappingMongoConverterUnitTests.java | 56 +++++++++++++++---- 2 files changed, 52 insertions(+), 18 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 785b2ea1e..544c2f234 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 @@ -31,6 +31,7 @@ import org.bson.Document; import org.bson.conversions.Bson; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -55,7 +56,6 @@ import org.springframework.data.mapping.model.SpELContext; import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.mapping.model.SpELExpressionParameterValueProvider; import org.springframework.data.mongodb.MongoDbFactory; -import org.springframework.data.mongodb.core.convert.MongoConverters.ObjectIdToBigIntegerConverter; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.data.mongodb.core.mapping.event.AfterConvertEvent; @@ -241,10 +241,9 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } // Retrieve persistent entity info - Document target = bson instanceof BasicDBObject ? new Document((BasicDBObject)bson) : (Document) bson; + Document target = bson instanceof BasicDBObject ? new Document((BasicDBObject) bson) : (Document) bson; - return read((MongoPersistentEntity) mappingContext.getRequiredPersistentEntity(typeToUse), target, - path); + return read((MongoPersistentEntity) mappingContext.getRequiredPersistentEntity(typeToUse), target, path); } private ParameterValueProvider getParameterProvider(MongoPersistentEntity entity, @@ -870,7 +869,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } return dbRefResolver.createDbRef(property == null ? null : property.getDBRef(), entity, - idMapper.convertId(id instanceof Optional ? (Optional)id : Optional.ofNullable(id)).orElse(null)); + idMapper.convertId(id instanceof Optional ? (Optional) id : Optional.ofNullable(id)).orElse(null)); }).orElseThrow(() -> new MappingException("No id property found on class " + entity.getType())); } @@ -913,7 +912,10 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) { - return bulkReadAndConvertDBRefs((List) (List) (sourceValue), componentType, path, rawComponentType); + + List objects = bulkReadAndConvertDBRefs((List) sourceValue, componentType, path, + rawComponentType); + return getPotentiallyConvertedSimpleRead(objects, targetType.getType()); } for (Object dbObjItem : sourceValue) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java index a298dd3b2..1ec87d6ee 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java @@ -29,19 +29,18 @@ import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; -import com.mongodb.BasicDBObject; -import org.bson.BsonDocument; import org.bson.Document; import org.bson.conversions.Bson; import org.bson.types.ObjectId; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Matchers; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; + import org.springframework.data.annotation.AccessType; import org.springframework.data.annotation.AccessType.Type; import org.springframework.data.annotation.Id; @@ -65,7 +64,7 @@ import com.mongodb.client.MongoDatabase; /** * Unit tests for {@link DbRefMappingMongoConverter}. - * + * * @author Oliver Gierke * @author Thomas Darimont * @author Christoph Strobl @@ -485,7 +484,8 @@ public class DbRefMappingMongoConverterUnitTests { ClassWithLazyDbRefs result = converter.read(ClassWithLazyDbRefs.class, object); PersistentPropertyAccessor accessor = propertyEntity.getPropertyAccessor(result.dbRefToConcreteType); - MongoPersistentProperty idProperty = mappingContext.getRequiredPersistentEntity(LazyDbRefTarget.class).getIdProperty().get(); + MongoPersistentProperty idProperty = mappingContext.getRequiredPersistentEntity(LazyDbRefTarget.class) + .getIdProperty().get(); assertThat(accessor.getProperty(idProperty), is(notNullValue())); assertProxyIsResolved(result.dbRefToConcreteType, false); @@ -512,7 +512,8 @@ public class DbRefMappingMongoConverterUnitTests { @Test // DATAMONGO-1076 public void shouldNotTriggerResolvingOfLazyLoadedProxyWhenFinalizeMethodIsInvoked() throws Exception { - MongoPersistentEntity entity = mappingContext.getRequiredPersistentEntity(WithObjectMethodOverrideLazyDbRefs.class); + MongoPersistentEntity entity = mappingContext + .getRequiredPersistentEntity(WithObjectMethodOverrideLazyDbRefs.class); MongoPersistentProperty property = entity.getRequiredPersistentProperty("dbRefToPlainObject"); String idValue = new ObjectId().toString(); @@ -534,8 +535,9 @@ public class DbRefMappingMongoConverterUnitTests { String value = "val"; MappingMongoConverter converterSpy = spy(converter); - doReturn(Arrays.asList(new Document("_id", id1).append("value", value), - new Document("_id", id2).append("value", value))).when(converterSpy).bulkReadRefs(anyListOf(DBRef.class)); + doReturn( + Arrays.asList(new Document("_id", id1).append("value", value), new Document("_id", id2).append("value", value))) + .when(converterSpy).bulkReadRefs(anyListOf(DBRef.class)); Document document = new Document(); ClassWithLazyDbRefs lazyDbRefs = new ClassWithLazyDbRefs(); @@ -553,6 +555,28 @@ public class DbRefMappingMongoConverterUnitTests { verify(converterSpy, never()).readRef(Mockito.any(DBRef.class)); } + @Test // DATAMONGO-1666 + public void shouldBulkFetchSetOfReferencesForConstructorCreation() { + + String id1 = "1"; + String id2 = "2"; + String value = "val"; + + MappingMongoConverter converterSpy = spy(converter); + doReturn( + Arrays.asList(new Document("_id", id1).append("value", value), new Document("_id", id2).append("value", value))) + .when(converterSpy).bulkReadRefs(any()); + + Document document = new Document("dbRefToInterface", + Arrays.asList(new DBRef("lazyDbRefTarget", "1"), new DBRef("lazyDbRefTarget", "2"))); + + ClassWithDbRefSetConstructor result = converterSpy.read(ClassWithDbRefSetConstructor.class, document); + + assertThat(result.dbRefToInterface, is(instanceOf(Set.class))); + + verify(converterSpy, never()).readRef(Mockito.any(DBRef.class)); + } + @Test // DATAMONGO-1194 public void shouldFallbackToOneByOneFetchingWhenElementsInListOfReferencesPointToDifferentCollections() { @@ -561,9 +585,8 @@ public class DbRefMappingMongoConverterUnitTests { String value = "val"; MappingMongoConverter converterSpy = spy(converter); - doReturn(new Document("_id", id1).append("value", value)) - .doReturn(new Document("_id", id2).append("value", value)).when(converterSpy) - .readRef(Mockito.any(DBRef.class)); + doReturn(new Document("_id", id1).append("value", value)).doReturn(new Document("_id", id2).append("value", value)) + .when(converterSpy).readRef(Mockito.any(DBRef.class)); Document document = new Document(); ClassWithLazyDbRefs lazyDbRefs = new ClassWithLazyDbRefs(); @@ -678,6 +701,15 @@ public class DbRefMappingMongoConverterUnitTests { lazy = true) LazyDbRefTargetWithPeristenceConstructorWithoutDefaultConstructor dbRefToConcreteTypeWithPersistenceConstructorWithoutDefaultConstructor; } + static class ClassWithDbRefSetConstructor { + + final @org.springframework.data.mongodb.core.mapping.DBRef Set dbRefToInterface; + + public ClassWithDbRefSetConstructor(Set dbRefToInterface) { + this.dbRefToInterface = dbRefToInterface; + } + } + static class SerializableClassWithLazyDbRefs implements Serializable { private static final long serialVersionUID = 1L; @@ -785,7 +817,7 @@ public class DbRefMappingMongoConverterUnitTests { super(id, value); } - /* + /* * (non-Javadoc) * @see java.lang.Object#toString() */