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: spring-projects#457

Author: mp911de

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

@@ -31,6 +31,7 @@
 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.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 @@ private <S extends Object> S read(TypeInformation<S> type, Bson bson, ObjectPath
 		}
 		// 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<S>) mappingContext.getRequiredPersistentEntity(typeToUse), target,
-				path);
+		return read((MongoPersistentEntity<S>) mappingContext.getRequiredPersistentEntity(typeToUse), target, path);
 	}
 
 	private ParameterValueProvider<MongoPersistentProperty> getParameterProvider(MongoPersistentEntity<?> entity,
@@ -870,7 +869,7 @@ protected DBRef createDBRef(Object target, MongoPersistentProperty property) {
 			}
 
 			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 @@ private Object readCollectionOrArray(TypeInformation<?> targetType, List sourceV
 		}
 
 		if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) {
-			return bulkReadAndConvertDBRefs((List<DBRef>) (List) (sourceValue), componentType, path, rawComponentType);
+
+			List<Object> objects = bulkReadAndConvertDBRefs((List<DBRef>) sourceValue, componentType, path,
+					rawComponentType);
+			return getPotentiallyConvertedSimpleRead(objects, targetType.getType());
 		}
 
 		for (Object dbObjItem : sourceValue) {

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

@@ -29,19 +29,18 @@
 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 @@
 
 /**
  * Unit tests for {@link DbRefMappingMongoConverter}.
- * 
+ *
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Christoph Strobl
@@ -485,7 +484,8 @@ public void shouldEagerlyResolveIdPropertyWithFieldAccess() {
 		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 void shouldNotEagerlyResolveIdPropertyWithPropertyAccess() {
 	@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 void shouldBulkFetchListOfReferences() {
 		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 void shouldBulkFetchListOfReferences() {
 		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 void shouldFallbackToOneByOneFetchingWhenElementsInListOfReferencesPointT
 		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 @@ static class ClassWithLazyDbRefs {
 				lazy = true) LazyDbRefTargetWithPeristenceConstructorWithoutDefaultConstructor dbRefToConcreteTypeWithPersistenceConstructorWithoutDefaultConstructor;
 	}
 
+	static class ClassWithDbRefSetConstructor {
+
+		final @org.springframework.data.mongodb.core.mapping.DBRef Set<LazyDbRefTarget> dbRefToInterface;
+
+		public ClassWithDbRefSetConstructor(Set<LazyDbRefTarget> dbRefToInterface) {
+			this.dbRefToInterface = dbRefToInterface;
+		}
+	}
+
 	static class SerializableClassWithLazyDbRefs implements Serializable {
 
 		private static final long serialVersionUID = 1L;
@@ -785,7 +817,7 @@ public ToStringObjectMethodOverrideLazyDbRefTarget(String id, String value) {
 			super(id, value);
 		}
 
-		/* 
+		/*
 		 * (non-Javadoc)
 		 * @see java.lang.Object#toString()
 		 */