From f664dc0368a00ddb8f51a9c2ab1a60a6fb4465a1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 19 Mar 2024 09:40:20 +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 e364ecc9b5..9b2c5c5863 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.3.0-SNAPSHOT + 4.3.0-GH-4664-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 34d95eb205..a6a6b1f513 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.3.0-SNAPSHOT + 4.3.0-GH-4664-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 124a6bf5ad..3b613de2a5 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.3.0-SNAPSHOT + 4.3.0-GH-4664-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index e7282be0fa..b770a3e66f 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.3.0-SNAPSHOT + 4.3.0-GH-4664-SNAPSHOT ../pom.xml From 8fdad82c61e52bca9b68f7dc42c512ea9f9061fb Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 19 Mar 2024 10:03:01 +0100 Subject: [PATCH 2/2] Do not set `AggregationOptions.allowDiskUse` by default. With MongoDB changing its default, we now no longer set allowDiskUse to false if the value is not configured. --- .../data/mongodb/core/MongoTemplate.java | 14 +++++--- .../mongodb/core/ReactiveMongoTemplate.java | 7 ++-- .../core/aggregation/AggregationOptions.java | 36 ++++++++++++------- .../aggregation/AggregationOptionsTests.java | 16 +++++++++ 4 files changed, 55 insertions(+), 18 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 5c8c2702b3..e0938bcaff 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 @@ -2191,8 +2191,11 @@ protected AggregationResults doAggregate(Aggregation aggregation, String .getCollation()); AggregateIterable aggregateIterable = delegate.prepare(collection).aggregate(pipeline, Document.class) // - .collation(collation.map(Collation::toMongoCollation).orElse(null)) // - .allowDiskUse(options.isAllowDiskUse()); + .collation(collation.map(Collation::toMongoCollation).orElse(null)); + + if (options.isAllowDiskUseSet()) { + aggregateIterable = aggregateIterable.allowDiskUse(options.isAllowDiskUse()); + } if (options.getCursorBatchSize() != null) { aggregateIterable = aggregateIterable.batchSize(options.getCursorBatchSize()); @@ -2255,8 +2258,11 @@ protected Stream aggregateStream(Aggregation aggregation, String collecti CollectionPreparerDelegate delegate = CollectionPreparerDelegate.of(options); - AggregateIterable cursor = delegate.prepare(collection).aggregate(pipeline, Document.class) // - .allowDiskUse(options.isAllowDiskUse()); + AggregateIterable cursor = delegate.prepare(collection).aggregate(pipeline, Document.class); + + if (options.isAllowDiskUseSet()) { + cursor = cursor.allowDiskUse(options.isAllowDiskUse()); + } if (options.getCursorBatchSize() != null) { cursor = cursor.batchSize(options.getCursorBatchSize()); 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 0f1b8905a2..146bd0cb91 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 @@ -1027,8 +1027,11 @@ private Flux aggregateAndMap(MongoCollection collection, List inputType) { ReactiveCollectionPreparerDelegate collectionPreparer = ReactiveCollectionPreparerDelegate.of(options); - AggregatePublisher cursor = collectionPreparer.prepare(collection).aggregate(pipeline, Document.class) - .allowDiskUse(options.isAllowDiskUse()); + AggregatePublisher cursor = collectionPreparer.prepare(collection).aggregate(pipeline, Document.class); + + if (options.isAllowDiskUseSet()) { + cursor = cursor.allowDiskUse(options.isAllowDiskUse()); + } if (options.getCursorBatchSize() != null) { cursor = cursor.batchSize(options.getCursorBatchSize()); 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 ebee5122fc..972980b1a6 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 @@ -55,7 +55,7 @@ public class AggregationOptions implements ReadConcernAware, ReadPreferenceAware private static final String MAX_TIME = "maxTimeMS"; private static final String HINT = "hint"; - private final boolean allowDiskUse; + private final Optional allowDiskUse; private final boolean explain; private final Optional cursor; private final Optional collation; @@ -123,10 +123,10 @@ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Docum * @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, + private AggregationOptions(@Nullable Boolean allowDiskUse, boolean explain, @Nullable Document cursor, @Nullable Collation collation, @Nullable String comment, @Nullable Object hint) { - this.allowDiskUse = allowDiskUse; + this.allowDiskUse = Optional.ofNullable(allowDiskUse); this.explain = explain; this.cursor = Optional.ofNullable(cursor); this.collation = Optional.ofNullable(collation); @@ -159,7 +159,7 @@ public static AggregationOptions fromDocument(Document document) { Assert.notNull(document, "Document must not be null"); - boolean allowDiskUse = document.getBoolean(ALLOW_DISK_USE, false); + Boolean allowDiskUse = document.get(ALLOW_DISK_USE, Boolean.class); boolean explain = document.getBoolean(EXPLAIN, false); Document cursor = document.get(CURSOR, Document.class); Collation collation = document.containsKey(COLLATION) ? Collation.from(document.get(COLLATION, Document.class)) @@ -185,13 +185,23 @@ public static Builder builder() { } /** - * Enables writing to temporary files. When set to true, aggregation stages can write data to the _tmp subdirectory in - * the dbPath directory. + * Enables writing to temporary files. When set to {@literal true}, aggregation stages can write data to the + * {@code _tmp} subdirectory in the {@code dbPath} directory. * - * @return {@literal true} if enabled. + * @return {@literal true} if enabled; {@literal false} otherwise (or if not set). */ public boolean isAllowDiskUse() { - return allowDiskUse; + return allowDiskUse.orElse(false); + } + + /** + * Return whether {@link #isAllowDiskUse} is configured. + * + * @return {@literal true} if is {@code allowDiskUse} is configured, {@literal false} otherwise. + * @since 4.2.5 + */ + public boolean isAllowDiskUseSet() { + return allowDiskUse.isPresent(); } /** @@ -335,8 +345,8 @@ Document applyAndReturnPotentiallyChangedCommand(Document command) { Document result = new Document(command); - if (allowDiskUse && !result.containsKey(ALLOW_DISK_USE)) { - result.put(ALLOW_DISK_USE, allowDiskUse); + if (isAllowDiskUseSet() && !result.containsKey(ALLOW_DISK_USE)) { + result.put(ALLOW_DISK_USE, isAllowDiskUse()); } if (explain && !result.containsKey(EXPLAIN)) { @@ -370,7 +380,9 @@ Document applyAndReturnPotentiallyChangedCommand(Document command) { public Document toDocument() { Document document = new Document(); - document.put(ALLOW_DISK_USE, allowDiskUse); + if (isAllowDiskUseSet()) { + document.put(ALLOW_DISK_USE, isAllowDiskUse()); + } document.put(EXPLAIN, explain); cursor.ifPresent(val -> document.put(CURSOR, val)); @@ -410,7 +422,7 @@ static Document createCursor(int cursorBatchSize) { */ public static class Builder { - private boolean allowDiskUse; + private Boolean allowDiskUse; private boolean explain; private @Nullable Document cursor; private @Nullable Collation collation; 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 719a10d1c4..249fea9879 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 @@ -77,6 +77,22 @@ void shouldInitializeFromDocument() { assertThat(aggregationOptions.getHintObject()).contains(dummyHint); } + @Test // GH-4664 + void omitsAllowDiskUseByDefault() { + + aggregationOptions = AggregationOptions.fromDocument(new Document()); + + assertThat(aggregationOptions.isAllowDiskUse()).isFalse(); + assertThat(aggregationOptions.isAllowDiskUseSet()).isFalse(); + + assertThat(aggregationOptions.toDocument()).doesNotContainKey("allowDiskUse"); + + Document empty = new Document(); + aggregationOptions.applyAndReturnPotentiallyChangedCommand(empty); + + assertThat(empty).doesNotContainKey("allowDiskUse"); + } + @Test // DATAMONGO-960, DATAMONGO-2153, DATAMONGO-1836 void aggregationOptionsToString() {