diff --git a/pom.xml b/pom.xml index 7e3b0b9432..4e4108d3d9 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.5.0-SNAPSHOT + 4.5.x-GH-4877-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 58c63dfc97..bd97cd888a 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.5.0-SNAPSHOT + 4.5.x-GH-4877-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 98516a5ba9..ea233306d2 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 4.5.0-SNAPSHOT + 4.5.x-GH-4877-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappingMongoJsonSchemaCreator.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappingMongoJsonSchemaCreator.java index 11d5e8f0df..6bf8343ab1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MappingMongoJsonSchemaCreator.java +++ b/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(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 35b1d77792..864cc1c3e3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/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, MongoPersistentProperty> PROPERTY_FILTER = (e, property) -> { - if (property.isIdProperty()) { + if (e.isIdProperty(property)) { return false; } @@ -1929,14 +1927,6 @@ private static T peek(Iterable result) { return result.iterator().next(); } - static Predicate isIdentifier(PersistentEntity entity) { - return entity::isIdProperty; - } - - static Predicate 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}. 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 516d83ffa6..39559b9979 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 @@ -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 getPath(String pathExpre continue; } - if (associationDetected && !property.isIdProperty()) { + if (associationDetected && !property.getOwner().isIdProperty(property)) { throw new MappingException(String.format(INVALID_ASSOCIATION_REFERENCE, pathExpression)); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java index 78bba2308d..5c3b4e6532 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentProperty.java +++ b/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(); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java index f5722364df..d9a550a0f7 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializer.java +++ b/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()); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index 2536d28291..f44e094705 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/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; + } + } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java index 86c9ec44ea..116505143e 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/BasicMongoPersistentPropertyUnitTests.java +++ b/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 { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java index 471b9f0671..56e17b7590 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SpringDataMongodbSerializerUnitTests.java +++ b/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
builder = new PathBuilder
(Address.class, "address"); SimplePath 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))