From bc5db1854807dc45782eb4fde6d210273886d7c3 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 22 Mar 2023 09:19:29 +0100 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 d864b2e4e6..b27221bf53 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.1.0-SNAPSHOT + 4.1.x-GH-4088-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 1b2a1390e6..7c153eae2c 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.1.0-SNAPSHOT + 4.1.x-GH-4088-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 8db8d798fb..eaa339a110 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.1.0-SNAPSHOT + 4.1.x-GH-4088-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 597ca94f38..d0c69baaaf 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.1.0-SNAPSHOT + 4.1.x-GH-4088-SNAPSHOT ../pom.xml From 286b2b4393ac7eaaff200e57fff7796994421b96 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 22 Mar 2023 10:17:30 +0100 Subject: [PATCH 2/2] SkipOutput for void methods using declarative Aggregations having $out stage. We now set the skipOutput flag if an annotated Aggregation defines an $out stage and when the method is declared to return no result (void / Mono, kotlin.Unit) Closes: #4088 --- .../repository/query/AbstractMongoQuery.java | 1 + .../repository/query/AggregationUtils.java | 7 +++-- .../repository/query/MongoQueryExecution.java | 2 ++ .../query/ReactiveStringBasedAggregation.java | 16 ++++++++--- .../query/StringBasedAggregation.java | 25 +++++++++++++---- ...activeStringBasedAggregationUnitTests.java | 28 +++++++++++++++++++ .../StringBasedAggregationUnitTests.java | 28 +++++++++++++++++++ .../mongo-repositories-aggregation.adoc | 7 +++++ 8 files changed, 101 insertions(+), 13 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java index 930a733315..e45254a2c5 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java @@ -127,6 +127,7 @@ public Object execute(Object[] parameters) { * @param accessor for providing invocation arguments. Never {@literal null}. * @param typeToRead the desired component target type. Can be {@literal null}. */ + @Nullable protected Object doExecute(MongoQueryMethod method, ResultProcessor processor, ConvertingParameterAccessor accessor, @Nullable Class typeToRead) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AggregationUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AggregationUtils.java index 0ee488c336..4df4f63832 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AggregationUtils.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AggregationUtils.java @@ -27,6 +27,7 @@ import org.springframework.data.mongodb.core.aggregation.Aggregation; import org.springframework.data.mongodb.core.aggregation.AggregationOperation; import org.springframework.data.mongodb.core.aggregation.AggregationOptions; +import org.springframework.data.mongodb.core.aggregation.AggregationPipeline; import org.springframework.data.mongodb.core.convert.MongoConverter; import org.springframework.data.mongodb.core.query.Collation; import org.springframework.data.mongodb.core.query.Meta; @@ -109,7 +110,7 @@ static AggregationOptions.Builder applyMeta(AggregationOptions.Builder builder, * @param accessor * @param targetType */ - static void appendSortIfPresent(List aggregationPipeline, ConvertingParameterAccessor accessor, + static void appendSortIfPresent(AggregationPipeline aggregationPipeline, ConvertingParameterAccessor accessor, Class targetType) { if (accessor.getSort().isUnsorted()) { @@ -134,7 +135,7 @@ static void appendSortIfPresent(List aggregationPipeline, * @param aggregationPipeline * @param accessor */ - static void appendLimitAndOffsetIfPresent(List aggregationPipeline, + static void appendLimitAndOffsetIfPresent(AggregationPipeline aggregationPipeline, ConvertingParameterAccessor accessor) { appendLimitAndOffsetIfPresent(aggregationPipeline, accessor, LongUnaryOperator.identity(), IntUnaryOperator.identity()); @@ -150,7 +151,7 @@ static void appendLimitAndOffsetIfPresent(List aggregation * @param limitOperator * @since 3.3 */ - static void appendLimitAndOffsetIfPresent(List aggregationPipeline, + static void appendLimitAndOffsetIfPresent(AggregationPipeline aggregationPipeline, ConvertingParameterAccessor accessor, LongUnaryOperator offsetOperator, IntUnaryOperator limitOperator) { Pageable pageable = accessor.getPageable(); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryExecution.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryExecution.java index dca76ff7fb..a512fd6e06 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryExecution.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryExecution.java @@ -38,6 +38,7 @@ import org.springframework.data.mongodb.core.query.UpdateDefinition; import org.springframework.data.support.PageableExecutionUtils; import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -55,6 +56,7 @@ @FunctionalInterface interface MongoQueryExecution { + @Nullable Object execute(Query query); /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java index 13c49b5219..151c3b0ae0 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java @@ -15,6 +15,8 @@ */ package org.springframework.data.mongodb.repository.query; +import org.springframework.data.mongodb.core.aggregation.AggregationPipeline; +import org.springframework.data.util.ReflectionUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -81,7 +83,7 @@ protected Publisher doExecute(ReactiveMongoQueryMethod method, ResultPro Class sourceType = method.getDomainClass(); Class targetType = typeToRead; - List pipeline = it; + AggregationPipeline pipeline = new AggregationPipeline(it); AggregationUtils.appendSortIfPresent(pipeline, accessor, typeToRead); AggregationUtils.appendLimitAndOffsetIfPresent(pipeline, accessor); @@ -93,10 +95,13 @@ protected Publisher doExecute(ReactiveMongoQueryMethod method, ResultPro targetType = Document.class; } - AggregationOptions options = computeOptions(method, accessor); - TypedAggregation aggregation = new TypedAggregation<>(sourceType, pipeline, options); + AggregationOptions options = computeOptions(method, accessor, pipeline); + TypedAggregation aggregation = new TypedAggregation<>(sourceType, pipeline.getOperations(), options); Flux flux = reactiveMongoOperations.aggregate(aggregation, targetType); + if(ReflectionUtils.isVoid(typeToRead)) { + return flux.then(); + } if (isSimpleReturnType && !isRawReturnType) { flux = flux.handle((item, sink) -> { @@ -121,13 +126,16 @@ private Mono> computePipeline(ConvertingParameterAcce return parseAggregationPipeline(getQueryMethod().getAnnotatedAggregation(), accessor); } - private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingParameterAccessor accessor) { + private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingParameterAccessor accessor, AggregationPipeline pipeline) { AggregationOptions.Builder builder = Aggregation.newAggregationOptions(); AggregationUtils.applyCollation(builder, method.getAnnotatedCollation(), accessor, method.getParameters(), expressionParser, evaluationContextProvider); AggregationUtils.applyMeta(builder, method); + if(ReflectionUtils.isVoid(method.getReturnType().getComponentType().getType()) && pipeline.isOutOrMerge()) { + builder.skipOutput(); + } return builder.build(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedAggregation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedAggregation.java index a0ee58fd25..6b48e3ef0f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedAggregation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedAggregation.java @@ -29,6 +29,7 @@ import org.springframework.data.mongodb.core.aggregation.Aggregation; import org.springframework.data.mongodb.core.aggregation.AggregationOperation; import org.springframework.data.mongodb.core.aggregation.AggregationOptions; +import org.springframework.data.mongodb.core.aggregation.AggregationPipeline; import org.springframework.data.mongodb.core.aggregation.AggregationResults; import org.springframework.data.mongodb.core.aggregation.TypedAggregation; import org.springframework.data.mongodb.core.convert.MongoConverter; @@ -36,8 +37,12 @@ import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.ResultProcessor; +import org.springframework.data.util.ReflectionUtils; import org.springframework.expression.ExpressionParser; +import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; +import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; /** * {@link AbstractMongoQuery} implementation to run string-based aggregations using @@ -84,13 +89,14 @@ public StringBasedAggregation(MongoQueryMethod method, MongoOperations mongoOper * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#doExecute(org.springframework.data.mongodb.repository.query.MongoQueryMethod, org.springframework.data.repository.query.ResultProcessor, org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor, java.lang.Class) */ @Override + @Nullable protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProcessor, ConvertingParameterAccessor accessor, Class typeToRead) { Class sourceType = method.getDomainClass(); Class targetType = typeToRead; - List pipeline = computePipeline(method, accessor); + AggregationPipeline pipeline = computePipeline(method, accessor); AggregationUtils.appendSortIfPresent(pipeline, accessor, typeToRead); if (method.isSliceQuery()) { @@ -111,8 +117,8 @@ protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProces targetType = method.getReturnType().getRequiredActualType().getRequiredComponentType().getType(); } - AggregationOptions options = computeOptions(method, accessor); - TypedAggregation aggregation = new TypedAggregation<>(sourceType, pipeline, options); + AggregationOptions options = computeOptions(method, accessor, pipeline); + TypedAggregation aggregation = new TypedAggregation<>(sourceType, pipeline.getOperations(), options); if (method.isStreamQuery()) { @@ -126,6 +132,9 @@ protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProces } AggregationResults result = (AggregationResults) mongoOperations.aggregate(aggregation, targetType); + if(ReflectionUtils.isVoid(typeToRead)) { + return null; + } if (isRawAggregationResult) { return result; @@ -167,11 +176,11 @@ private boolean isSimpleReturnType(Class targetType) { return MongoSimpleTypes.HOLDER.isSimpleType(targetType); } - List computePipeline(MongoQueryMethod method, ConvertingParameterAccessor accessor) { - return parseAggregationPipeline(method.getAnnotatedAggregation(), accessor); + AggregationPipeline computePipeline(MongoQueryMethod method, ConvertingParameterAccessor accessor) { + return new AggregationPipeline(parseAggregationPipeline(method.getAnnotatedAggregation(), accessor)); } - private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingParameterAccessor accessor) { + private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingParameterAccessor accessor, AggregationPipeline pipeline) { AggregationOptions.Builder builder = Aggregation.newAggregationOptions(); @@ -179,6 +188,10 @@ private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingPar expressionParser, evaluationContextProvider); AggregationUtils.applyMeta(builder, method); + if(ReflectionUtils.isVoid(method.getReturnType().getType()) && pipeline.isOutOrMerge()) { + builder.skipOutput(); + } + return builder.build(); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregationUnitTests.java index 8b46457654..f83be9de24 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregationUnitTests.java @@ -78,6 +78,7 @@ public class ReactiveStringBasedAggregationUnitTests { private static final String RAW_SORT_STRING = "{ '$sort' : { 'lastname' : -1 } }"; private static final String RAW_GROUP_BY_LASTNAME_STRING = "{ '$group': { '_id' : '$lastname', 'names' : { '$addToSet' : '$firstname' } } }"; + private static final String RAW_OUT = "{ '$out' : 'authors' }"; private static final String GROUP_BY_LASTNAME_STRING_WITH_PARAMETER_PLACEHOLDER = "{ '$group': { '_id' : '$lastname', names : { '$addToSet' : '$?0' } } }"; private static final String GROUP_BY_LASTNAME_STRING_WITH_SPEL_PARAMETER_PLACEHOLDER = "{ '$group': { '_id' : '$lastname', 'names' : { '$addToSet' : '$?#{[0]}' } } }"; @@ -188,6 +189,22 @@ private AggregationInvocation executeAggregation(String name, Object... args) { return new AggregationInvocation(aggregationCaptor.getValue(), targetTypeCaptor.getValue(), result); } + @Test // GH-4088 + void aggregateWithVoidReturnTypeSkipsResultOnOutStage() { + + AggregationInvocation invocation = executeAggregation("outSkipResult"); + + assertThat(skipResultsOf(invocation)).isTrue(); + } + + @Test // GH-4088 + void aggregateWithOutStageDoesNotSkipResults() { + + AggregationInvocation invocation = executeAggregation("outDoNotSkipResult"); + + assertThat(skipResultsOf(invocation)).isFalse(); + } + private ReactiveStringBasedAggregation createAggregationForMethod(String name, Class... parameters) { Method method = ClassUtils.getMethod(SampleRepository.class, name, parameters); @@ -216,6 +233,11 @@ private Collation collationOf(AggregationInvocation invocation) { : null; } + private Boolean skipResultsOf(AggregationInvocation invocation) { + return invocation.aggregation.getOptions() != null ? invocation.aggregation.getOptions().isSkipResults() + : false; + } + private Class targetTypeOf(AggregationInvocation invocation) { return invocation.getTargetType(); } @@ -243,6 +265,12 @@ private interface SampleRepository extends ReactiveCrudRepository @Aggregation(pipeline = RAW_GROUP_BY_LASTNAME_STRING, collation = "de_AT") Mono aggregateWithCollation(Collation collation); + + @Aggregation(pipeline = { RAW_GROUP_BY_LASTNAME_STRING, RAW_OUT }) + Flux outDoNotSkipResult(); + + @Aggregation(pipeline = { RAW_GROUP_BY_LASTNAME_STRING, RAW_OUT }) + Mono outSkipResult(); } static class PersonAggregate { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedAggregationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedAggregationUnitTests.java index 62c699e929..f3e4e5a8c5 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedAggregationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedAggregationUnitTests.java @@ -91,6 +91,7 @@ public class StringBasedAggregationUnitTests { private static final String RAW_SORT_STRING = "{ '$sort' : { 'lastname' : -1 } }"; private static final String RAW_GROUP_BY_LASTNAME_STRING = "{ '$group': { '_id' : '$lastname', 'names' : { '$addToSet' : '$firstname' } } }"; + private static final String RAW_OUT = "{ '$out' : 'authors' }"; private static final String GROUP_BY_LASTNAME_STRING_WITH_PARAMETER_PLACEHOLDER = "{ '$group': { '_id' : '$lastname', names : { '$addToSet' : '$?0' } } }"; private static final String GROUP_BY_LASTNAME_STRING_WITH_SPEL_PARAMETER_PLACEHOLDER = "{ '$group': { '_id' : '$lastname', 'names' : { '$addToSet' : '$?#{[0]}' } } }"; @@ -260,6 +261,22 @@ void aggregateRaisesErrorOnInvalidReturnType() { .withMessageContaining("Page"); } + @Test // GH-4088 + void aggregateWithVoidReturnTypeSkipsResultOnOutStage() { + + AggregationInvocation invocation = executeAggregation("outSkipResult"); + + assertThat(skipResultsOf(invocation)).isTrue(); + } + + @Test // GH-4088 + void aggregateWithOutStageDoesNotSkipResults() { + + AggregationInvocation invocation = executeAggregation("outDoNotSkipResult"); + + assertThat(skipResultsOf(invocation)).isFalse(); + } + private AggregationInvocation executeAggregation(String name, Object... args) { Class[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new); @@ -302,6 +319,11 @@ private Collation collationOf(AggregationInvocation invocation) { : null; } + private Boolean skipResultsOf(AggregationInvocation invocation) { + return invocation.aggregation.getOptions() != null ? invocation.aggregation.getOptions().isSkipResults() + : false; + } + private Class targetTypeOf(AggregationInvocation invocation) { return invocation.getTargetType(); } @@ -350,6 +372,12 @@ private interface SampleRepository extends Repository { @Aggregation(RAW_GROUP_BY_LASTNAME_STRING) String simpleReturnType(); + + @Aggregation(pipeline = { RAW_GROUP_BY_LASTNAME_STRING, RAW_OUT }) + List outDoNotSkipResult(); + + @Aggregation(pipeline = { RAW_GROUP_BY_LASTNAME_STRING, RAW_OUT }) + void outSkipResult(); } private interface UnsupportedRepository extends Repository { diff --git a/src/main/asciidoc/reference/mongo-repositories-aggregation.adoc b/src/main/asciidoc/reference/mongo-repositories-aggregation.adoc index 1e6b40ac37..d2a160eb06 100644 --- a/src/main/asciidoc/reference/mongo-repositories-aggregation.adoc +++ b/src/main/asciidoc/reference/mongo-repositories-aggregation.adoc @@ -37,6 +37,12 @@ public interface PersonRepository extends CrudRepository { @Aggregation("{ '$project': { '_id' : '$lastname' } }") List findAllLastnames(); <9> + + @Aggregation(pipeline = { + "{ $group : { _id : '$author', books: { $push: '$title' } } }", + "{ $out : 'authors' }" + }) + void groupAndOutSkippingOutput(); <10> } ---- [source,java] @@ -75,6 +81,7 @@ Therefore, the `Sort` properties are mapped against the methods return type `Per To gain more control, you might consider `AggregationResult` as method return type as shown in <7>. <8> Obtain the raw `AggregationResults` mapped to the generic target wrapper type `SumValue` or `org.bson.Document`. <9> Like in <6>, a single value can be directly obtained from multiple result ``Document``s. +<10> Skips the output of the `$out` stage when return type is `void`. ==== In some scenarios, aggregations might require additional options, such as a maximum run time, additional log comments, or the permission to temporarily write data to disk.