Fix non association mapping when id value matches already resolved instance.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>4.0.0-SNAPSHOT</version>
+	<version>4.0.x-GH-4098-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.0.0-SNAPSHOT</version>
+		<version>4.0.x-GH-4098-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.0.0-SNAPSHOT</version>
+		<version>4.0.x-GH-4098-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -12,7 +12,7 @@
   <parent>
     <groupId>org.springframework.data</groupId>
     <artifactId>spring-data-mongodb-parent</artifactId>
-    <version>4.0.0-SNAPSHOT</version>
+    <version>4.0.x-GH-4098-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

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

@@ -39,7 +39,6 @@
 import org.bson.conversions.Bson;
 import org.bson.json.JsonReader;
 import org.bson.types.ObjectId;
-
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.context.ApplicationContext;
@@ -188,7 +187,7 @@ protected ConversionContext getConversionContext(ObjectPath path) {
 
 		Assert.notNull(path, "ObjectPath must not be null");
 
-		return new ConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray,
+		return new DefaultConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray,
 				this::readMap, this::readDBRef, this::getPotentiallyConvertedSimpleRead);
 	}
 
@@ -385,7 +384,46 @@ private Object doReadOrProject(ConversionContext context, Bson source, TypeInfor
 		return readDocument(context, source, typeHint);
 	}
 
-	class ProjectingConversionContext extends ConversionContext {
+	static class AssociationConversionContext implements ConversionContext {
+
+		private final ConversionContext delegate;
+
+		public AssociationConversionContext(ConversionContext delegate) {
+			this.delegate = delegate;
+		}
+
+		@Override
+		public <S> S convert(Object source, TypeInformation<? extends S> typeHint, ConversionContext context) {
+			return delegate.convert(source, typeHint, context);
+		}
+
+		@Override
+		public ConversionContext withPath(ObjectPath currentPath) {
+			return new AssociationConversionContext(delegate.withPath(currentPath));
+		}
+
+		@Override
+		public ObjectPath getPath() {
+			return delegate.getPath();
+		}
+
+		@Override
+		public CustomConversions getCustomConversions() {
+			return delegate.getCustomConversions();
+		}
+
+		@Override
+		public MongoConverter getSourceConverter() {
+			return delegate.getSourceConverter();
+		}
+
+		@Override
+		public boolean resolveIdsInContext() {
+			return true;
+		}
+	}
+
+	class ProjectingConversionContext extends DefaultConversionContext {
 
 		private final EntityProjection<?, ?> returnedTypeDescriptor;
 
@@ -401,20 +439,21 @@ class ProjectingConversionContext extends ConversionContext {
 		}
 
 		@Override
-		public ConversionContext forProperty(String name) {
+		public DefaultConversionContext forProperty(String name) {
 
 			EntityProjection<?, ?> property = returnedTypeDescriptor.findProperty(name);
 			if (property == null) {
-				return new ConversionContext(sourceConverter, conversions, path, MappingMongoConverter.this::readDocument,
-						collectionConverter, mapConverter, dbRefConverter, elementConverter);
+				return new DefaultConversionContext(sourceConverter, conversions, path,
+						MappingMongoConverter.this::readDocument, collectionConverter, mapConverter, dbRefConverter,
+						elementConverter);
 			}
 
 			return new ProjectingConversionContext(sourceConverter, conversions, path, collectionConverter, mapConverter,
 					dbRefConverter, elementConverter, property);
 		}
 
 		@Override
-		public ConversionContext withPath(ObjectPath currentPath) {
+		public DefaultConversionContext withPath(ObjectPath currentPath) {
 			return new ProjectingConversionContext(sourceConverter, conversions, currentPath, collectionConverter,
 					mapConverter, dbRefConverter, elementConverter, returnedTypeDescriptor);
 		}
@@ -529,7 +568,7 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
 		SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(bson, spELContext);
 		DocumentAccessor documentAccessor = new DocumentAccessor(bson);
 
-		if (hasIdentifier(bson)) {
+		if (context.resolveIdsInContext() && hasIdentifier(bson)) {
 			S existing = findContextualEntity(context, entity, bson);
 			if (existing != null) {
 				return existing;
@@ -632,7 +671,7 @@ private void readProperties(ConversionContext context, MongoPersistentEntity<?>
 				continue;
 			}
 
-			ConversionContext propertyContext = context.forProperty(prop.getName());
+			ConversionContext propertyContext = context.forProperty(prop);
 			MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext);
 
 			if (prop.isAssociation() && !entity.isConstructorArgument(prop)) {
@@ -706,7 +745,7 @@ private void readAssociation(Association<MongoPersistentProperty> association, P
 				accessor.setProperty(property,
 						dbRefResolver.resolveReference(property,
 								new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
-								referenceLookupDelegate, context::convert));
+								referenceLookupDelegate, context.forProperty(property)::convert));
 			}
 			return;
 		}
@@ -1935,13 +1974,13 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
 				return null;
 			}
 
-			CustomConversions conversions = context.conversions;
+			CustomConversions conversions = context.getCustomConversions();
 			if (conversions.hasValueConverter(property)) {
 				return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value,
-						new MongoConversionContext(property, context.sourceConverter));
+						new MongoConversionContext(property, context.getSourceConverter()));
 			}
 
-			ConversionContext contextToUse = context.forProperty(property.getName());
+			ConversionContext contextToUse = context.forProperty(property);
 
 			return (T) contextToUse.convert(value, property.getTypeInformation());
 		}
