Fix query/update document reference computation for non-id properties.

We now consider using the target keyword when computing document references. This fixes an issue where query/update statements had not been rendered correctly for references like { 'name' : ?#{#target} }.

Closes #3853
Original pull request: #3856.

Author: christophstrobl

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

@@ -151,6 +151,7 @@ static class LinkageDocument {
 		private final String lookup;
 		private final org.bson.Document documentPointer;
 		private final Map<String, String> placeholderMap;
+		private final boolean isSimpleTargetPointer;
 
 		static LinkageDocument from(String lookup) {
 			return new LinkageDocument(lookup);
@@ -177,6 +178,7 @@ private LinkageDocument(String lookup) {
 			}
 
 			this.documentPointer = org.bson.Document.parse(targetLookup);
+			this.isSimpleTargetPointer = placeholderMap.size() == 1 && placeholderMap.containsValue("target") && lookup.contains("#target");
 		}
 
 		private String placeholder(int index) {
@@ -194,7 +196,7 @@ DocumentPointer<Object> getDocumentPointer(
 					propertyAccessor);
 		}
 
-		Document updatePlaceholders(org.bson.Document source, org.bson.Document target,
+		Object updatePlaceholders(org.bson.Document source, org.bson.Document target,
 				MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext,
 				MongoPersistentEntity<?> persistentEntity, PersistentPropertyAccessor<?> propertyAccessor) {
 
@@ -245,6 +247,11 @@ Document updatePlaceholders(org.bson.Document source, org.bson.Document target,
 
 				target.put(entry.getKey(), entry.getValue());
 			}
+
+			if(target.size()==1 && isSimpleTargetPointer) {
+				return target.values().iterator().next();
+			}
+
 			return target;
 		}
 	}

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

@@ -517,6 +517,10 @@ protected boolean isAssociationConversionNecessary(Field documentField, @Nullabl
 			return true;
 		}
 
+		if(property.isDocumentReference()) {
+			return true;
+		}
+
 		MongoPersistentEntity<?> entity = documentField.getPropertyEntity();
 		return entity.hasIdProperty()
 				&& (type.equals(DBRef.class) || entity.getRequiredIdProperty().getActualType().isAssignableFrom(type));

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

@@ -50,6 +50,7 @@
 import org.springframework.data.mongodb.core.geo.GeoJsonPolygon;
 import org.springframework.data.mongodb.core.mapping.DBRef;
 import org.springframework.data.mongodb.core.mapping.Document;
+import org.springframework.data.mongodb.core.mapping.DocumentReference;
 import org.springframework.data.mongodb.core.mapping.Field;
 import org.springframework.data.mongodb.core.mapping.FieldType;
 import org.springframework.data.mongodb.core.mapping.MongoId;
@@ -424,6 +425,60 @@ void convertsNestedDBRefsCorrectly() {
 		assertThat(inClause.get(0)).isInstanceOf(com.mongodb.DBRef.class);
 	}
 
+	@Test // GH-3853
+	void convertsDocumentReferenceOnIdPropertyCorrectly() {
+
+		Sample reference = new Sample();
+		reference.foo = "s1";
+
+		Query query = query(where("sample").is(reference));
+		org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedQuery).containsEntry("sample", "s1");
+	}
+
+	@Test // GH-3853
+	void convertsListDocumentReferenceOnIdPropertyCorrectly() {
+
+		Sample reference = new Sample();
+		reference.foo = "s1";
+
+		Query query = query(where("samples").is(Arrays.asList(reference)));
+		org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedQuery).containsEntry("samples", Arrays.asList("s1"));
+	}
+
+	@Test // GH-3853
+	void convertsDocumentReferenceOnNonIdPropertyCorrectly() {
+
+		Customer reference = new Customer();
+		reference.id = new ObjectId();
+		reference.name = "c1";
+
+		Query query = query(where("customer").is(reference));
+		org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedQuery).containsEntry("customer", "c1");
+	}
+
+	@Test // GH-3853
+	void convertsListDocumentReferenceOnNonIdPropertyCorrectly() {
+
+		Customer reference = new Customer();
+		reference.id = new ObjectId();
+		reference.name = "c1";
+
+		Query query = query(where("customers").is(Arrays.asList(reference)));
+		org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedQuery).containsEntry("customers", Arrays.asList("c1"));
+	}
+
 	@Test // DATAMONGO-752
 	void mapsSimpleValuesStartingWith$Correctly() {
 
@@ -1496,6 +1551,25 @@ class WithMapDBRef {
 		@DBRef Map<String, Sample> mapWithDBRef;
 	}
 
+	static class WithDocumentReference {
+
+		private ObjectId id;
+
+		private String name;
+
+		@DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case.
+		private Customer customer;
+
+		@DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case.
+		private List<Customer> customers;
+
+		@DocumentReference
+		private Sample sample;
+
+		@DocumentReference
+		private List<Sample> samples;
+	}
+
 	class WithTextScoreProperty {
 
 		@Id String id;

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

@@ -31,6 +31,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.bson.Document;
+import org.bson.types.ObjectId;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -49,6 +50,7 @@
 import org.springframework.data.mongodb.MongoDatabaseFactory;
 import org.springframework.data.mongodb.core.DocumentTestUtils;
 import org.springframework.data.mongodb.core.MongoExceptionTranslator;
+import org.springframework.data.mongodb.core.mapping.DocumentReference;
 import org.springframework.data.mongodb.core.mapping.Field;
 import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
 import org.springframework.data.mongodb.core.mapping.Unwrapped;
@@ -1251,6 +1253,64 @@ void multipleKeysStartingWithANumberInNestedPath() {
 		assertThat(mappedUpdate).isEqualTo("{\"$set\": {\"intKeyedMap.1a.map.0b\": \"testing\"}}");
 	}
 
+	@Test // GH-3853
+	void updateWithDocuRefOnId() {
+
+		Sample sample = new Sample();
+		sample.foo = "s1";
+
+		Update update = new Update().set("sample", sample);
+
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("sample","s1")));
+	}
+
+	@Test // GH-3853
+	void updateListWithDocuRefOnId() {
+
+		Sample sample = new Sample();
+		sample.foo = "s1";
+
+		Update update = new Update().set("samples", Arrays.asList(sample));
+
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("samples",Arrays.asList("s1"))));
+	}
+
+	@Test // GH-3853
+	void updateWithDocuRefOnProperty() {
+
+		Customer customer = new Customer();
+		customer.id = new ObjectId();
+		customer.name = "c-name";
+
+		Update update = new Update().set("customer", customer);
+
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("customer","c-name")));
+	}
+
+	@Test // GH-3853
+	void updateListWithDocuRefOnProperty() {
+
+		Customer customer = new Customer();
+		customer.id = new ObjectId();
+		customer.name = "c-name";
+
+		Update update = new Update().set("customers", Arrays.asList(customer));
+
+		Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(),
+				context.getPersistentEntity(WithDocumentReference.class));
+
+		assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("customers", Arrays.asList("c-name"))));
+	}
+
 	static class DomainTypeWrappingConcreteyTypeHavingListOfInterfaceTypeAttributes {
 		ListModelWrapper concreteTypeWithListAttributeOfInterfaceType;
 	}
@@ -1621,4 +1681,35 @@ static class EntityWithNestedMap {
 		Map<String, Map<String, Map<String, Object>>> levelOne;
 	}
 
+	static class Customer {
+
+		@Id
+		private ObjectId id;
+		private String name;
+	}
+
+	static class Sample {
+
+		@Id private String foo;
+	}
+
+	static class WithDocumentReference {
+
+		private ObjectId id;
+
+		private String name;
+
+		@DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case.
+		private Customer customer;
+
+		@DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case.
+		private List<Customer> customers;
+
+		@DocumentReference
+		private Sample sample;
+
+		@DocumentReference
+		private List<Sample> samples;
+	}
+
 }