diff --git a/pom.xml b/pom.xml index de66da1866..7a5ae7e1b5 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-4722-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index a3dc49f892..d50e04dd55 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-4722-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index e33930bfd2..3e03f8ddef 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-4722-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index fafe9c8793..67dee32271 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-4722-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java index ea29f751de..c15eaaf85a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java @@ -62,7 +62,7 @@ static List toDocument(List operations, Aggregat if (operation instanceof InheritsFieldsAggregationOperation || exposedFieldsOperation.inheritsFields()) { contextToUse = contextToUse.inheritAndExpose(fields); } else { - contextToUse = fields.exposesNoFields() ? DEFAULT_CONTEXT + contextToUse = fields.exposesNoFields() ? ConverterAwareNoOpContext.instance(rootContext) : contextToUse.expose(fields); } } @@ -72,6 +72,39 @@ static List toDocument(List operations, Aggregat return operationDocuments; } + private static class ConverterAwareNoOpContext implements AggregationOperationContext { + + AggregationOperationContext ctx; + + static ConverterAwareNoOpContext instance(AggregationOperationContext ctx) { + + if(ctx instanceof ConverterAwareNoOpContext noOpContext) { + return noOpContext; + } + + return new ConverterAwareNoOpContext(ctx); + } + + ConverterAwareNoOpContext(AggregationOperationContext ctx) { + this.ctx = ctx; + } + + @Override + public Document getMappedObject(Document document, @Nullable Class type) { + return ctx.getMappedObject(document, null); + } + + @Override + public FieldReference getReference(Field field) { + return new DirectFieldReference(new ExposedField(field, true)); + } + + @Override + public FieldReference getReference(String name) { + return new DirectFieldReference(new ExposedField(new AggregationField(name), true)); + } + } + /** * Simple {@link AggregationOperationContext} that just returns {@link FieldReference}s as is. * diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRendererUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRendererUnitTests.java index a8b32f957e..792a86249a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRendererUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRendererUnitTests.java @@ -15,18 +15,35 @@ */ package org.springframework.data.mongodb.core.aggregation; -import static org.mockito.Mockito.*; -import static org.springframework.data.domain.Sort.Direction.*; -import static org.springframework.data.mongodb.core.aggregation.Aggregation.*; - +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.springframework.data.domain.Sort.Direction.DESC; +import static org.springframework.data.mongodb.core.aggregation.Aggregation.newAggregation; +import static org.springframework.data.mongodb.core.aggregation.Aggregation.project; +import static org.springframework.data.mongodb.core.aggregation.Aggregation.sort; + +import java.time.ZonedDateTime; import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.assertj.core.api.Assertions; +import org.bson.Document; import org.junit.jupiter.api.Test; - +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.data.annotation.Id; +import org.springframework.data.convert.ConverterBuilder; +import org.springframework.data.convert.CustomConversions; +import org.springframework.data.convert.CustomConversions.StoreConversions; +import org.springframework.data.domain.Sort.Direction; 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.Field; +import org.springframework.data.mongodb.core.query.Criteria; import org.springframework.data.mongodb.test.util.MongoTestMappingContext; /** @@ -47,6 +64,79 @@ void nonFieldsExposingAggregationOperationContinuesWithSameContextForNextStage() verify(stage2).toPipelineStages(eq(rootContext)); } + @Test + void contextShouldCarryOnRelaxedFieldMapping() { + + MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> { + cfg.initialEntitySet(TestRecord.class); + }); + + MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx); + + Aggregation agg = Aggregation.newAggregation(Aggregation.unwind("layerOne.layerTwo"), + project().and("layerOne.layerTwo.layerThree").as("layerOne.layerThree"), + sort(DESC, "layerOne.layerThree.fieldA")); + + AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(), + new RelaxedTypeBasedAggregationOperationContext(TestRecord.class, ctx, new QueryMapper(mongoConverter))); + } + + @Test // GH-4722 + void appliesConversionToValuesUsedInAggregation() { + + MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> { + cfg.initialEntitySet(TestRecord.class); + }); + + MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx); + mongoConverter.setCustomConversions(new CustomConversions(StoreConversions.NONE, + Set.copyOf(ConverterBuilder.writing(ZonedDateTime.class, String.class, ZonedDateTime::toString) + .andReading(it -> ZonedDateTime.parse(it)).getConverters()))); + mongoConverter.afterPropertiesSet(); + + var agg = Aggregation.newAggregation(Aggregation.sort(Direction.DESC, "version"), + Aggregation.group("entityId").first(Aggregation.ROOT).as("value"), Aggregation.replaceRoot("value"), + Aggregation.match(Criteria.where("createdDate").lt(ZonedDateTime.now())) // here is the problem + ); + + List document = AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(), + new RelaxedTypeBasedAggregationOperationContext(TestRecord.class, ctx, new QueryMapper(mongoConverter))); + Assertions.assertThat(document).last() + .extracting(it -> it.getEmbedded(List.of("$match", "createdDate", "$lt"), Object.class)) + .isInstanceOf(String.class); + } + + @ParameterizedTest // GH-4722 + @MethodSource("studentAggregationContexts") + void mapsOperationThatDoesNotExposeDedicatedFieldsCorrectly(AggregationOperationContext aggregationContext) { + + var agg = newAggregation(Student.class, Aggregation.unwind("grades"), Aggregation.replaceRoot("grades"), + Aggregation.project("grades")); + + List mappedPipeline = AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(), + aggregationContext); + + Assertions.assertThat(mappedPipeline).last().isEqualTo(Document.parse("{\"$project\": {\"grades\": 1}}")); + } + + private static Stream studentAggregationContexts() { + + MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> { + cfg.initialEntitySet(Student.class); + }); + + MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx); + mongoConverter.afterPropertiesSet(); + + QueryMapper queryMapper = new QueryMapper(mongoConverter); + + return Stream.of( + Arguments + .of(new TypeBasedAggregationOperationContext(Student.class, ctx, queryMapper, FieldLookupPolicy.strict())), + Arguments.of( + new TypeBasedAggregationOperationContext(Student.class, ctx, queryMapper, FieldLookupPolicy.relaxed()))); + } + record TestRecord(@Id String field1, String field2, LayerOne layerOne) { record LayerOne(List layerTwo) { } @@ -54,25 +144,20 @@ record LayerOne(List layerTwo) { record LayerTwo(LayerThree layerThree) { } - record LayerThree(int fieldA, int fieldB) - {} + record LayerThree(int fieldA, int fieldB) { + } } - @Test - void xxx() { + static class Student { - MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> { - cfg.initialEntitySet(TestRecord.class); - }); + @Field("mark") List grades; - MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx); + } - Aggregation agg = Aggregation.newAggregation( - Aggregation.unwind("layerOne.layerTwo"), - project().and("layerOne.layerTwo.layerThree").as("layerOne.layerThree"), - sort(DESC, "layerOne.layerThree.fieldA") - ); + static class Grade { - AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(), new RelaxedTypeBasedAggregationOperationContext(TestRecord.class, ctx, new QueryMapper(mongoConverter))); + int points; + String grades; } + }