@@ -2158,13 +2197,49 @@ public TypeDescriptor toTypeDescriptor() {
 		}
 	}
 
+	interface ConversionContext {
+
+		default <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {
+			return convert(source, typeHint, this);
+		}
+
+		<S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint, ConversionContext context);
+
+		ConversionContext withPath(ObjectPath currentPath);
+
+		ObjectPath getPath();
+
+		default ConversionContext forProperty(String name) {
+			return this;
+		}
+
+		default ConversionContext forProperty(@Nullable PersistentProperty property) {
+
+			if (property != null) {
+				if (property.isAssociation()) {
+					return new AssociationConversionContext(forProperty(property.getName()));
+				}
+				return forProperty(property.getName());
+			}
+			return this;
+		}
+
+		default boolean resolveIdsInContext() {
+			return false;
+		}
+
+		CustomConversions getCustomConversions();
+
+		MongoConverter getSourceConverter();
+	}
+
 	/**
 	 * Conversion context holding references to simple {@link ValueConverter} and {@link ContainerValueConverter}.
 	 * Entrypoint for recursive conversion of {@link Document} and other types.
 	 *
 	 * @since 3.2
 	 */
-	protected static class ConversionContext {
+	protected static class DefaultConversionContext implements ConversionContext {
 
 		final MongoConverter sourceConverter;
 		final org.springframework.data.convert.CustomConversions conversions;
@@ -2175,7 +2250,7 @@ protected static class ConversionContext {
 		final ContainerValueConverter<DBRef> dbRefConverter;
 		final ValueConverter<Object> elementConverter;
 
-		ConversionContext(MongoConverter sourceConverter,
+		DefaultConversionContext(MongoConverter sourceConverter,
 				org.springframework.data.convert.CustomConversions customConversions, ObjectPath path,
 				ContainerValueConverter<Bson> documentConverter, ContainerValueConverter<Collection<?>> collectionConverter,
 				ContainerValueConverter<Bson> mapConverter, ContainerValueConverter<DBRef> dbRefConverter,
@@ -2199,7 +2274,8 @@ protected static class ConversionContext {
 		 * @return the converted object.
 		 */
 		@SuppressWarnings("unchecked")
-		public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {
+		public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint,
+				ConversionContext context) {
 
 			Assert.notNull(source, "Source must not be null");
 			Assert.notNull(typeHint, "TypeInformation must not be null");
@@ -2219,26 +2295,26 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
 				}
 
 				if (typeHint.isCollectionLike() || typeHint.getType().isAssignableFrom(Collection.class)) {
-					return (S) collectionConverter.convert(this, (Collection<?>) source, typeHint);
+					return (S) collectionConverter.convert(context, (Collection<?>) source, typeHint);
 				}
 			}
 
 			if (typeHint.isMap()) {
 
 				if (ClassUtils.isAssignable(Document.class, typeHint.getType())) {
-					return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint);
+					return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint);
 				}
 
 				if (BsonUtils.supportsBson(source)) {
-					return (S) mapConverter.convert(this, BsonUtils.asBson(source), typeHint);
+					return (S) mapConverter.convert(context, BsonUtils.asBson(source), typeHint);
 				}
 
 				throw new IllegalArgumentException(
 						String.format("Expected map like structure but found %s", source.getClass()));
 			}
 
 			if (source instanceof DBRef) {
-				return (S) dbRefConverter.convert(this, (DBRef) source, typeHint);
+				return (S) dbRefConverter.convert(context, (DBRef) source, typeHint);
 			}
 
 			if (source instanceof Collection) {
@@ -2247,31 +2323,41 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
 			}
 
 			if (BsonUtils.supportsBson(source)) {
-				return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint);
+				return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint);
 			}
 
 			return (S) elementConverter.convert(source, typeHint);
 		}
 
