Polishing.

Remove sysout from tests.

See #4491

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>4.2.0-M2</version>
+	<version>4.2.0-GH-4491-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>
@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data.build</groupId>
 		<artifactId>spring-data-parent</artifactId>
-		<version>3.2.0-M2</version>
+		<version>3.2.0-SNAPSHOT</version>
 	</parent>
 
 	<modules>
@@ -26,7 +26,7 @@
 	<properties>
 		<project.type>multi</project.type>
 		<dist.id>spring-data-mongodb</dist.id>
-		<springdata.commons>3.2.0-M2</springdata.commons>
+		<springdata.commons>3.2.0-SNAPSHOT</springdata.commons>
 		<mongo>4.10.2</mongo>
 		<mongo.reactivestreams>${mongo}</mongo.reactivestreams>
 		<jmh.version>1.19</jmh.version>
@@ -144,6 +144,16 @@
 	</dependencies>
 
 	<repositories>
+		<repository>
+			<id>spring-snapshot</id>
+			<url>https://repo.spring.io/snapshot</url>
+			<snapshots>
+				<enabled>true</enabled>
+			</snapshots>
+			<releases>
+				<enabled>false</enabled>
+			</releases>
+		</repository>
 		<repository>
 			<id>spring-milestone</id>
 			<url>https://repo.spring.io/milestone</url>

spring-data-mongodb-benchmarks/pom.xml

@@ -7,7 +7,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-M2</version>
+		<version>4.2.0-GH-4491-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb-distribution/pom.xml

@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-M2</version>
+		<version>4.2.0-GH-4491-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -13,7 +13,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.2.0-M2</version>
+		<version>4.2.0-GH-4491-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

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

@@ -17,7 +17,16 @@ package org.springframework.data.mongodb.core.convert;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -41,7 +50,13 @@ import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.data.annotation.Reference;
 import org.springframework.data.convert.CustomConversions;
 import org.springframework.data.convert.TypeMapper;
-import org.springframework.data.mapping.*;
+import org.springframework.data.mapping.Association;
+import org.springframework.data.mapping.InstanceCreatorMetadata;
+import org.springframework.data.mapping.MappingException;
+import org.springframework.data.mapping.Parameter;
+import org.springframework.data.mapping.PersistentEntity;
+import org.springframework.data.mapping.PersistentProperty;
+import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.callback.EntityCallbacks;
 import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
@@ -98,6 +113,7 @@ import com.mongodb.DBRef;
  * @author Roman Puchkovskiy
  * @author Heesu Jung
  * @author Divya Srivastava
+ * @author Julia Lee
  */
 public class MappingMongoConverter extends AbstractMongoConverter implements ApplicationContextAware {
 
@@ -491,7 +507,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		S instance = instantiator.createInstance(entity, provider);
 
 		if (entity.requiresPropertyPopulation()) {
-
 			return populateProperties(context, entity, documentAccessor, evaluator, instance);
 		}
 
@@ -570,14 +585,18 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 			ConversionContext propertyContext = context.forProperty(prop);
 			MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext);
 
-			if (prop.isAssociation() && !entity.isCreatorArgument(prop)) {
+			if (prop.isAssociation()) {
 
 				if (callback == null) {
 					callback = getDbRefResolverCallback(propertyContext, documentAccessor, evaluator);
 				}
 
-				readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback,
-						propertyContext, evaluator);
+				Object value = readAssociation(prop.getRequiredAssociation(), documentAccessor, dbRefProxyHandler, callback,
+						propertyContext);
+
+				if (value != null) {
+					accessor.setProperty(prop, value);
+				}
 				continue;
 			}
 
@@ -592,17 +611,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 				continue;
 			}
 
-			if (prop.isAssociation()) {
-
-				if (callback == null) {
-					callback = getDbRefResolverCallback(propertyContext, documentAccessor, evaluator);
-				}
-
-				readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback,
-						propertyContext, evaluator);
-				continue;
-			}
-
 			accessor.setProperty(prop, valueProviderToUse.getPropertyValue(prop));
 		}
 	}
