From c7e1ca58630684f220d573f9d2471241b860dea7 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 9 Nov 2020 09:48:26 +0100 Subject: [PATCH] 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. Original pull request: #890. --- .../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 36fc19a09..2cebf6531 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 @@ class AggregationUtil { } if (!(aggregation instanceof TypedAggregation)) { - return Aggregation.DEFAULT_CONTEXT; + return new RelaxedTypeBasedAggregationOperationContext(Object.class, mappingContext, queryMapper); } Class inputType = ((TypedAggregation) aggregation).getInputType(); @@ -98,7 +98,7 @@ class AggregationUtil { */ 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 438e53c5d..659db0811 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 @@ class QueryOperations { */ 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 64fe99f08..977ceb394 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 @@ package org.springframework.data.mongodb.core; 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 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati 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 aab2ffe5f..a3158e18d 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.ArrayList; 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.aggregation.ExposedFields.FieldRefe 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 class TypeBasedAggregationOperationContext implements AggregationOperatio this.type = type; this.mappingContext = mappingContext; this.mapper = mapper; + this.entity = Lazy.of(() -> mappingContext.getPersistentEntity(type)); } /* @@ -151,10 +155,14 @@ public class TypeBasedAggregationOperationContext implements AggregationOperatio 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 8bbb730b3..2ebddfd68 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 class UnionWithOperation implements AggregationOperation { 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 abc994a05..7eb7f37f3 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 @@ public class AggregationTests { 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 @@ public class AggregationTests { String p1; String p2; } + + static enum MyEnum { + ONE, TWO + } + + @lombok.Data + static class WithEnum { + + @Id String id; + MyEnum enumValue; + } }