From 5795a507bde1551ffdc03057a59549e65426a92b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 6 Aug 2020 11:25:26 +0200 Subject: [PATCH] DATAMONGO-1836 - Polishing. Revert constructor change of AggregationOptions to not break existing code. Update since tags. Reformat code. Align visibility of AggregationOptionsTests with JUnit 5 rules. Update documentation. Original pull request: #878. --- .../data/mongodb/core/MongoTemplate.java | 2 - .../mongodb/core/ReactiveMongoTemplate.java | 1 - .../core/aggregation/AggregationOptions.java | 38 ++++++++++++------- .../core/ReactiveMongoTemplateUnitTests.java | 1 + .../aggregation/AggregationOptionsTests.java | 31 +++++++-------- src/main/asciidoc/new-features.adoc | 1 + 6 files changed, 39 insertions(+), 35 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 fba9171fe..97deaae46 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 @@ -2149,7 +2149,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, } options.getComment().ifPresent(aggregateIterable::comment); - options.getHint().ifPresent(aggregateIterable::hint); if (options.hasExecutionTimeLimit()) { @@ -2209,7 +2208,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, } options.getComment().ifPresent(cursor::comment); - options.getHint().ifPresent(cursor::hint); Class domainType = aggregation instanceof TypedAggregation ? ((TypedAggregation) aggregation).getInputType() 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 c7ee5cc87..64fe99f08 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 @@ -1024,7 +1024,6 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati } options.getComment().ifPresent(cursor::comment); - options.getHint().ifPresent(cursor::hint); Optionals.firstNonEmpty(options::getCollation, () -> operations.forType(inputType).getCollation()) // diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOptions.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOptions.java index 520f3b18f..2c500149d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOptions.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOptions.java @@ -74,7 +74,7 @@ public class AggregationOptions { * @param allowDiskUse whether to off-load intensive sort-operations to disk. * @param explain whether to get the execution plan for the aggregation instead of the actual results. * @param cursor can be {@literal null}, used to pass additional options (such as {@code batchSize}) to the - * aggregation. + * aggregation. * @param collation collation for string comparison. Can be {@literal null}. * @since 2.0 */ @@ -89,13 +89,29 @@ public class AggregationOptions { * @param allowDiskUse whether to off-load intensive sort-operations to disk. * @param explain whether to get the execution plan for the aggregation instead of the actual results. * @param cursor can be {@literal null}, used to pass additional options (such as {@code batchSize}) to the - * aggregation. + * aggregation. * @param collation collation for string comparison. Can be {@literal null}. * @param comment execution comment. Can be {@literal null}. - * @param hint can be {@literal null}, used to provide an index that would be forcibly used by query optimizer. * @since 2.2 */ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Document cursor, + @Nullable Collation collation, @Nullable String comment) { + this(allowDiskUse, explain, cursor, collation, comment, null); + } + + /** + * Creates a new {@link AggregationOptions}. + * + * @param allowDiskUse whether to off-load intensive sort-operations to disk. + * @param explain whether to get the execution plan for the aggregation instead of the actual results. + * @param cursor can be {@literal null}, used to pass additional options (such as {@code batchSize}) to the + * aggregation. + * @param collation collation for string comparison. Can be {@literal null}. + * @param comment execution comment. Can be {@literal null}. + * @param hint can be {@literal null}, used to provide an index that would be forcibly used by query optimizer. + * @since 3.1 + */ + private AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Document cursor, @Nullable Collation collation, @Nullable String comment, @Nullable Document hint) { this.allowDiskUse = allowDiskUse; @@ -222,12 +238,12 @@ public class AggregationOptions { * Get the hint used to to fulfill the aggregation. * * @return never {@literal null}. + * @since 3.1 */ public Optional getHint() { return hint; } - /** * @return the time limit for processing. {@link Duration#ZERO} is used for the default unbounded behavior. * @since 3.0 @@ -419,11 +435,11 @@ public class AggregationOptions { } /** - * Define a hint is used forcibly by query optimizer to to fulfill the aggregation. + * Define a hint that is used by query optimizer to to fulfill the aggregation. * * @param hint can be {@literal null}. * @return this. - * @since 2.2 + * @since 3.1 */ public Builder hint(@Nullable Document hint) { @@ -435,7 +451,7 @@ public class AggregationOptions { * Set the time limit for processing. * * @param maxTime {@link Duration#ZERO} is used for the default unbounded behavior. {@link Duration#isNegative() - * Negative} values will be ignored. + * Negative} values will be ignored. * @return this. * @since 3.0 */ @@ -466,13 +482,7 @@ public class AggregationOptions { */ public AggregationOptions build() { - AggregationOptions options = new AggregationOptions( - allowDiskUse, - explain, - cursor, - collation, - comment, - hint); + AggregationOptions options = new AggregationOptions(allowDiskUse, explain, cursor, collation, comment, hint); if (maxTime != null) { options.maxTime = maxTime; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java index 65041210e..916123d70 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java @@ -631,6 +631,7 @@ public class ReactiveMongoTemplateUnitTests { @Test // DATAMONGO-1836 void aggregateShouldHonorOptionsHint() { + Document hint = new Document("dummyHint", 1); AggregationOptions options = AggregationOptions.builder().hint(hint).build(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOptionsTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOptionsTests.java index 0bb41022f..15e0af58a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOptionsTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationOptionsTests.java @@ -31,13 +31,13 @@ import org.junit.jupiter.api.Test; * @author Yadhukrishna S Pai * @since 1.6 */ -public class AggregationOptionsTests { +class AggregationOptionsTests { private final Document dummyHint = new Document("dummyField", 1); AggregationOptions aggregationOptions; @BeforeEach - public void setup() { + void setup() { aggregationOptions = newAggregationOptions().explain(true) // .cursorBatchSize(1) // .allowDiskUse(true) // @@ -47,16 +47,16 @@ public class AggregationOptionsTests { } @Test // DATAMONGO-960, DATAMONGO-1836 - public void aggregationOptionsBuilderShouldSetOptionsAccordingly() { + void aggregationOptionsBuilderShouldSetOptionsAccordingly() { assertThat(aggregationOptions.isAllowDiskUse()).isTrue(); assertThat(aggregationOptions.isExplain()).isTrue(); - assertThat(aggregationOptions.getCursor().get()).isEqualTo(new Document("batchSize", 1)); - assertThat(aggregationOptions.getHint().get()).isEqualTo(dummyHint); + assertThat(aggregationOptions.getCursor()).contains(new Document("batchSize", 1)); + assertThat(aggregationOptions.getHint()).contains(dummyHint); } @Test // DATAMONGO-1637, DATAMONGO-2153, DATAMONGO-1836 - public void shouldInitializeFromDocument() { + void shouldInitializeFromDocument() { Document document = new Document(); document.put("cursor", new Document("batchSize", 1)); @@ -69,22 +69,17 @@ public class AggregationOptionsTests { assertThat(aggregationOptions.isAllowDiskUse()).isTrue(); assertThat(aggregationOptions.isExplain()).isTrue(); - assertThat(aggregationOptions.getCursor().get()).isEqualTo(new Document("batchSize", 1)); + assertThat(aggregationOptions.getCursor()).contains(new Document("batchSize", 1)); assertThat(aggregationOptions.getCursorBatchSize()).isEqualTo(1); - assertThat(aggregationOptions.getComment().get()).isEqualTo("hola!"); - assertThat(aggregationOptions.getHint().get()).isEqualTo(dummyHint); + assertThat(aggregationOptions.getComment()).contains("hola!"); + assertThat(aggregationOptions.getHint()).contains(dummyHint); } @Test // DATAMONGO-960, DATAMONGO-2153, DATAMONGO-1836 - public void aggregationOptionsToString() { + void aggregationOptionsToString() { - assertThat(aggregationOptions.toDocument()).isEqualTo(Document.parse( - "{ " + - "\"allowDiskUse\" : true , " + - "\"explain\" : true , " + - "\"cursor\" : { \"batchSize\" : 1}, " + - "\"comment\": \"hola!\", " + - "\"hint\" : { \"dummyField\" : 1}" + - "}")); + assertThat(aggregationOptions.toDocument()).isEqualTo(Document + .parse("{ " + "\"allowDiskUse\" : true , " + "\"explain\" : true , " + "\"cursor\" : { \"batchSize\" : 1}, " + + "\"comment\": \"hola!\", " + "\"hint\" : { \"dummyField\" : 1}" + "}")); } } diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index f83375382..7727e2839 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -6,6 +6,7 @@ * <> enabled through `@EnableReactiveMongoAuditing`. `@EnableMongoAuditing` no longer registers `ReactiveAuditingEntityCallback`. * Reactive SpEL support in `@Query` and `@Aggregation` query methods. +* Aggregation hints via `AggregationOptions.builder().hint(bson).build()`. [[new-features.3.0]] == What's New in Spring Data MongoDB 3.0