From 9f77aba8bb7c38192a9dd3668bb488ed906119ba Mon Sep 17 00:00:00 2001
From: Christoph Strobl <cstrobl@pivotal.io>
Date: Wed, 30 May 2018 15:10:49 +0200
Subject: [PATCH] DATAMONGO-1988 - Fix query creation for id property
 references using ObjectId hex String representation.

We now follow the conversion rules for id properties with a valid ObjectId representation when creating queries. Prior to this change e.g. String values would have been turned into ObejctIds when saving a document, but not when querying the latter.

Original pull request: #565.
---
 .../mongodb/core/convert/QueryMapper.java     |  7 +++---
 .../data/mongodb/core/MongoTemplateTests.java | 24 +++++++++++++++++++
 .../core/convert/QueryMapperUnitTests.java    | 23 ++++++++++++++++--
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java
index daf5acd40..f19496c43 100644
--- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java
+++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java
@@ -312,7 +312,7 @@ public class QueryMapper {
 	@SuppressWarnings("unchecked")
 	protected Object getMappedValue(Field documentField, Object value) {
 
-		if (documentField.isIdField()) {
+		if (documentField.isIdField() && !documentField.isAssociation()) {
 
 			if (isDBObject(value)) {
 				DBObject valueDbo = (DBObject) value;
@@ -855,10 +855,11 @@ public class QueryMapper {
 		@Override
 		public boolean isIdField() {
 
-			MongoPersistentProperty idProperty = entity.getIdProperty();
+			MongoPersistentProperty idProperty = (property != null && property.isIdProperty()) ? property
+					: entity.getIdProperty();
 
 			if (idProperty != null) {
-				return idProperty.getName().equals(name) || idProperty.getFieldName().equals(name);
+				return name.endsWith(idProperty.getName()) || name.endsWith(idProperty.getFieldName());
 			}
 
 			return DEFAULT_ID_NAMES.contains(name);
diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java
index 9aa7990b5..a7fe1777b 100644
--- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java
+++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java
@@ -214,6 +214,7 @@ public class MongoTemplateTests {
 		template.dropCollection(Address.class);
 		template.dropCollection(DocumentWithCollectionOfSamples.class);
 		template.dropCollection(WithGeoJson.class);
+		template.dropCollection(DocumentWithNestedTypeHavingStringIdProperty.class);
 	}
 
 	@Test
@@ -3361,6 +3362,22 @@ public class MongoTemplateTests {
 		assertThat(template.count(query(where("field").is("stark")), Sample.class)).isEqualTo(0L);
 	}
 
+	@Test // DATAMONGO-1988
+	public void findByNestedDocumentWithStringIdMappingToObjectIdMatchesDocumentsCorrectly() {
+
+		DocumentWithNestedTypeHavingStringIdProperty source = new DocumentWithNestedTypeHavingStringIdProperty();
+		source.id = "id-1";
+		source.sample = new Sample();
+		source.sample.id = new ObjectId().toHexString();
+
+		template.save(source);
+
+		DocumentWithNestedTypeHavingStringIdProperty target = template.query(DocumentWithNestedTypeHavingStringIdProperty.class)
+				.matching(query(where("sample.id").is(source.sample.id))).firstValue();
+
+		assertThat(target).isEqualTo(source);
+	}
+
 	static class TypeWithNumbers {
 
 		@Id String id;
@@ -3462,6 +3479,13 @@ public class MongoTemplateTests {
 		List<Sample> samples;
 	}
 
+	@EqualsAndHashCode
+	static class DocumentWithNestedTypeHavingStringIdProperty {
+
+		@Id String id;
+		Sample sample;
+	}
+
 	static class DocumentWithMultipleCollections {
 		@Id String id;
 		List<String> string1;
diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java
index 740537cad..c5106b18d 100644
--- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java
+++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java
@@ -773,13 +773,32 @@ public class QueryMapperUnitTests {
 		assertThat(document.get("legacyPoint.y"), Is.<Object> is(20D));
 	}
 
+	@Test // DATAMONGO-1988
+	public void mapsStringObjectIdRepresentationToObjectIdWhenReferencingIdProperty() {
+
+		Query query = query(where("sample.foo").is(new ObjectId().toHexString()));
+		org.bson.Document document = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(ClassWithEmbedded.class));
+
+		assertThat(document.get("sample._id"), instanceOf(ObjectId.class));
+	}
+
+	@Test // DATAMONGO-1988
+	public void leavesNonObjectIdStringIdRepresentationUntouchedWhenReferencingIdProperty() {
+
+		Query query = query(where("sample.foo").is("id-1"));
+		org.bson.Document document = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(ClassWithEmbedded.class));
+
+		assertThat(document.get("sample._id"), instanceOf(String.class));
+	}
+
 	@Document
 	public class Foo {
 		@Id private ObjectId id;
 		EmbeddedClass embedded;
 
-		@Field("my_items")
-		List<EmbeddedClass> listOfItems;
+		@Field("my_items") List<EmbeddedClass> listOfItems;
 	}
 
 	public class EmbeddedClass {