From e40f3bf3a44b6cf9813a07a648ff22a1272e2281 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 29 Oct 2020 13:53:23 +0100 Subject: [PATCH 1/3] DATAMONGO-2635 - 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 99721676cb..8c745b598f 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 3.2.0-SNAPSHOT + 3.2.0-DATAMONGO-2635-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index f0fbb601c8..781fb7f7ed 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.2.0-SNAPSHOT + 3.2.0-DATAMONGO-2635-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 1a17321782..793782255d 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.2.0-SNAPSHOT + 3.2.0-DATAMONGO-2635-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 0248517caf..220b13b226 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.2.0-SNAPSHOT + 3.2.0-DATAMONGO-2635-SNAPSHOT ../pom.xml From fa9e34f7a92e8589740952fa8fe051b9c01c57a2 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 9 Nov 2020 09:48:26 +0100 Subject: [PATCH 2/3] DATAMONGO-2635 - Enforce aggregation pipeline mapping. Avoid using the Aggregation.DEFAULT_CONTEXT which does not map contained values to the according MongoDB representation. We now use a relaxed aggregation context, preserving given field names, where possible. --- .../data/mongodb/core/AggregationUtil.java | 4 +-- .../data/mongodb/core/QueryOperations.java | 5 ++-- .../mongodb/core/ReactiveMongoTemplate.java | 3 ++- .../TypeBasedAggregationOperationContext.java | 12 +++++++-- .../core/aggregation/UnionWithOperation.java | 6 +---- .../core/aggregation/AggregationTests.java | 27 +++++++++++++++++++ 6 files changed, 44 insertions(+), 13 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/AggregationUtil.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/AggregationUtil.java index 36fc19a098..2cebf65312 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/AggregationUtil.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/AggregationUtil.java @@ -77,7 +77,7 @@ AggregationOperationContext prepareAggregationContext(Aggregation aggregation, } if (!(aggregation instanceof TypedAggregation)) { - return Aggregation.DEFAULT_CONTEXT; + return new RelaxedTypeBasedAggregationOperationContext(Object.class, mappingContext, queryMapper); } Class inputType = ((TypedAggregation) aggregation).getInputType(); @@ -98,7 +98,7 @@ AggregationOperationContext prepareAggregationContext(Aggregation aggregation, */ List createPipeline(Aggregation aggregation, AggregationOperationContext context) { - if (!ObjectUtils.nullSafeEquals(context, Aggregation.DEFAULT_CONTEXT)) { + if (ObjectUtils.nullSafeEquals(context, Aggregation.DEFAULT_CONTEXT)) { return aggregation.toPipeline(context); } 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 438e53c5dc..659db0811f 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 @@ -707,10 +707,9 @@ Document getMappedShardKey(MongoPersistentEntity entity) { */ List getUpdatePipeline(@Nullable Class domainType) { - AggregationOperationContext context = domainType != null - ? new RelaxedTypeBasedAggregationOperationContext(domainType, mappingContext, queryMapper) - : Aggregation.DEFAULT_CONTEXT; + Class type = domainType != null ? domainType : Object.class; + AggregationOperationContext context = new RelaxedTypeBasedAggregationOperationContext(type, mappingContext, queryMapper); return aggregationUtil.createPipeline((AggregationUpdate) update, context); } 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 64fe99f08a..977ceb3944 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 @@ -17,6 +17,7 @@ import static org.springframework.data.mongodb.core.query.SerializationUtils.*; +import org.springframework.data.mongodb.core.aggregation.RelaxedTypeBasedAggregationOperationContext; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; @@ -2112,7 +2113,7 @@ List prepareFilter(ChangeStreamOptions options) { AggregationOperationContext context = agg instanceof TypedAggregation ? new TypeBasedAggregationOperationContext(((TypedAggregation) agg).getInputType(), getConverter().getMappingContext(), queryMapper) - : Aggregation.DEFAULT_CONTEXT; + : new RelaxedTypeBasedAggregationOperationContext(Object.class, mappingContext, queryMapper); return agg.toPipeline(new PrefixingDelegatingAggregationOperationContext(context, "fullDocument", Arrays.asList("operationType", "fullDocument", "documentKey", "updateDescription", "ns"))); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java index aab2ffe5f5..a3158e18d5 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContext.java @@ -21,6 +21,7 @@ import java.util.List; import org.bson.Document; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.aggregation.ExposedFields.DirectFieldReference; @@ -29,6 +30,7 @@ import org.springframework.data.mongodb.core.convert.QueryMapper; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; +import org.springframework.data.util.Lazy; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -46,6 +48,7 @@ public class TypeBasedAggregationOperationContext implements AggregationOperatio private final Class type; private final MappingContext, MongoPersistentProperty> mappingContext; private final QueryMapper mapper; + private final Lazy> entity; /** * Creates a new {@link TypeBasedAggregationOperationContext} for the given type, {@link MappingContext} and @@ -65,6 +68,7 @@ public TypeBasedAggregationOperationContext(Class type, this.type = type; this.mappingContext = mappingContext; this.mapper = mapper; + this.entity = Lazy.of(() -> mappingContext.getPersistentEntity(type)); } /* @@ -151,10 +155,14 @@ public AggregationOperationContext continueOnMissingFieldReference(Class type protected FieldReference getReferenceFor(Field field) { + if(entity.getNullable() == null) { + return new DirectFieldReference(new ExposedField(field, true)); + } + PersistentPropertyPath propertyPath = mappingContext - .getPersistentPropertyPath(field.getTarget(), type); + .getPersistentPropertyPath(field.getTarget(), type); Field mappedField = field(field.getName(), - propertyPath.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE)); + propertyPath.toDotPath(MongoPersistentProperty.PropertyToFieldNameConverter.INSTANCE)); return new DirectFieldReference(new ExposedField(mappedField, true)); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/UnionWithOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/UnionWithOperation.java index 8bbb730b3d..2ebddfd68e 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/UnionWithOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/UnionWithOperation.java @@ -142,12 +142,8 @@ public Document toDocument(AggregationOperationContext context) { private AggregationOperationContext computeContext(AggregationOperationContext source) { - if (domainType == null) { - return Aggregation.DEFAULT_CONTEXT; - } - if (source instanceof TypeBasedAggregationOperationContext) { - return ((TypeBasedAggregationOperationContext) source).continueOnMissingFieldReference(domainType); + return ((TypeBasedAggregationOperationContext) source).continueOnMissingFieldReference(domainType != null ? domainType : Object.class); } if (source instanceof ExposedFieldsAggregationOperationContext) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java index abc994a050..7eb7f37f33 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java @@ -1928,6 +1928,22 @@ void skipOutputDoesNotReadBackAggregationResults() { assertThat(results.getRawResults()).isEmpty(); } + @Test // DATAMONGO-2635 + void mapsEnumsInMatchClauseUsingInCriteriaCorrectly() { + + WithEnum source = new WithEnum(); + source.enumValue = MyEnum.TWO; + source.id = "id-1"; + + mongoTemplate.save(source); + + Aggregation agg = newAggregation(match(where("enumValue").in(Collections.singletonList(MyEnum.TWO)))); + + AggregationResults results = mongoTemplate.aggregate(agg, mongoTemplate.getCollectionName(WithEnum.class), + Document.class); + assertThat(results.getMappedResults()).hasSize(1); + } + private void createUsersWithReferencedPersons() { mongoTemplate.dropCollection(User.class); @@ -2240,4 +2256,15 @@ static class ComplexId { String p1; String p2; } + + static enum MyEnum { + ONE, TWO + } + + @lombok.Data + static class WithEnum { + + @Id String id; + MyEnum enumValue; + } } From 3bc7556cfd34fa3aa556ffe43579c01070a3b7dd Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 9 Nov 2020 11:15:54 +0100 Subject: [PATCH 3/3] DATAMONGO-2644 - ProjectOperation no longer errors on inclusion of default _id field. --- .../core/aggregation/ProjectionOperation.java | 8 ++++++++ .../core/aggregation/AggregationUnitTests.java | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java index 21278041ae..bc683dbc7f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java @@ -1450,6 +1450,14 @@ private Object renderFieldValue(AggregationOperationContext context) { return field.getTarget(); } + if(field.getTarget().equals(Fields.UNDERSCORE_ID)) { + try { + return context.getReference(field).getReferenceValue(); + } catch (java.lang.IllegalArgumentException e) { + return Fields.UNDERSCORE_ID_REF; + } + } + // check whether referenced field exists in the context return context.getReference(field).getReferenceValue(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java index e51932abe7..37dc8727f7 100755 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java @@ -29,6 +29,11 @@ import org.junit.jupiter.api.Test; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.mongodb.core.aggregation.ConditionalOperators.Cond; +import org.springframework.data.mongodb.core.aggregation.ProjectionOperationUnitTests.BookWithFieldAnnotation; +import org.springframework.data.mongodb.core.convert.MappingMongoConverter; +import org.springframework.data.mongodb.core.convert.NoOpDbRefResolver; +import org.springframework.data.mongodb.core.convert.QueryMapper; +import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.query.Criteria; /** @@ -582,6 +587,17 @@ void shouldAllowInternalThisAndValueReferences() { "{\"attributeRecordArrays\": {\"$reduce\": {\"input\": \"$attributeRecordArrays\", \"initialValue\": [], \"in\": {\"$concatArrays\": [\"$$value\", \"$$this\"]}}}}")); } + @Test // DATAMONGO-2644 + void projectOnIdIsAlwaysValid() { + + MongoMappingContext mappingContext = new MongoMappingContext(); + Document target = new Aggregation(bucket("start"), project("_id")).toDocument("collection-1", + new RelaxedTypeBasedAggregationOperationContext(BookWithFieldAnnotation.class, mappingContext, + new QueryMapper(new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, mappingContext)))); + + assertThat(extractPipelineElement(target, 1, "$project")).isEqualTo(Document.parse(" { \"_id\" : \"$_id\" }")); + } + private Document extractPipelineElement(Document agg, int index, String operation) { List pipeline = (List) agg.get("pipeline");