Correctly read unwrapped properties during constructor creation

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>4.2.0-SNAPSHOT</version>
+	<version>4.2.0-GH-4491-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

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-SNAPSHOT</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-SNAPSHOT</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-SNAPSHOT</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 @@
 
 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.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;
@@ -492,7 +507,6 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
 		S instance = instantiator.createInstance(entity, provider);
 
 		if (entity.requiresPropertyPopulation()) {
-
 			return populateProperties(context, entity, documentAccessor, evaluator, instance);
 		}
 
@@ -571,14 +585,18 @@ private void readProperties(ConversionContext context, MongoPersistentEntity<?>
 			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;
 			}
 
@@ -593,17 +611,6 @@ private void readProperties(ConversionContext context, MongoPersistentEntity<?>
 				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));
 		}
 	}
@@ -615,9 +622,10 @@ private DbRefResolverCallback getDbRefResolverCallback(ConversionContext context
 				(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);
@@ -630,30 +638,27 @@ private void readAssociation(Association<MongoPersistentProperty> association, P
 			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);
 		}
 
 		/*
@@ -664,18 +669,18 @@ private void readAssociation(Association<MongoPersistentProperty> association, P
 		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);
 		}
 	}
 
@@ -1961,25 +1966,26 @@ class AssociationAwareMongoDbPropertyValueProvider extends MongoDbPropertyValueP
 		@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.isUnwrapped()) {
+
+				return (T) readUnwrapped(propertyContext, accessor, property,
+						mappingContext.getRequiredPersistentEntity(property));
 			}
 
-			if (property.isDocumentReference()) {
-				return (T) dbRefResolver.resolveReference(property,
-					new DocumentReferenceSource(accessor.getDocument(), accessor.get(property)),
-					referenceLookupDelegate, context::convert);
+			if (!accessor.hasValue(property)) {
+				return null;
 			}
 
 			return super.getPropertyValue(property);

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

@@ -2346,6 +2346,24 @@ void readUnwrappedTypeWithComplexValue() {
 				.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 @@ static class WithNullableUnwrapped {
 		@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 @@ void altKeyDetection(@Autowired CachingMongoClientEncryption mongoClientEncrypti
 		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;
 		});