Fix reading id field value when multiple id property candidates exist

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>4.5.0-SNAPSHOT</version>
+	<version>4.5.x-GH-4877-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

spring-data-mongodb-distribution/pom.xml

@@ -15,7 +15,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.5.0-SNAPSHOT</version>
+		<version>4.5.x-GH-4877-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -13,7 +13,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>4.5.0-SNAPSHOT</version>
+		<version>4.5.x-GH-4877-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

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

@@ -348,7 +348,7 @@ private Class<?> computeTargetType(PersistentProperty<?> property) {
 			return property.getType();
 		}
 
-		if (!mongoProperty.isIdProperty()) {
+		if (!property.getOwner().isIdProperty(property)) {
 			return mongoProperty.getFieldType();
 		}
 

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

@@ -28,7 +28,6 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiPredicate;
-import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.apache.commons.logging.Log;
@@ -61,7 +60,6 @@
 import org.springframework.data.mapping.InstanceCreatorMetadata;
 import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.Parameter;
-import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.PersistentProperty;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.callback.EntityCallbacks;
@@ -134,7 +132,7 @@ public class MappingMongoConverter extends AbstractMongoConverter
 	private static final BiPredicate<MongoPersistentEntity<?>, MongoPersistentProperty> PROPERTY_FILTER = (e,
 			property) -> {
 
-		if (property.isIdProperty()) {
+		if (e.isIdProperty(property)) {
 			return false;
 		}
 
@@ -1929,14 +1927,6 @@ private static <T> T peek(Iterable<T> result) {
 		return result.iterator().next();
 	}
 
-	static Predicate<MongoPersistentProperty> isIdentifier(PersistentEntity<?, ?> entity) {
-		return entity::isIdProperty;
-	}
-
-	static Predicate<MongoPersistentProperty> isConstructorArgument(PersistentEntity<?, ?> entity) {
-		return entity::isCreatorArgument;
-	}
-
 	/**
 	 * {@link PropertyValueProvider} to evaluate a SpEL expression if present on the property or simply accesses the field
 	 * of the configured source {@link Document}.

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

@@ -487,7 +487,8 @@ protected Object getMappedValue(Field documentField, Object sourceValue) {
 	}
 
 	private boolean isIdField(Field documentField) {
-		return documentField.getProperty() != null && documentField.getProperty().isIdProperty();
+		return documentField.getProperty() != null
+				&& documentField.getProperty().getOwner().isIdProperty(documentField.getProperty());
 	}
 
 	private Class<?> getIdTypeForField(Field documentField) {
@@ -635,7 +636,8 @@ protected Object convertAssociation(@Nullable Object source, @Nullable MongoPers
 
 		if (source instanceof DBRef ref) {
 
-			Object id = convertId(ref.getId(), property.isIdProperty() ? property.getFieldType() : ObjectId.class);
+			Object id = convertId(ref.getId(),
+					property.getOwner().isIdProperty(property) ? property.getFieldType() : ObjectId.class);
 
 			if (StringUtils.hasText(ref.getDatabaseName())) {
 				return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id);
@@ -1187,7 +1189,7 @@ public MetadataBackedField with(String name) {
 		public boolean isIdField() {
 
 			if (property != null) {
-				return property.isIdProperty();
+				return property.getOwner().isIdProperty(property);
 			}
 
 			MongoPersistentProperty idProperty = entity.getIdProperty();
@@ -1317,7 +1319,7 @@ private PersistentPropertyPath<MongoPersistentProperty> getPath(String pathExpre
 					continue;
 				}
 
-				if (associationDetected && !property.isIdProperty()) {
+				if (associationDetected && !property.getOwner().isIdProperty(property)) {
 					throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression));
 				}
 			}

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

@@ -117,7 +117,7 @@ public Class<?> getFieldType() {
 
 		Field fieldAnnotation = findAnnotation(Field.class);
 
-		if (!isIdProperty()) {
+		if (!getOwner().isIdProperty(this)) {
 
 			if (fieldAnnotation == null || fieldAnnotation.targetType() == FieldType.IMPLICIT) {
 				return getType();

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java

@@ -135,13 +135,14 @@ protected String asDBKey(@Nullable Operation<?> expr, int index) {
 
 		MongoPersistentProperty property = getPropertyFor(path);
 
-		return property != null && property.isIdProperty() ? key.replaceAll("." + ID_KEY + "$", "") : key;
+		return property != null && property.getOwner().isIdProperty(property) ? key.replaceAll("." + ID_KEY + "$", "")
+				: key;
 	}
 
 	@Override
 	protected boolean isId(Path<?> arg) {
 		MongoPersistentProperty propertyFor = getPropertyFor(arg);
-		return propertyFor == null ? super.isId(arg) : propertyFor.isIdProperty();
+		return propertyFor == null ? super.isId(arg) : propertyFor.getOwner().isIdProperty(propertyFor);
 	}
 
 	@Override
@@ -159,7 +160,7 @@ protected Object convert(@Nullable Path<?> path, @Nullable Constant<?> constant)
 				return super.convert(path, constant);
 			}
 
-			if (property.isIdProperty()) {
+			if (property.getOwner().isIdProperty(property)) {
 				return mapper.convertId(constant.getConstant(), property.getFieldType());
 			}
 
@@ -177,7 +178,7 @@ protected Object convert(@Nullable Path<?> path, @Nullable Constant<?> constant)
 				return converter.toDocumentPointer(constant.getConstant(), property).getPointer();
 			}
 
-			if (property.isIdProperty()) {
+			if (property.getOwner().isIdProperty(property)) {
 
 				MongoPersistentProperty propertyForPotentialDbRef = getPropertyForPotentialDbRef(path);
 				if (propertyForPotentialDbRef != null && propertyForPotentialDbRef.isDocumentReference()) {
@@ -221,7 +222,8 @@ private MongoPersistentProperty getPropertyForPotentialDbRef(@Nullable Path<?> p
 		MongoPersistentProperty property = getPropertyFor(path);
 		PathMetadata metadata = path.getMetadata();
 
-		if (property != null && property.isIdProperty() && metadata != null && metadata.getParent() != null) {
+		if (property != null && property.getOwner().isIdProperty(property) && metadata != null
+				&& metadata.getParent() != null) {
 			return getPropertyFor(metadata.getParent());
 		}
 

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

@@ -3313,6 +3313,21 @@ void projectShouldReadComplexIdType(Class<?> projectionTargetType) {
 				.extracting("id").isEqualTo(idValue);
 	}
 
+	@Test // GH-4877
+	void shouldReadNonIdFieldCalledIdFromSource() {
+
+		WithRenamedIdPropertyAndAnotherPropertyNamedId source = new WithRenamedIdPropertyAndAnotherPropertyNamedId();
+		source.abc = "actual-id-value";
+		source.id = "just-a-field";
+
+		org.bson.Document document = write(source);
+		assertThat(document).containsEntry("_id", source.abc).containsEntry("id", source.id);
+
+		WithRenamedIdPropertyAndAnotherPropertyNamedId target = converter.read(WithRenamedIdPropertyAndAnotherPropertyNamedId.class, document);
+		assertThat(target.abc).isEqualTo(source.abc);
+		assertThat(target.id).isEqualTo(source.id);
+	}
+
 	org.bson.Document write(Object source) {
 
 		org.bson.Document target = new org.bson.Document();
@@ -4531,4 +4546,10 @@ public DoubleHolderDto(DoubleHolder number) {
 		}
 	}
 
+	static class WithRenamedIdPropertyAndAnotherPropertyNamedId {
+
+		@Id String abc;
+		String id;
+	}
+
 }

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

@@ -263,8 +263,12 @@ private static MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> e
 	}
 
 	private static MongoPersistentProperty getPropertyFor(MongoPersistentEntity<?> entity, Field field) {
-		return new BasicMongoPersistentProperty(Property.of(entity.getTypeInformation(), field), entity,
-				SimpleTypeHolder.DEFAULT, PropertyNameFieldNamingStrategy.INSTANCE);
+		BasicMongoPersistentProperty property = new BasicMongoPersistentProperty(
+				Property.of(entity.getTypeInformation(), field), entity, SimpleTypeHolder.DEFAULT,
+				PropertyNameFieldNamingStrategy.INSTANCE);
+
+		entity.addPersistentProperty(property);
+		return property;
 	}
 
 	class Person {
@@ -335,12 +339,14 @@ static class DocumentWithExplicitlyRenamedIdProperty {
 
 	static class DocumentWithExplicitlyRenamedIdPropertyHavingIdAnnotation {
 
-		@Id @org.springframework.data.mongodb.core.mapping.Field("id") String id;
+		@Id
+		@org.springframework.data.mongodb.core.mapping.Field("id") String id;
 	}
 
 	static class DocumentWithComposedAnnotations {
 
-		@ComposedIdAnnotation @ComposedFieldAnnotation String myId;
+		@ComposedIdAnnotation
+		@ComposedFieldAnnotation String myId;
 		@ComposedFieldAnnotation(name = "myField") String myField;
 	}
 
@@ -356,7 +362,8 @@ static class DocumentWithComposedAnnotations {
 	@Retention(RetentionPolicy.RUNTIME)
 	@Target(ElementType.FIELD)
 	@Id
-	static @interface ComposedIdAnnotation {}
+	static @interface ComposedIdAnnotation {
+	}
 
 	static class WithStringMongoId {
 
@@ -375,7 +382,8 @@ static class ComplexId {
 
 	static class WithComplexId {
 
-		@Id @org.springframework.data.mongodb.core.mapping.Field ComplexId id;
+		@Id
+		@org.springframework.data.mongodb.core.mapping.Field ComplexId id;
 	}
 
 	static class WithJMoleculesIdentity {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java

@@ -68,11 +68,11 @@
 public class SpringDataMongodbSerializerUnitTests {
 
 	@Mock DbRefResolver dbFactory;
-	MongoConverter converter;
-	SpringDataMongodbSerializer serializer;
+	private MongoConverter converter;
+	private SpringDataMongodbSerializer serializer;
 
 	@BeforeEach
-	public void setUp() {
+	void setUp() {
 
 		MongoMappingContext context = new MongoMappingContext();
 
@@ -81,21 +81,21 @@ public void setUp() {
 	}
 
 	@Test
-	public void uses_idAsKeyForIdProperty() {
+	void uses_idAsKeyForIdProperty() {
 
 		StringPath path = QPerson.person.id;
 		assertThat(serializer.getKeyForPath(path, path.getMetadata())).isEqualTo("_id");
 	}
 
 	@Test
-	public void buildsNestedKeyCorrectly() {
+	void buildsNestedKeyCorrectly() {
 
 		StringPath path = QPerson.person.address.street;
 		assertThat(serializer.getKeyForPath(path, path.getMetadata())).isEqualTo("street");
 	}
 
 	@Test
-	public void convertsComplexObjectOnSerializing() {
+	void convertsComplexObjectOnSerializing() {
 
 		Address address = new Address();
 		address.street = "Foo";
@@ -111,14 +111,14 @@ public void convertsComplexObjectOnSerializing() {
 	}
 
 	@Test // DATAMONGO-376
-	public void returnsEmptyStringIfNoPathExpressionIsGiven() {
+	void returnsEmptyStringIfNoPathExpressionIsGiven() {
 
 		QAddress address = QPerson.person.shippingAddresses.any();
 		assertThat(serializer.getKeyForPath(address, address.getMetadata())).isEmpty();
 	}
 
 	@Test // DATAMONGO-467, DATAMONGO-1798
-	public void appliesImplicitIdConversion() {
+	void appliesImplicitIdConversion() {
 
 		ObjectId id = new ObjectId();
 
@@ -130,7 +130,7 @@ public void appliesImplicitIdConversion() {
 	}
 
 	@Test // DATAMONGO-761
-	public void looksUpKeyForNonPropertyPath() {
+	void looksUpKeyForNonPropertyPath() {
 
 		PathBuilder<Address> builder = new PathBuilder<Address>(Address.class, "address");
 		SimplePath<Object> firstElementPath = builder.getArray("foo", String[].class).get(0);
@@ -140,7 +140,7 @@ public void looksUpKeyForNonPropertyPath() {
 	}
 
 	@Test // DATAMONGO-1485
-	public void takesCustomConversionForEnumsIntoAccount() {
+	void takesCustomConversionForEnumsIntoAccount() {
 
 		MongoMappingContext context = new MongoMappingContext();
 
@@ -158,7 +158,7 @@ public void takesCustomConversionForEnumsIntoAccount() {
 	}
 
 	@Test // DATAMONGO-1848, DATAMONGO-1943
-	public void shouldRemarshallListsAndDocuments() {
+	void shouldRemarshallListsAndDocuments() {
 
 		BooleanExpression criteria = QPerson.person.lastname.isNotEmpty()
 				.and(QPerson.person.firstname.containsIgnoreCase("foo")).not();
@@ -168,7 +168,7 @@ public void shouldRemarshallListsAndDocuments() {
 	}
 
 	@Test // DATAMONGO-2228
-	public void retainsOpsInAndExpression() {
+	void retainsOpsInAndExpression() {
 
 		PredicateOperation testExpression = predicate(Ops.AND,
 				predicate(Ops.OR, predicate(Ops.EQ, path(Object.class, "firstname"), constant("John")),
@@ -181,7 +181,7 @@ public void retainsOpsInAndExpression() {
 	}
 
 	@Test // DATAMONGO-2475
-	public void chainedOrsInSameDocument() {
+	void chainedOrsInSameDocument() {
 
 		Predicate predicate = QPerson.person.firstname.eq("firstname_value")
 				.or(QPerson.person.lastname.eq("lastname_value")).or(QPerson.person.age.goe(30)).or(QPerson.person.age.loe(20))
@@ -192,7 +192,7 @@ public void chainedOrsInSameDocument() {
 	}
 
 	@Test // DATAMONGO-2475
-	public void chainedNestedOrsInSameDocument() {
+	void chainedNestedOrsInSameDocument() {
 
 		Predicate predicate = QPerson.person.firstname.eq("firstname_value")
 				.or(QPerson.person.lastname.eq("lastname_value")).or(QPerson.person.address.street.eq("spring"));
@@ -202,7 +202,7 @@ public void chainedNestedOrsInSameDocument() {
 	}
 
 	@Test // DATAMONGO-2475
-	public void chainedAndsInSameDocument() {
+	void chainedAndsInSameDocument() {
 
 		Predicate predicate = QPerson.person.firstname.eq("firstname_value")
 				.and(QPerson.person.lastname.eq("lastname_value")).and(QPerson.person.age.goe(30))