From d03b1222e900e0a6965a3a0c0e1d31ea8902dbbc Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 10 Jun 2024 16:15:37 +0200 Subject: [PATCH 1/4] 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 de66da1866..e40df2d1e5 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.4.0-SNAPSHOT + 4.4.x-GH-4707-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index a3dc49f892..6a01c82946 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 - 4.4.0-SNAPSHOT + 4.4.x-GH-4707-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index e33930bfd2..497cad96e5 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.4.0-SNAPSHOT + 4.4.x-GH-4707-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index fafe9c8793..9b4d7d6bd5 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.4.0-SNAPSHOT + 4.4.x-GH-4707-SNAPSHOT ../pom.xml From 5b350f70649ac8a17717ebdd1a9fc8219d5c9177 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 10 Jun 2024 16:24:02 +0200 Subject: [PATCH 2/4] Fix id conversion for fields of complex id. This commit fixes an issue where the property type for nested fields of an complex id is not handed over correctly leading to wrong conversion results eg. for Instant types that got then turned into ObjectIds. --- .../data/mongodb/core/convert/QueryMapper.java | 7 ++++++- .../data/mongodb/core/MongoTemplateTests.java | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) 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 bba8054cc1..a67a56dc11 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 @@ -728,7 +728,12 @@ private Object convertIdField(Field documentField, Object source) { } else if (isKeyword(key)) { resultDbo.put(key, convertIdField(documentField, entry.getValue())); } else { - resultDbo.put(key, getMappedValue(documentField, entry.getValue())); + if(documentField.getProperty() != null && documentField.getProperty().isEntity()) { + Field propertyField = createPropertyField(documentField.getPropertyEntity(), key, mappingContext); + resultDbo.put(key, getMappedValue(propertyField, entry.getValue())); + } else { + resultDbo.put(key, getMappedValue(documentField, entry.getValue())); + } } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index a1543aba86..9753664298 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -1409,12 +1409,13 @@ public void executesQueryWithNegatedRegexCorrectly() { assertThat(result.get(0).field).isEqualTo("Beauford"); } - @Test // DATAMONGO-447 + @Test // DATAMONGO-447, GH-4707 public void storesAndRemovesTypeWithComplexId() { MyId id = new MyId(); id.first = "foo"; id.second = "bar"; + id.time = Instant.now().minusSeconds(2); TypeWithMyId source = new TypeWithMyId(); source.id = id; @@ -4397,6 +4398,7 @@ static class MyId { String first; String second; + Instant time; } static class TypeWithMyId { From 389a92c2a537084f4dd401d3690b0bcdb404fc34 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 11 Jun 2024 14:46:49 +0200 Subject: [PATCH 3/4] Polishing. Avoid duplicate query mapping for document replacement operations when the filter query can be determined from the already mapped _id field. --- .../data/mongodb/core/MongoTemplate.java | 4 +--- .../data/mongodb/core/QueryOperations.java | 16 +++++++++++++--- .../data/mongodb/core/ReactiveMongoTemplate.java | 2 +- .../data/mongodb/core/MongoTemplateTests.java | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index ba59d86056..33bf89e970 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -1603,9 +1603,7 @@ protected Object saveDocument(String collectionName, Document dbDoc, Class en MongoPersistentEntity entity = mappingContext.getPersistentEntity(entityClass); UpdateContext updateContext = queryOperations.replaceSingleContext(mapped, true); Document replacement = updateContext.getMappedUpdate(entity); - - Document filter = updateContext.getMappedQuery(entity); - + Document filter = updateContext.getReplacementQuery(); if (updateContext.requiresShardKey(filter, entity)) { if (entity.getShardKey().isImmutable()) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java index fabe52f3a3..9a862cf2c0 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryOperations.java @@ -400,7 +400,7 @@ private Document evaluateFields(@Nullable MongoPersistentEntity entity) { for (Entry entry : fields.entrySet()) { - if (entry.getValue()instanceof MongoExpression mongoExpression) { + if (entry.getValue() instanceof MongoExpression mongoExpression) { AggregationOperationContext ctx = entity == null ? Aggregation.DEFAULT_CONTEXT : new RelaxedTypeBasedAggregationOperationContext(entity.getType(), mappingContext, queryMapper); @@ -809,13 +809,23 @@ ReplaceOptions getReplaceOptions(@Nullable Class domainType, @Nullable Consum @Override Document getMappedQuery(@Nullable MongoPersistentEntity domainType) { + return applyIsolation(super.getMappedQuery(domainType)); + } - Document mappedQuery = super.getMappedQuery(domainType); + /** + * A replacement query that is derived from the already {@link MappedDocument}. + * + * @return + */ + Document getReplacementQuery() { + return applyIsolation(getQueryObject()); + } + private Document applyIsolation(Document mappedQuery) { if (multi && update != null && update.isIsolated() && !mappedQuery.containsKey("$isolated")) { + mappedQuery = new Document(mappedQuery); mappedQuery.put("$isolated", 1); } - return mappedQuery; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index 99dfea85e3..10ae7a9ead 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -1649,7 +1649,7 @@ protected Mono saveDocument(String collectionName, Document document, Cl MongoPersistentEntity entity = mappingContext.getPersistentEntity(entityClass); UpdateContext updateContext = queryOperations.replaceSingleContext(mapped, true); - Document filter = updateContext.getMappedQuery(entity); + Document filter = updateContext.getReplacementQuery(); Document replacement = updateContext.getMappedUpdate(entity); Mono deferredFilter; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 9753664298..934fe91e53 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -1415,7 +1415,7 @@ public void storesAndRemovesTypeWithComplexId() { MyId id = new MyId(); id.first = "foo"; id.second = "bar"; - id.time = Instant.now().minusSeconds(2); + id.id = Instant.now().minusSeconds(2); TypeWithMyId source = new TypeWithMyId(); source.id = id; @@ -4398,7 +4398,7 @@ static class MyId { String first; String second; - Instant time; + Instant id; } static class TypeWithMyId { From b002774e83a7c0ba475742f8085e00026c0acaff Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 12 Jun 2024 10:39:41 +0200 Subject: [PATCH 4/4] Additional tests for using complex ids. --- .../data/mongodb/core/MongoTemplateTests.java | 32 +++++++++++++++++-- .../mongodb/test/util/MongoTestTemplate.java | 9 +++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 934fe91e53..ecee7cd15d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -1413,15 +1413,17 @@ public void executesQueryWithNegatedRegexCorrectly() { public void storesAndRemovesTypeWithComplexId() { MyId id = new MyId(); + id.id = Instant.now().minusSeconds(2); id.first = "foo"; id.second = "bar"; - id.id = Instant.now().minusSeconds(2); + id.time = Instant.now().minusSeconds(3); TypeWithMyId source = new TypeWithMyId(); source.id = id; template.save(source); - template.remove(query(where("id").is(id)), TypeWithMyId.class); + assertThat(template.remove(query(where("id").is(id)), TypeWithMyId.class)).extracting(DeleteResult::getDeletedCount) + .isEqualTo(1L); } @Test // DATAMONGO-506 @@ -2555,6 +2557,29 @@ public void findAndReplaceShouldProjectReturnedObjectCorrectly() { assertThat(projection.getName()).isEqualTo("Walter"); } + @Test // GH-4707 + public void findAndReplaceUpsertsObjectWithComplexId() { + + MyId id = new MyId(); + id.id = Instant.now().minusSeconds(2); + id.first = "foo"; + id.second = "bar"; + id.time = Instant.now().minusSeconds(3); + + TypeWithMyId replacement = new TypeWithMyId(); + replacement.value = "spring"; + + template.findAndReplace(query(where("id").is(id)), replacement, FindAndReplaceOptions.options().upsert()); + template.doInCollection(TypeWithMyId.class, collection -> { + + org.bson.Document dbValue = collection.find(new org.bson.Document("_id.first", "foo")).first(); + + assertThat(dbValue).isNotNull(); + assertThat(dbValue.getEmbedded(List.of("_id", "_id"), Object.class)).isInstanceOf(Date.class); + assertThat(dbValue.getEmbedded(List.of("_id", "t"), Object.class)).isInstanceOf(Date.class); + }); + } + @Test // GH-4609 public void shouldReadNestedProjection() { @@ -4399,11 +4424,14 @@ static class MyId { String first; String second; Instant id; + + @Field("t") Instant time; } static class TypeWithMyId { @Id MyId id; + String value; } static class Sample { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplate.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplate.java index bab535cb56..1b4c3a1e27 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplate.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/test/util/MongoTestTemplate.java @@ -25,11 +25,11 @@ import org.springframework.data.mapping.callback.EntityCallbacks; import org.springframework.data.mapping.context.PersistentEntities; import org.springframework.data.mongodb.core.MongoTemplate; +import org.springframework.data.mongodb.util.MongoCompatibilityAdapter; import com.mongodb.MongoWriteException; import com.mongodb.client.MongoClient; import com.mongodb.client.MongoCollection; -import org.springframework.data.mongodb.util.MongoCompatibilityAdapter; /** * A {@link MongoTemplate} with configuration hooks and extension suitable for tests. @@ -147,4 +147,11 @@ public void dropIndexes(Class... entities) { getCollection(getCollectionName(entity)).dropIndexes(); } } + + public void doInCollection(Class entityClass, Consumer> callback) { + execute(entityClass, (collection -> { + callback.accept(collection); + return null; + })); + } }