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

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>1.7.0.BUILD-SNAPSHOT</version>
+	<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

spring-data-mongodb-cross-store/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>1.7.0.BUILD-SNAPSHOT</version>
+		<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 
@@ -48,7 +48,7 @@
 		<dependency>
 			<groupId>org.springframework.data</groupId>
 			<artifactId>spring-data-mongodb</artifactId>
-			<version>1.7.0.BUILD-SNAPSHOT</version>
+			<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 		</dependency>
 
 		<dependency>

spring-data-mongodb-distribution/pom.xml

@@ -13,7 +13,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>1.7.0.BUILD-SNAPSHOT</version>
+		<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb-log4j/pom.xml

@@ -5,7 +5,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>1.7.0.BUILD-SNAPSHOT</version>
+		<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -11,7 +11,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>1.7.0.BUILD-SNAPSHOT</version>
+		<version>1.7.0.DATAMONGO-1050-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

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;
@@ -1869,6 +1886,81 @@ public void readShouldRespectExplicitFieldNameForDbRef() {
 				Mockito.any(DbRefResolverCallback.class), Mockito.any(DbRefProxyHandler.class));
 	}
 
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void writeShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
+
+		RootForClassWithExplicitlyRenamedIdField source = new RootForClassWithExplicitlyRenamedIdField();
+		source.id = "rootId";
+		source.nested = new ClassWithExplicitlyRenamedField();
+		source.nested.id = "nestedId";
+
+		DBObject sink = new BasicDBObject();
+		converter.write(source, sink);
+
+		assertThat((String) sink.get("_id"), is("rootId"));
+		assertThat((DBObject) sink.get("nested"), is(new BasicDBObjectBuilder().add("id", "nestedId").get()));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void readShouldUseExplicitFieldnameForIdPropertyWhenAnnotated() {
+
+		DBObject source = new BasicDBObjectBuilder().add("_id", "rootId")
+				.add("nested", new BasicDBObject("id", "nestedId")).get();
+
+		RootForClassWithExplicitlyRenamedIdField sink = converter.read(RootForClassWithExplicitlyRenamedIdField.class,
+				source);
+
+		assertThat(sink.id, is("rootId"));
+		assertThat(sink.nested, notNullValue());
+		assertThat(sink.nested.id, is("nestedId"));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void namedIdFieldShouldExtractValueFromUnderscoreIdField() {
+
+		DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
+
+		ClassWithNamedIdField withNamedIdField = converter.read(ClassWithNamedIdField.class, dbo);
+
+		assertThat(withNamedIdField.id, is("A"));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void explicitlyRenamedIfFieldShouldExtractValueFromIdField() {
+
+		DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
+
+		ClassWithExplicitlyRenamedField withExplicitlyRenamedField = converter.read(ClassWithExplicitlyRenamedField.class,
+				dbo);
+
+		assertThat(withExplicitlyRenamedField.id, is("B"));
+	}
+
+	/**
+	 * @see DATAMONGO-1050
+	 */
+	@Test
+	public void annotatedIdFieldShouldExtractValueFromUnderscoreIdField() {
+
+		DBObject dbo = new BasicDBObjectBuilder().add("_id", "A").add("id", "B").get();
+
+		ClassWithAnnotatedIdField withAnnotatedIdField = converter.read(ClassWithAnnotatedIdField.class, dbo);
+
+		assertThat(withAnnotatedIdField.key, is("A"));
+	}
+
 	static class GenericType<T> {
 		T content;
 	}
@@ -2128,6 +2220,32 @@ class ClassWithExplicitlyNamedDBRefProperty {
 		public ClassWithIntId getDbRefProperty() {
 			return dbRefProperty;
 		}
+	}
+
+	static class RootForClassWithExplicitlyRenamedIdField {
+
+		@Id String id;
+		ClassWithExplicitlyRenamedField nested;
+	}
+
+	static class ClassWithExplicitlyRenamedField {
+
+		@Field("id") String id;
+	}
+
+	static class RootForClassWithNamedIdField {
+
+		String id;
+		ClassWithNamedIdField nested;
+	}
+
+	static class ClassWithNamedIdField {
+
+		String id;
+	}
+
+	static class ClassWithAnnotatedIdField {
 
+		@Id String key;
 	}
 }

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

@@ -659,6 +659,7 @@ public void getMappedSortIgnoresTextScoreWhenNotSortedByScore() {
 	}
 
 	/**
+<<<<<<< HEAD
 	 * @see DATAMONGO-1070
 	 */
 	@Test
@@ -673,6 +674,34 @@ public void mapsIdReferenceToDBRefCorrectly() {
 		com.mongodb.DBRef reference = getTypedValue(result, "reference", com.mongodb.DBRef.class);
 		assertThat(reference.getId(), is(instanceOf(ObjectId.class)));
 	}
+
+	/**
+	 * @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 {
@@ -755,4 +784,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;
+	}
 }