DATAMONGO-1050 - Explicitly annotated Field should not be considered Id.

We changed the id resolution to skip properties having an explicit name set via @field unless they are marked with @id. This means that

@field(“id”) String id;

will be stored as “id” within mongodb. Prior to this change the fieldname would have been changed to “_id”.

Author: christophstrobl

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

@@ -100,7 +100,8 @@ public boolean isIdProperty() {
 		}
 
 		// We need to support a wider range of ID types than just the ones that can be converted to an ObjectId
-		return SUPPORTED_ID_PROPERTY_NAMES.contains(getName());
+		// but still we need to check if there happens to be an explicit name set
+		return SUPPORTED_ID_PROPERTY_NAMES.contains(getName()) && !hasExplicitFieldName();
 	}
 
 	/*
@@ -134,10 +135,8 @@ public String getFieldName() {
 			}
 		}
 
-		org.springframework.data.mongodb.core.mapping.Field annotation = findAnnotation(org.springframework.data.mongodb.core.mapping.Field.class);
-
-		if (annotation != null && StringUtils.hasText(annotation.value())) {
-			return annotation.value();
+		if (hasExplicitFieldName()) {
+			return getAnnotatedFieldName();
 		}
 
 		String fieldName = fieldNamingStrategy.getFieldName(this);
@@ -150,6 +149,26 @@ public String getFieldName() {
 		return fieldName;
 	}
 
+	/**
+	 * @return true if {@link org.springframework.data.mongodb.core.mapping.Field} having non blank
+	 *         {@link org.springframework.data.mongodb.core.mapping.Field#value()} present.
+	 * @since 1.7
+	 */
+	protected boolean hasExplicitFieldName() {
+		return StringUtils.hasText(getAnnotatedFieldName());
+	}
+
+	private String getAnnotatedFieldName() {
+
+		org.springframework.data.mongodb.core.mapping.Field annotation = findAnnotation(org.springframework.data.mongodb.core.mapping.Field.class);
+
+		if (annotation != null && StringUtils.hasText(annotation.value())) {
+			return annotation.value();
+		}
+
+		return null;
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mongodb.core.mapping.MongoPersistentProperty#getFieldOrder()

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

@@ -15,10 +15,26 @@
  */
 package org.springframework.data.mongodb.core.convert;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.mockito.Mockito.*;
-import static org.springframework.data.mongodb.core.DBObjectTestUtils.*;
+import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.springframework.data.mongodb.core.DBObjectTestUtils.getAsDBObject;
+import static org.springframework.data.mongodb.core.DBObjectTestUtils.getTypedValue;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -83,6 +99,7 @@
 
 import com.mongodb.BasicDBList;
 import com.mongodb.BasicDBObject;
+import com.mongodb.BasicDBObjectBuilder;
 import com.mongodb.DB;
 import com.mongodb.DBObject;
 import com.mongodb.DBRef;
@@ -1868,6 +1885,41 @@ public void readShouldRespectExplicitFieldNameForDbRef() {
 		verify(resolver, times(1)).resolveDbRef(Mockito.any(MongoPersistentProperty.class), Mockito.any(DBRef.class),
 				Mockito.any(DbRefResolverCallback.class), Mockito.any(DbRefProxyHandler.class));
 	}
+	
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void writeShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
+
+		RootForClassWithExplicitlyRenamedIdField source = new RootForClassWithExplicitlyRenamedIdField();
+		source.id = "root";
+		source.nested = new ClassWithExplicitlyRenamedField();
+		source.nested.id = "nested";
+
+		DBObject sink = new BasicDBObject();
+		converter.write(source, sink);
+
+		assertThat((String) sink.get("_id"), is("root"));
+		assertThat((DBObject) sink.get("nested"), is(new BasicDBObjectBuilder().add("id", "nested").get()));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void readShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
+
+		DBObject source = new BasicDBObjectBuilder().add("_id", "root").add("nested", new BasicDBObject("id", "nested"))
+				.get();
+
+		RootForClassWithExplicitlyRenamedIdField sink = converter.read(RootForClassWithExplicitlyRenamedIdField.class,
+				source);
+
+		assertThat(sink.id, is("root"));
+		assertThat(sink.nested, notNullValue());
+		assertThat(sink.nested.id, is("nested"));
+	}
 
 	static class GenericType<T> {
 		T content;
@@ -2128,6 +2180,16 @@ class ClassWithExplicitlyNamedDBRefProperty {
 		public ClassWithIntId getDbRefProperty() {
 			return dbRefProperty;
 		}
+	}
+
+	static class RootForClassWithExplicitlyRenamedIdField {
+
+		@Id String id;
+		ClassWithExplicitlyRenamedField nested;
+	}
+
+	static class ClassWithExplicitlyRenamedField {
 
+		@Field("id") String id;
 	}
 }

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