@@ -614,9 +622,10 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 				(prop, bson, e, path) -> MappingMongoConverter.this.getValueInternal(context, prop, bson, e));
 	}
 
-	private void readAssociation(Association<MongoPersistentProperty> association, PersistentPropertyAccessor<?> accessor,
+	@Nullable
+	private Object readAssociation(Association<MongoPersistentProperty> association,
 			DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback,
-			ConversionContext context, SpELExpressionEvaluator evaluator) {
+			ConversionContext context) {
 
 		MongoPersistentProperty property = association.getInverse();
 		Object value = documentAccessor.get(property);
@@ -629,30 +638,27 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 			if (conversionService.canConvert(DocumentPointer.class, property.getActualType())) {
 
 				if (value == null) {
-					return;
+					return null;
 				}
 
 				DocumentPointer<?> pointer = () -> value;
 
 				// collection like special treatment
-				accessor.setProperty(property, conversionService.convert(pointer, property.getActualType()));
+				return conversionService.convert(pointer, property.getActualType());
 			} else {
 
-				accessor.setProperty(property,
-						dbRefResolver.resolveReference(property,
+				return dbRefResolver.resolveReference(property,
 								new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
-								referenceLookupDelegate, context.forProperty(property)::convert));
+						referenceLookupDelegate, context.forProperty(property)::convert);
 			}
-			return;
 		}
 
 		if (value == null) {
-			return;
+			return null;
 		}
 
 		if (value instanceof DBRef dbref) {
-			accessor.setProperty(property, dbRefResolver.resolveDbRef(property, dbref, callback, handler));
-			return;
+			return dbRefResolver.resolveDbRef(property, dbref, callback, handler);
 		}
 
 		/*
@@ -663,18 +669,18 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		if (value instanceof Document document) {
 			if (property.isMap()) {
 				if (document.isEmpty() || peek(document.values()) instanceof DBRef) {
-					accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler));
+					return dbRefResolver.resolveDbRef(property, null, callback, handler);
 				} else {
-					accessor.setProperty(property, readMap(context, document, property.getTypeInformation()));
+					return readMap(context, document, property.getTypeInformation());
 				}
 			} else {
-				accessor.setProperty(property, read(property.getActualType(), document));
+				return read(property.getActualType(), document);
 			}
 		} else if (value instanceof Collection<?> collection && !collection.isEmpty()
 				&& peek(collection) instanceof Document) {
-			accessor.setProperty(property, readCollectionOrArray(context, collection, property.getTypeInformation()));
+			return readCollectionOrArray(context, collection, property.getTypeInformation());
 		} else {
-			accessor.setProperty(property, dbRefResolver.resolveDbRef(property, null, callback, handler));
+			return dbRefResolver.resolveDbRef(property, null, callback, handler);
 		}
 	}
 
@@ -1960,24 +1966,26 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		@SuppressWarnings("unchecked")
 		public <T> T getPropertyValue(MongoPersistentProperty property) {
 
-			if (property.isDbReference() && property.getDBRef().lazy()) {
+			ConversionContext propertyContext = context.forProperty(property);
 
-				Object rawRefValue = accessor.get(property);
-				if (rawRefValue == null) {
-					return null;
-				}
+			if (property.isAssociation()) {
 
 				DbRefResolverCallback callback = new DefaultDbRefResolverCallback(accessor.getDocument(), context.getPath(),
 						evaluator, (prop, bson, evaluator, path) -> MappingMongoConverter.this.getValueInternal(context, prop, bson,
 								evaluator));
 
-				DBRef dbref = rawRefValue instanceof DBRef dbRef ? dbRef : null;
-				return (T) dbRefResolver.resolveDbRef(property, dbref, callback, dbRefProxyHandler);
+				return (T) readAssociation(property.getRequiredAssociation(), accessor, dbRefProxyHandler, callback,
+						propertyContext);
 			}
 
-			if (property.isDocumentReference()) {
-				return (T) dbRefResolver.resolveReference(property, accessor.get(property), referenceLookupDelegate,
-						context::convert);
+			if (property.isUnwrapped()) {
+
+				return (T) readUnwrapped(propertyContext, accessor, property,
+						mappingContext.getRequiredPersistentEntity(property));
+			}
+
+			if (!accessor.hasValue(property)) {
+				return null;
 			}
 
 			return super.getPropertyValue(property);

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/UnwrappedMongoPersistentProperty.java

@@ -255,6 +255,11 @@ class UnwrappedMongoPersistentProperty implements MongoPersistentProperty {
 		return delegate.isWritable();
 	}
 
+	@Override
+	public boolean isReadable() {
+		return delegate.isReadable();
+	}
+
 	@Override
 	public boolean isImmutable() {
 		return delegate.isImmutable();

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/observability/MongoObservationCommandListener.java

@@ -15,6 +15,12 @@
  */
 package org.springframework.data.mongodb.observability;
 
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;
+
+import java.util.function.BiConsumer;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.lang.Nullable;
@@ -27,10 +33,6 @@ import com.mongodb.event.CommandListener;
 import com.mongodb.event.CommandStartedEvent;
 import com.mongodb.event.CommandSucceededEvent;
 
-import io.micrometer.observation.Observation;
-import io.micrometer.observation.ObservationRegistry;
-import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;
-
 /**
  * Implement MongoDB's {@link CommandListener} using Micrometer's {@link Observation} API.
  *
@@ -126,50 +128,54 @@ public class MongoObservationCommandListener implements CommandListener {
 	@Override
 	public void commandSucceeded(CommandSucceededEvent event) {
 
-		RequestContext requestContext = event.getRequestContext();
+		doInObservation(event.getRequestContext(), (observation, context) -> {
 
-		if (requestContext == null) {
-			return;
-		}
+			context.setCommandSucceededEvent(event);
 
-		Observation observation = requestContext.getOrDefault(ObservationThreadLocalAccessor.KEY, null);
-		if (observation == null) {
-			return;
-		}
+			if (log.isDebugEnabled()) {
+				log.debug("Command succeeded - will stop observation [" + observation + "]");
+			}
 
-		MongoHandlerContext context = (MongoHandlerContext) observation.getContext();
-		context.setCommandSucceededEvent(event);
-
-		if (log.isDebugEnabled()) {
-			log.debug("Command succeeded - will stop observation [" + observation + "]");
-		}
-
-		observation.stop();
+			observation.stop();
+		});
 	}
 
 	@Override
 	public void commandFailed(CommandFailedEvent event) {
 
-		RequestContext requestContext = event.getRequestContext();
+		doInObservation(event.getRequestContext(), (observation, context) -> {
+
+			context.setCommandFailedEvent(event);
+
+			if (log.isDebugEnabled()) {
+				log.debug("Command failed - will stop observation [" + observation + "]");
+			}
+
+			observation.error(event.getThrowable());
+			observation.stop();
+		});
+	}
+
+	/**
+	 * Performs the given action for the {@link Observation} and {@link MongoHandlerContext} if there is an ongoing Mongo
+	 * Observation. Exceptions thrown by the action are relayed to the caller.
+	 *
+	 * @param requestContext the context to extract the Observation from.
+	 * @param action the action to invoke.
+	 */
+	private void doInObservation(@Nullable RequestContext requestContext,
+			BiConsumer<Observation, MongoHandlerContext> action) {
 
 		if (requestContext == null) {
 			return;
 		}
 
 		Observation observation = requestContext.getOrDefault(ObservationThreadLocalAccessor.KEY, null);
-		if (observation == null) {
+		if (observation == null || !(observation.getContext()instanceof MongoHandlerContext context)) {
 			return;
 		}
 
-		MongoHandlerContext context = (MongoHandlerContext) observation.getContext();
-		context.setCommandFailedEvent(event);
-
-		if (log.isDebugEnabled()) {
-			log.debug("Command failed - will stop observation [" + observation + "]");
-		}
-
-		observation.error(event.getThrowable());
-		observation.stop();
+		action.accept(observation, context);
 	}
 
 	/**

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

@@ -57,6 +57,7 @@ import com.mongodb.client.model.Filters;
  * {@link DocumentReference} related integration tests for {@link MongoTemplate}.
  *
  * @author Christoph Strobl
+ * @author Julia Lee
  */
 @ExtendWith(MongoClientExtension.class)
 public class MongoTemplateDocumentReferenceTests {
@@ -1265,6 +1266,32 @@ public class MongoTemplateDocumentReferenceTests {
 				.isEqualTo(new ObjectRefHavingStringIdTargetType(id.toHexString(), "me-the-referenced-object"));
 	}
 
+	@Test // GH-4484
+	void resolveReferenceForOneToManyLookupWithSelfVariableWhenUsedInCtorArgument() {
+
+		OneToManyStylePublisherWithRequiredArgsCtor publisher = new OneToManyStylePublisherWithRequiredArgsCtor("p-100", null);
+		template.save(publisher);
+
+		OneToManyStyleBook book1 = new OneToManyStyleBook();
+		book1.id = "id-1";
+		book1.publisherId = publisher.id;
+
+		OneToManyStyleBook book2 = new OneToManyStyleBook();
+		book2.id = "id-2";
+		book2.publisherId = "p-200";
+
+		OneToManyStyleBook book3 = new OneToManyStyleBook();
+		book3.id = "id-3";
+		book3.publisherId = publisher.id;
+
+		template.save(book1);
+		template.save(book2);
+		template.save(book3);
+
+		OneToManyStylePublisherWithRequiredArgsCtor target = template.findOne(query(where("id").is(publisher.id)), OneToManyStylePublisherWithRequiredArgsCtor.class);
+		assertThat(target.books).containsExactlyInAnyOrder(book1, book3);
+	}
+
 	static class SingleRefRoot {
 
 		String id;
@@ -2249,4 +2276,40 @@ public class MongoTemplateDocumentReferenceTests {
 			return "MongoTemplateDocumentReferenceTests.WithListOfRefs(id=" + this.getId() + ", refs=" + this.getRefs() + ")";
 		}
 	}
+
+	static class OneToManyStylePublisherWithRequiredArgsCtor {
+
+		@Id
+		String id;
+
+		@ReadOnlyProperty
+		@DocumentReference(lookup="{'publisherId':?#{#self._id} }")
+		List<OneToManyStyleBook> books;
+
+		public OneToManyStylePublisherWithRequiredArgsCtor(String id, List<OneToManyStyleBook> books) {
+			this.id = id;
+			this.books = books;
+		}
+
+		public String getId() {
+			return this.id;
+		}
+
+		public List<OneToManyStyleBook> getBooks() {
+			return this.books;
+		}
+
+		public void setId(String id) {
+			this.id = id;
+		}
+
+		public void setBooks(List<OneToManyStyleBook> books) {
+			this.books = books;
+		}
+
+		public String toString() {
+			return "MongoTemplateDocumentReferenceTests.OneToManyStylePublisherWithRequiredArgsCtor(id=" + this.getId() + ", book="
+				+ this.getBooks() + ")";
+		}
+	}
 }

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

@@ -2346,6 +2346,24 @@ class MappingMongoConverterUnitTests {
 				.isEqualTo(expected);
 	}
 
+	@Test // GH-4491
+	void readUnwrappedTypeWithComplexValueUsingConstructor() {
+
+		org.bson.Document source = new org.bson.Document("_id", "id-1").append("stringValue", "hello").append("address",
+				new org.bson.Document("s", "1007 Mountain Drive").append("city", "Gotham"));
+
+		WithUnwrappedConstructor target = converter.read(WithUnwrappedConstructor.class, source);
+
+		Address expected = new Address();
+		expected.city = "Gotham";
+		expected.street = "1007 Mountain Drive";
+
+		assertThat(target.embeddableValue.stringValue) //
+				.isEqualTo("hello");
+		assertThat(target.embeddableValue.address) //
+				.isEqualTo(expected);
+	}
+
 	@Test // DATAMONGO-1902
 	void writeUnwrappedTypeWithComplexValue() {
 
@@ -3422,6 +3440,18 @@ class MappingMongoConverterUnitTests {
 		@Unwrapped.Nullable EmbeddableType embeddableValue;
 	}
 
+	static class WithUnwrappedConstructor {
+
+		private final String id;
+
+		private final @Unwrapped.Empty EmbeddableType embeddableValue;
+
+		public WithUnwrappedConstructor(String id, EmbeddableType embeddableValue) {
+			this.id = id;
+			this.embeddableValue = embeddableValue;
+		}
+	}
+
 	static class WithPrefixedNullableUnwrapped {
 
 		String id;

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

@@ -353,7 +353,7 @@ public abstract class AbstractEncryptionTestBase {
 		template.save(p3);
 
 		template.execute(Person.class, collection -> {
-			collection.find(new Document()).forEach(it -> System.out.println(it.toJson()));
+			collection.find(new Document());
 			return null;
 		});
 

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/observability/MongoObservationCommandListenerTests.java

@@ -16,6 +16,15 @@
 package org.springframework.data.mongodb.observability;
 
 import static io.micrometer.core.tck.MeterRegistryAssert.*;
+import static org.mockito.Mockito.*;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationRegistry;
+import io.micrometer.observation.contextpropagation.ObservationThreadLocalAccessor;
 
 import org.bson.BsonDocument;
 import org.bson.BsonString;
@@ -33,18 +42,12 @@ import com.mongodb.event.CommandFailedEvent;
 import com.mongodb.event.CommandStartedEvent;
 import com.mongodb.event.CommandSucceededEvent;
 
-import io.micrometer.common.KeyValues;
-import io.micrometer.core.instrument.MeterRegistry;
-import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler;
-import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
-import io.micrometer.observation.Observation;
-import io.micrometer.observation.ObservationRegistry;
-
 /**
  * Series of test cases exercising {@link MongoObservationCommandListener}.
  *
  * @author Marcin Grzejszczak
  * @author Greg Turnquist
+ * @author Mark Paluch
  */
 class MongoObservationCommandListenerTests {
 
@@ -176,6 +179,38 @@ class MongoObservationCommandListenerTests {
 		assertThatTimerRegisteredWithTags();
 	}
 
+	@Test // GH-4481
+	void completionShouldIgnoreIncompatibleObservationContext() {
+
+		// given
+		RequestContext traceRequestContext = getContext();
+
+		Observation observation = mock(Observation.class);
+		traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);
+
+		// when
+		listener.commandSucceeded(new CommandSucceededEvent(traceRequestContext, 0, null, "insert", null, 0));
+
+		verify(observation).getContext();
+		verifyNoMoreInteractions(observation);
+	}
+
+	@Test // GH-4481
+	void failureShouldIgnoreIncompatibleObservationContext() {
+
+		// given
+		RequestContext traceRequestContext = getContext();
+
+		Observation observation = mock(Observation.class);
+		traceRequestContext.put(ObservationThreadLocalAccessor.KEY, observation);
+
+		// when
+		listener.commandFailed(new CommandFailedEvent(traceRequestContext, 0, null, "insert", 0, null));
+
+		verify(observation).getContext();
+		verifyNoMoreInteractions(observation);
+	}
+
 	private RequestContext getContext() {
 		return ((SynchronousContextProvider) ContextProviderFactory.create(observationRegistry)).getContext();
 	}