DATAMONGO-2004 - Polishing.

Make sure to place the LazyLoadingProxy early in the mapping process to avoid eager fetching of documents that might then get replaced by the LazyLoadingProxy.

Original Pull Request: #571

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

@@ -62,7 +62,7 @@ class DocumentAccessor {
 	 * @return the underlying {@link Bson document}.
 	 * @since 2.1
 	 */
-	public Bson getDocument() {
+	Bson getDocument() {
 		return this.document;
 	}
 

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

@@ -660,7 +660,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	 * @param sink the {@link Collection} to write to.
 	 * @return
 	 */
-	private List<Object> writeCollectionInternal(Collection<?> source, @Nullable TypeInformation<?> type, Collection<?> sink) {
+	private List<Object> writeCollectionInternal(Collection<?> source, @Nullable TypeInformation<?> type,
+			Collection<?> sink) {
 
 		TypeInformation<?> componentType = null;
 
@@ -868,7 +869,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	 */
 	@Nullable
 	@SuppressWarnings({ "rawtypes", "unchecked" })
-	private Object getPotentiallyConvertedSimpleRead(@Nullable Object value, @Nullable  Class<?> target) {
+	private Object getPotentiallyConvertedSimpleRead(@Nullable Object value, @Nullable Class<?> target) {
 
 		if (value == null || target == null || ClassUtils.isAssignableValue(target, value)) {
 			return value;
@@ -1271,6 +1272,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	 * of the configured source {@link Document}.
 	 *
 	 * @author Oliver Gierke
+	 * @author Mark Paluch
+	 * @author Christoph Strobl
 	 */
 	class MongoDbPropertyValueProvider implements PropertyValueProvider<MongoPersistentProperty> {
 
@@ -1286,15 +1289,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		 * @param evaluator must not be {@literal null}.
 		 * @param path must not be {@literal null}.
 		 */
-		public MongoDbPropertyValueProvider(Bson source, SpELExpressionEvaluator evaluator, ObjectPath path) {
-
-			Assert.notNull(source, "Source document must no be null!");
-			Assert.notNull(evaluator, "SpELExpressionEvaluator must not be null!");
-			Assert.notNull(path, "ObjectPath must not be null!");
-
-			this.source = new DocumentAccessor(source);
-			this.evaluator = evaluator;
-			this.path = path;
+		MongoDbPropertyValueProvider(Bson source, SpELExpressionEvaluator evaluator, ObjectPath path) {
+			this(new DocumentAccessor(source), evaluator, path);
 		}
 
 		/**
@@ -1305,7 +1301,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		 * @param evaluator must not be {@literal null}.
 		 * @param path must not be {@literal null}.
 		 */
-		public MongoDbPropertyValueProvider(DocumentAccessor accessor, SpELExpressionEvaluator evaluator, ObjectPath path) {
+		MongoDbPropertyValueProvider(DocumentAccessor accessor, SpELExpressionEvaluator evaluator, ObjectPath path) {
 
 			Assert.notNull(accessor, "DocumentAccessor must no be null!");
 			Assert.notNull(evaluator, "SpELExpressionEvaluator must not be null!");
@@ -1340,6 +1336,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	 * resolution to {@link DbRefResolver}.
 	 *
 	 * @author Mark Paluch
+	 * @author Christoph Strobl
 	 * @since 2.1
 	 */
 	class AssociationAwareMongoDbPropertyValueProvider extends MongoDbPropertyValueProvider {
@@ -1352,8 +1349,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		 * @param evaluator must not be {@literal null}.
 		 * @param path must not be {@literal null}.
 		 */
-		public AssociationAwareMongoDbPropertyValueProvider(Bson source, SpELExpressionEvaluator evaluator,
-				ObjectPath path) {
+		AssociationAwareMongoDbPropertyValueProvider(Bson source, SpELExpressionEvaluator evaluator, ObjectPath path) {
 			super(source, evaluator, path);
 		}
 
@@ -1365,17 +1361,21 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		@SuppressWarnings("unchecked")
 		public <T> T getPropertyValue(MongoPersistentProperty property) {
 
-			T value = super.getPropertyValue(property);
+			if (property.isDbReference() && property.getDBRef().lazy()) {
 
-			if (value == null || !property.isAssociation()) {
-				return value;
-			}
+				Object rawRefValue = source.get(property);
+				if (rawRefValue == null) {
+					return null;
+				}
+
+				DbRefResolverCallback callback = new DefaultDbRefResolverCallback(source.getDocument(), path, evaluator,
+						MappingMongoConverter.this);
 
-			DbRefResolverCallback callback = new DefaultDbRefResolverCallback(source.getDocument(), path, evaluator,
-					MappingMongoConverter.this);
-			DBRef dbref = value instanceof DBRef ? (DBRef) value : null;
+				DBRef dbref = rawRefValue instanceof DBRef ? (DBRef) rawRefValue : null;
+				return (T) dbRefResolver.resolveDbRef(property, dbref, callback, dbRefProxyHandler);
+			}
 
-			return (T) dbRefResolver.resolveDbRef(property, dbref, callback, dbRefProxyHandler);
+			return super.getPropertyValue(property);
 		}
 	}
 

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

@@ -3448,7 +3448,7 @@ public class MongoTemplateTests {
 				DocumentWithLazyDBRefsAndConstructorCreation.class);
 
 		assertThat(target.lazyDbRefAnnotatedList, instanceOf(LazyLoadingProxy.class));
-		assertThat(target.lazyDbRefAnnotatedList, contains(two, one));
+		assertThat(target.getLazyDbRefAnnotatedList(), contains(two, one));
 	}
 
 	@Test // DATAMONGO-1513

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

@@ -0,0 +1,145 @@
+/*
+ * Copyright 2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mongodb.core.convert;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.any;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.NoArgsConstructor;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.bson.Document;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.data.annotation.Id;
+import org.springframework.data.mongodb.MongoDbFactory;
+import org.springframework.data.mongodb.core.SimpleMongoDbFactory;
+import org.springframework.data.mongodb.core.mapping.DBRef;
+import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
+
+import com.mongodb.MongoClient;
+
+/**
+ * Integration tests for {@link MappingMongoConverter}.
+ *
+ * @author Christoph Strobl
+ */
+public class MappingMongoConverterTests {
+
+	MongoClient client;
+
+	MappingMongoConverter converter;
+	MongoMappingContext mappingContext;
+	DbRefResolver dbRefResolver;
+
+	@Before
+	public void setUp() {
+
+		client = new MongoClient();
+		client.dropDatabase("mapping-converter-tests");
+
+		MongoDbFactory factory = new SimpleMongoDbFactory(client, "mapping-converter-tests");
+
+		dbRefResolver = spy(new DefaultDbRefResolver(factory));
+		mappingContext = new MongoMappingContext();
+		mappingContext.afterPropertiesSet();
+
+		converter = new MappingMongoConverter(dbRefResolver, mappingContext);
+	}
+
+	@Test // DATAMONGO-2004
+	public void resolvesLazyDBRefOnAccess() {
+
+		client.getDatabase("mapping-converter-tests").getCollection("samples")
+				.insertMany(Arrays.asList(new Document("_id", "sample-1").append("value", "one"),
+						new Document("_id", "sample-2").append("value", "two")));
+
+		Document source = new Document("_id", "id-1").append("lazyList",
+				Arrays.asList(new com.mongodb.DBRef("samples", "sample-1"), new com.mongodb.DBRef("samples", "sample-2")));
+
+		WithLazyDBRef target = converter.read(WithLazyDBRef.class, source);
+
+		verify(dbRefResolver).resolveDbRef(any(), isNull(), any(), any());
+		verifyNoMoreInteractions(dbRefResolver);
+
+		assertThat(target.lazyList).isInstanceOf(LazyLoadingProxy.class);
+		assertThat(target.getLazyList()).contains(new Sample("sample-1", "one"), new Sample("sample-2", "two"));
+
+		verify(dbRefResolver).bulkFetch(any());
+	}
+
+	@Test // DATAMONGO-2004
+	public void resolvesLazyDBRefConstructorArgOnAccess() {
+
+		client.getDatabase("mapping-converter-tests").getCollection("samples")
+				.insertMany(Arrays.asList(new Document("_id", "sample-1").append("value", "one"),
+						new Document("_id", "sample-2").append("value", "two")));
+
+		Document source = new Document("_id", "id-1").append("lazyList",
+				Arrays.asList(new com.mongodb.DBRef("samples", "sample-1"), new com.mongodb.DBRef("samples", "sample-2")));
+
+		WithLazyDBRefAsConstructorArg target = converter.read(WithLazyDBRefAsConstructorArg.class, source);
+
+		verify(dbRefResolver).resolveDbRef(any(), isNull(), any(), any());
+		verifyNoMoreInteractions(dbRefResolver);
+
+		assertThat(target.lazyList).isInstanceOf(LazyLoadingProxy.class);
+		assertThat(target.getLazyList()).contains(new Sample("sample-1", "one"), new Sample("sample-2", "two"));
+
+		verify(dbRefResolver).bulkFetch(any());
+	}
+
+	public static class WithLazyDBRef {
+
+		@Id String id;
+		@DBRef(lazy = true) List<Sample> lazyList;
+
+		public List<Sample> getLazyList() {
+			return lazyList;
+		}
+	}
+
+	public static class WithLazyDBRefAsConstructorArg {
+
+		@Id String id;
+		@DBRef(lazy = true) List<Sample> lazyList;
+
+		public WithLazyDBRefAsConstructorArg(String id, List<Sample> lazyList) {
+
+			this.id = id;
+			this.lazyList = lazyList;
+		}
+
+		public List<Sample> getLazyList() {
+			return lazyList;
+		}
+	}
+
+	@Data
+	@AllArgsConstructor
+	@NoArgsConstructor
+	static class Sample {
+
+		@Id String id;
+		String value;
+	}
+}

src/main/asciidoc/reference/mapping.adoc

@@ -584,9 +584,13 @@ public class Person {
 ====
 
 You need not use `@OneToMany` or similar mechanisms because the List of objects tells the mapping framework that you want a one-to-many relationship. When the object is stored in MongoDB, there is a list of DBRefs rather than the `Account` objects themselves.
+When it comes to loading collections of ``DBRef``s it is advisable to restrict references held in collection types to a specific MongoDB collection. This allows bulk loading of all references, whereas references pointing to different MongoDB collections need to be resolved one by one.
 
 IMPORTANT: The mapping framework does not handle cascading saves. If you change an `Account` object that is referenced by a `Person` object, you must save the `Account` object separately. Calling `save` on the `Person` object does not automatically save the `Account` objects in the `accounts` property.
 
+``DBRef``s can also be resolved lazily. In this case the actual `Object` or `Collection` of references is resolved on first access of the property. Use the `lazy` attribute of `@DBRef` to specify this.
+Required properties that are also defined as lazy loading ``DBRef`` and used as constructor arguments are also decorated with the lazy loading proxy making sure to put as little pressure on the database and network as possible.
+
 [[mapping-usage-events]]
 === Mapping Framework Events