+		@Override
+		public CustomConversions getCustomConversions() {
+			return conversions;
+		}
+
+		@Override
+		public MongoConverter getSourceConverter() {
+			return sourceConverter;
+		}
+
 		/**
-		 * Create a new {@link ConversionContext} with {@link ObjectPath currentPath} applied.
+		 * Create a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied.
 		 *
 		 * @param currentPath must not be {@literal null}.
-		 * @return a new {@link ConversionContext} with {@link ObjectPath currentPath} applied.
+		 * @return a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied.
 		 */
-		public ConversionContext withPath(ObjectPath currentPath) {
+		public DefaultConversionContext withPath(ObjectPath currentPath) {
 
 			Assert.notNull(currentPath, "ObjectPath must not be null");
 
-			return new ConversionContext(sourceConverter, conversions, currentPath, documentConverter, collectionConverter,
-					mapConverter, dbRefConverter, elementConverter);
+			return new DefaultConversionContext(sourceConverter, conversions, currentPath, documentConverter,
+					collectionConverter, mapConverter, dbRefConverter, elementConverter);
 		}
 
 		public ObjectPath getPath() {
 			return path;
 		}
 
-		public ConversionContext forProperty(String name) {
+		public DefaultConversionContext forProperty(String name) {
 			return this;
 		}
 

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

@@ -45,7 +45,6 @@
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
-
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.beans.ConversionNotSupportedException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -2823,6 +2822,15 @@ public org.bson.Document write(@Nullable String domainValue, MongoConversionCont
 		assertThat(read.viaRegisteredConverter).isEqualTo("spring");
 	}
 
+	@Test // GH-4098
+	void resolvesCyclicNonAssociationValueFromSource/* and does not attempt to be smart and look up id values in context */() {
+
+		org.bson.Document source = new org.bson.Document("_id", "id-1").append("value", "v1").append("cycle",
+				new org.bson.Document("_id", "id-1").append("value", "v2"));
+
+		assertThat(converter.read(Cyclic.class, source).cycle.value).isEqualTo("v2");
+	}
+
 	static class GenericType<T> {
 		T content;
 	}
@@ -3546,12 +3554,10 @@ static class WithFieldWrite {
 		@org.springframework.data.mongodb.core.mapping.Field(
 				write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways;
 
-		@org.springframework.data.mongodb.core.mapping.DBRef
-		@org.springframework.data.mongodb.core.mapping.Field(
+		@org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field(
 				write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
 
-		@org.springframework.data.mongodb.core.mapping.DBRef
-		@org.springframework.data.mongodb.core.mapping.Field(
+		@org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field(
 				write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
 
 	}
@@ -3665,4 +3671,12 @@ static class Author {
 
 	}
 
+	@Data
+	static class Cyclic {
+
+		@Id String id;
+		String value;
+		Cyclic cycle;
+	}
+
 }