@@ -658,6 +658,34 @@ public void getMappedSortIgnoresTextScoreWhenNotSortedByScore() {
 		assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("_id", 1).get()));
 	}
 
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void shouldUseExplicitlySetFieldnameForIdPropertyCandidates() {
+
+		Query query = query(where("nested.id").is("bar"));
+
+		DBObject dbo = mapper.getMappedObject(query.getQueryObject(),
+				context.getPersistentEntity(RootForClassWithExplicitlyRenamedIdField.class));
+
+		assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("nested.id", "bar").get()));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void shouldUseExplicitlySetFieldnameForIdPropertyCandidatesUsedInSortClause() {
+
+		Query query = new Query().with(new Sort("nested.id"));
+
+		DBObject dbo = mapper.getMappedSort(query.getSortObject(),
+				context.getPersistentEntity(RootForClassWithExplicitlyRenamedIdField.class));
+
+		assertThat(dbo, equalTo(new BasicDBObjectBuilder().add("nested.id", 1).get()));
+	}
+
 	@Document
 	public class Foo {
 		@Id private ObjectId id;
@@ -739,4 +767,15 @@ class WithTextScoreProperty {
 		@Id String id;
 		@TextScore @Field("score") Float textScore;
 	}
+
+	static class RootForClassWithExplicitlyRenamedIdField {
+
+		@Id String id;
+		ClassWithExplicitlyRenamedField nested;
+	}
+
+	static class ClassWithExplicitlyRenamedField {
+
+		@Field("id") String id;
+	}
 }

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

@@ -130,10 +130,7 @@ public void rejectsInvalidValueReturnedByFieldNamingStrategy() {
 	@Test
 	public void shouldDetectAnnotatedLanguagePropertyCorrectly() {
 
-		BasicMongoPersistentEntity<DocumentWithLanguageProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithLanguageProperty>(
-				ClassTypeInformation.from(DocumentWithLanguageProperty.class));
-
-		MongoPersistentProperty property = getPropertyFor(persistentEntity, "lang");
+		MongoPersistentProperty property = getPropertyFor(DocumentWithLanguageProperty.class, "lang");
 		assertThat(property.isLanguageProperty(), is(true));
 	}
 
@@ -143,10 +140,7 @@ public void shouldDetectAnnotatedLanguagePropertyCorrectly() {
 	@Test
 	public void shouldDetectIplicitLanguagePropertyCorrectly() {
 
-		BasicMongoPersistentEntity<DocumentWithImplicitLanguageProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithImplicitLanguageProperty>(
-				ClassTypeInformation.from(DocumentWithImplicitLanguageProperty.class));
-
-		MongoPersistentProperty property = getPropertyFor(persistentEntity, "language");
+		MongoPersistentProperty property = getPropertyFor(DocumentWithImplicitLanguageProperty.class, "language");
 		assertThat(property.isLanguageProperty(), is(true));
 	}
 
@@ -156,10 +150,7 @@ public void shouldDetectIplicitLanguagePropertyCorrectly() {
 	@Test
 	public void shouldDetectTextScorePropertyCorrectly() {
 
-		BasicMongoPersistentEntity<DocumentWithTextScoreProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithTextScoreProperty>(
-				ClassTypeInformation.from(DocumentWithTextScoreProperty.class));
-
-		MongoPersistentProperty property = getPropertyFor(persistentEntity, "score");
+		MongoPersistentProperty property = getPropertyFor(DocumentWithTextScoreProperty.class, "score");
 		assertThat(property.isTextScoreProperty(), is(true));
 	}
 
@@ -169,17 +160,39 @@ public void shouldDetectTextScorePropertyCorrectly() {
 	@Test
 	public void shouldDetectTextScoreAsReadOnlyProperty() {
 
-		BasicMongoPersistentEntity<DocumentWithTextScoreProperty> persistentEntity = new BasicMongoPersistentEntity<DocumentWithTextScoreProperty>(
-				ClassTypeInformation.from(DocumentWithTextScoreProperty.class));
-
-		MongoPersistentProperty property = getPropertyFor(persistentEntity, "score");
+		MongoPersistentProperty property = getPropertyFor(DocumentWithTextScoreProperty.class, "score");
 		assertThat(property.isWritable(), is(false));
 	}
 
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void shouldNotConsiderExplicitlyNameFieldAsIdProperty() {
+
+		MongoPersistentProperty property = getPropertyFor(DocumentWithExplicitlyRenamedIdProperty.class, "id");
+		assertThat(property.isIdProperty(), is(false));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void shouldConsiderPropertyAsIdWhenExplicitlyAnnotatedWithIdEvenWhenExplicitlyNamePresent() {
+
+		MongoPersistentProperty property = getPropertyFor(DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation.class,
+				"id");
+		assertThat(property.isIdProperty(), is(true));
+	}
+
 	private MongoPersistentProperty getPropertyFor(Field field) {
 		return getPropertyFor(entity, field);
 	}
 
+	private <T> MongoPersistentProperty getPropertyFor(Class<T> type, String fieldname) {
+		return getPropertyFor(new BasicMongoPersistentEntity<T>(ClassTypeInformation.from(type)), fieldname);
+	}
+
 	private MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> persistentEntity, String fieldname) {
 		return getPropertyFor(persistentEntity, ReflectionUtils.findField(persistentEntity.getType(), fieldname));
 	}
@@ -230,4 +243,14 @@ static class DocumentWithImplicitLanguageProperty {
 	static class DocumentWithTextScoreProperty {
 		@TextScore Float score;
 	}
+
+	static class DocumentWithExplicitlyRenamedIdProperty {
+
+		@org.springframework.data.mongodb.core.mapping.Field("id") String id;
+	}
+
+	static class DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation {
+
+		@Id @org.springframework.data.mongodb.core.mapping.Field("id") String id;
+	}
 }