From bd92f67e5410399470dedfd7094a4eccddf2d841 Mon Sep 17 00:00:00 2001
From: Mark Paluch <mpaluch@pivotal.io>
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   |  8 +++--
 .../DbRefMappingMongoConverterUnitTests.java  | 36 +++++++++++++++++--
 2 files changed, 40 insertions(+), 4 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 4d41b65eb..ef1efc877 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,6 +28,7 @@ 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;
@@ -904,7 +905,10 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		}
 
 		if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) {
-			return bulkReadAndConvertDBRefs((List<DBRef>) (List) (sourceValue), componentType, path, rawComponentType);
+
+			List<Object> objects = bulkReadAndConvertDBRefs((List<DBRef>)(List) sourceValue, componentType, path,
+					rawComponentType);
+			return getPotentiallyConvertedSimpleRead(objects, targetType.getType());
 		}
 
 		for (Object dbObjItem : sourceValue) {
@@ -1314,7 +1318,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 
 	/**
 	 * Returns whether the given {@link Iterable} contains {@link DBRef} instances all pointing to the same collection.
-	 * 
+	 *
 	 * @param source must not be {@literal null}.
 	 * @return
 	 */
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 240ce9760..00784ca14 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
@@ -31,6 +31,7 @@ import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.bson.types.ObjectId;
 import org.junit.Before;
@@ -63,7 +64,7 @@ import com.mongodb.DBRef;
 
 /**
  * Unit tests for {@link DbRefMappingMongoConverter}.
- * 
+ *
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Christoph Strobl
@@ -548,6 +549,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 BasicDBObject("_id", id1).append("value", value), new BasicDBObject("_id", id2).append("value", value)))
+						.when(converterSpy).bulkReadRefs(anyListOf(DBRef.class));
+
+		BasicDBObject document = new BasicDBObject("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() {
 
@@ -673,6 +696,15 @@ public class DbRefMappingMongoConverterUnitTests {
 				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;
@@ -780,7 +812,7 @@ public class DbRefMappingMongoConverterUnitTests {
 			super(id, value);
 		}
 
-		/* 
+		/*
 		 * (non-Javadoc)
 		 * @see java.lang.Object#toString()
 		 */