From 2d979fff287051b0b9e0f6ed497a859740bab28d Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 5 Oct 2021 13:56:08 +0200 Subject: [PATCH 1/2] Prepare issue branch. --- pom.xml | 2 +- spring-data-mongodb-benchmarks/pom.xml | 2 +- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 7cb1d10f85..f00898e8ab 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3853-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 0033bd11d5..b8e5e75730 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3853-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index f62c8dc7f4..95586ad5fb 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3853-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 2f73c10eba..7129b7181d 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -11,7 +11,7 @@ org.springframework.data spring-data-mongodb-parent - 3.3.0-SNAPSHOT + 3.3.0-GH-3853-SNAPSHOT ../pom.xml From ac3a80b708fb7b258f94e075acb10114b635c62b Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 5 Oct 2021 17:38:12 +0200 Subject: [PATCH 2/2] Fix query/update document reference computation for non id properties. We now consider using the target keyword when computing document references. This fixes an issue where query/update statements had not been rendered correctly for references like { 'name' : ?#{#target} }. --- .../core/convert/DocumentPointerFactory.java | 9 +- .../mongodb/core/convert/QueryMapper.java | 4 + .../core/convert/QueryMapperUnitTests.java | 74 +++++++++++++++ .../core/convert/UpdateMapperUnitTests.java | 91 +++++++++++++++++++ 4 files changed, 177 insertions(+), 1 deletion(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java index 2556e91304..45889049ec 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentPointerFactory.java @@ -151,6 +151,7 @@ static class LinkageDocument { private final String lookup; private final org.bson.Document documentPointer; private final Map placeholderMap; + private final boolean isSimpleTargetPointer; static LinkageDocument from(String lookup) { return new LinkageDocument(lookup); @@ -177,6 +178,7 @@ private LinkageDocument(String lookup) { } this.documentPointer = org.bson.Document.parse(targetLookup); + this.isSimpleTargetPointer = placeholderMap.size() == 1 && placeholderMap.containsValue("target") && lookup.contains("#target"); } private String placeholder(int index) { @@ -194,7 +196,7 @@ DocumentPointer getDocumentPointer( propertyAccessor); } - Document updatePlaceholders(org.bson.Document source, org.bson.Document target, + Object updatePlaceholders(org.bson.Document source, org.bson.Document target, MappingContext, MongoPersistentProperty> mappingContext, MongoPersistentEntity persistentEntity, PersistentPropertyAccessor propertyAccessor) { @@ -245,6 +247,11 @@ Document updatePlaceholders(org.bson.Document source, org.bson.Document target, target.put(entry.getKey(), entry.getValue()); } + + if(target.size()==1 && isSimpleTargetPointer) { + return target.values().iterator().next(); + } + return target; } } 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 69c85d6b67..051bbdc7be 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 @@ -517,6 +517,10 @@ protected boolean isAssociationConversionNecessary(Field documentField, @Nullabl return true; } + if(property.isDocumentReference()) { + return true; + } + MongoPersistentEntity entity = documentField.getPropertyEntity(); return entity.hasIdProperty() && (type.equals(DBRef.class) || entity.getRequiredIdProperty().getActualType().isAssignableFrom(type)); 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 11ea78fd4d..d102ef16d0 100755 --- 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 @@ -50,6 +50,7 @@ import org.springframework.data.mongodb.core.geo.GeoJsonPolygon; import org.springframework.data.mongodb.core.mapping.DBRef; import org.springframework.data.mongodb.core.mapping.Document; +import org.springframework.data.mongodb.core.mapping.DocumentReference; import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.FieldType; import org.springframework.data.mongodb.core.mapping.MongoId; @@ -424,6 +425,60 @@ void convertsNestedDBRefsCorrectly() { assertThat(inClause.get(0)).isInstanceOf(com.mongodb.DBRef.class); } + @Test // GH-3853 + void convertsDocumentReferenceOnIdPropertyCorrectly() { + + Sample reference = new Sample(); + reference.foo = "s1"; + + Query query = query(where("sample").is(reference)); + org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedQuery).containsEntry("sample", "s1"); + } + + @Test // GH-3853 + void convertsListDocumentReferenceOnIdPropertyCorrectly() { + + Sample reference = new Sample(); + reference.foo = "s1"; + + Query query = query(where("samples").is(Arrays.asList(reference))); + org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedQuery).containsEntry("samples", Arrays.asList("s1")); + } + + @Test // GH-3853 + void convertsDocumentReferenceOnNonIdPropertyCorrectly() { + + Customer reference = new Customer(); + reference.id = new ObjectId(); + reference.name = "c1"; + + Query query = query(where("customer").is(reference)); + org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedQuery).containsEntry("customer", "c1"); + } + + @Test // GH-3853 + void convertsListDocumentReferenceOnNonIdPropertyCorrectly() { + + Customer reference = new Customer(); + reference.id = new ObjectId(); + reference.name = "c1"; + + Query query = query(where("customers").is(Arrays.asList(reference))); + org.bson.Document mappedQuery = mapper.getMappedObject(query.getQueryObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedQuery).containsEntry("customers", Arrays.asList("c1")); + } + @Test // DATAMONGO-752 void mapsSimpleValuesStartingWith$Correctly() { @@ -1496,6 +1551,25 @@ class WithMapDBRef { @DBRef Map mapWithDBRef; } + static class WithDocumentReference { + + private ObjectId id; + + private String name; + + @DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case. + private Customer customer; + + @DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case. + private List customers; + + @DocumentReference + private Sample sample; + + @DocumentReference + private List samples; + } + class WithTextScoreProperty { @Id String id; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java index 44712fa8d1..c217cd5103 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java @@ -31,6 +31,7 @@ import java.util.concurrent.atomic.AtomicInteger; import org.bson.Document; +import org.bson.types.ObjectId; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -49,6 +50,7 @@ import org.springframework.data.mongodb.MongoDatabaseFactory; import org.springframework.data.mongodb.core.DocumentTestUtils; import org.springframework.data.mongodb.core.MongoExceptionTranslator; +import org.springframework.data.mongodb.core.mapping.DocumentReference; import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.Unwrapped; @@ -1251,6 +1253,64 @@ void multipleKeysStartingWithANumberInNestedPath() { assertThat(mappedUpdate).isEqualTo("{\"$set\": {\"intKeyedMap.1a.map.0b\": \"testing\"}}"); } + @Test // GH-3853 + void updateWithDocuRefOnId() { + + Sample sample = new Sample(); + sample.foo = "s1"; + + Update update = new Update().set("sample", sample); + + Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("sample","s1"))); + } + + @Test // GH-3853 + void updateListWithDocuRefOnId() { + + Sample sample = new Sample(); + sample.foo = "s1"; + + Update update = new Update().set("samples", Arrays.asList(sample)); + + Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("samples",Arrays.asList("s1")))); + } + + @Test // GH-3853 + void updateWithDocuRefOnProperty() { + + Customer customer = new Customer(); + customer.id = new ObjectId(); + customer.name = "c-name"; + + Update update = new Update().set("customer", customer); + + Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("customer","c-name"))); + } + + @Test // GH-3853 + void updateListWithDocuRefOnProperty() { + + Customer customer = new Customer(); + customer.id = new ObjectId(); + customer.name = "c-name"; + + Update update = new Update().set("customers", Arrays.asList(customer)); + + Document mappedUpdate = mapper.getMappedObject(update.getUpdateObject(), + context.getPersistentEntity(WithDocumentReference.class)); + + assertThat(mappedUpdate).isEqualTo(new org.bson.Document("$set",new org.bson.Document("customers", Arrays.asList("c-name")))); + } + static class DomainTypeWrappingConcreteyTypeHavingListOfInterfaceTypeAttributes { ListModelWrapper concreteTypeWithListAttributeOfInterfaceType; } @@ -1621,4 +1681,35 @@ static class EntityWithNestedMap { Map>> levelOne; } + static class Customer { + + @Id + private ObjectId id; + private String name; + } + + static class Sample { + + @Id private String foo; + } + + static class WithDocumentReference { + + private ObjectId id; + + private String name; + + @DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case. + private Customer customer; + + @DocumentReference(lookup = "{ 'name' : ?#{#target} }") // remove `lookup` for the other test case. + private List customers; + + @DocumentReference + private Sample sample; + + @DocumentReference + private List samples; + } + }