diff --git a/pom.xml b/pom.xml index 9a1889723d..b3a1645ffd 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 5.0.0-SNAPSHOT + 5.0.x-GH-4667-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index fc88571622..bc8ccd7396 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 - 5.0.0-SNAPSHOT + 5.0.x-GH-4667-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 595e5a4250..b5a81a224d 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 5.0.0-SNAPSHOT + 5.0.x-GH-4667-SNAPSHOT ../pom.xml 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 278da408c6..16756fc91b 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 @@ -23,6 +23,7 @@ import org.springframework.data.mongodb.core.ReadConcernAware; import org.springframework.data.mongodb.core.ReadPreferenceAware; import org.springframework.data.mongodb.core.query.Collation; +import org.springframework.data.mongodb.core.query.DiskUse; import org.springframework.data.mongodb.util.BsonUtils; import org.springframework.lang.Contract; import org.springframework.util.Assert; @@ -60,7 +61,7 @@ public class AggregationOptions implements ReadConcernAware, ReadPreferenceAware private static final String MAX_TIME = "maxTimeMS"; private static final String HINT = "hint"; - private final Optional allowDiskUse; + private final DiskUse diskUse; private final boolean explain; private final Optional cursor; private final Optional collation; @@ -85,6 +86,15 @@ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Docum this(allowDiskUse, explain, cursor, null); } + public AggregationOptions(DiskUse diskUse, boolean explain, @Nullable Document cursor) { + this(diskUse, explain, cursor, null); + } + + public AggregationOptions(DiskUse allowDiskUse, boolean explain, @Nullable Document cursor, + @Nullable Collation collation) { + this(allowDiskUse, explain, cursor, collation, null); + } + /** * Creates a new {@link AggregationOptions}. * @@ -97,7 +107,7 @@ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Docum */ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Document cursor, @Nullable Collation collation) { - this(allowDiskUse, explain, cursor, collation, null, null); + this(DiskUse.of(allowDiskUse), explain, cursor, collation, null, null); } /** @@ -113,13 +123,18 @@ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Docum */ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Document cursor, @Nullable Collation collation, @Nullable String comment) { + this(allowDiskUse ? DiskUse.ALLOW : DiskUse.DENY, explain, cursor, collation, comment, null); + } + + public AggregationOptions(DiskUse 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 diskUse 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. @@ -128,10 +143,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(@Nullable Boolean allowDiskUse, boolean explain, @Nullable Document cursor, + private AggregationOptions(DiskUse diskUse, boolean explain, @Nullable Document cursor, @Nullable Collation collation, @Nullable String comment, @Nullable Object hint) { - this.allowDiskUse = Optional.ofNullable(allowDiskUse); + this.diskUse = diskUse; this.explain = explain; this.cursor = Optional.ofNullable(cursor); this.collation = Optional.ofNullable(collation); @@ -172,7 +187,7 @@ public static AggregationOptions fromDocument(Document document) { String comment = document.getString(COMMENT); Document hint = document.get(HINT, Document.class); - AggregationOptions options = new AggregationOptions(allowDiskUse, explain, cursor, collation, comment, hint); + AggregationOptions options = new AggregationOptions(DiskUse.of(allowDiskUse) , explain, cursor, collation, comment, hint); if (document.containsKey(MAX_TIME)) { options.maxTime = Duration.ofMillis(document.getLong(MAX_TIME)); } @@ -196,7 +211,7 @@ public static Builder builder() { * @return {@literal true} if enabled; {@literal false} otherwise (or if not set). */ public boolean isAllowDiskUse() { - return allowDiskUse.orElse(false); + return diskUse.equals(DiskUse.ALLOW); } /** @@ -206,7 +221,7 @@ public boolean isAllowDiskUse() { * @since 4.2.5 */ public boolean isAllowDiskUseSet() { - return allowDiskUse.isPresent(); + return !diskUse.equals(DiskUse.DEFAULT); } /** @@ -427,7 +442,7 @@ static Document createCursor(int cursorBatchSize) { */ public static class Builder { - private @Nullable Boolean allowDiskUse; + private @Nullable DiskUse diskUse = DiskUse.DEFAULT; private boolean explain; private @Nullable Document cursor; private @Nullable Collation collation; @@ -447,8 +462,19 @@ public static class Builder { */ @Contract("_ -> this") public Builder allowDiskUse(boolean allowDiskUse) { + return diskUse(DiskUse.of(allowDiskUse)); + } + + /** + * Defines whether to off-load intensive sort-operations to disk. + * + * @param diskUse use {@literal true} to allow disk use during the aggregation. + * @return this. + */ + @Contract("_ -> this") + public Builder diskUse(DiskUse diskUse) { - this.allowDiskUse = allowDiskUse; + this.diskUse = diskUse; return this; } @@ -655,7 +681,7 @@ public Builder noMapping() { @Contract("-> new") public AggregationOptions build() { - AggregationOptions options = new AggregationOptions(allowDiskUse, explain, cursor, collation, comment, hint); + AggregationOptions options = new AggregationOptions(diskUse, explain, cursor, collation, comment, hint); if (maxTime != null) { options.maxTime = maxTime; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/DiskUse.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/DiskUse.java new file mode 100644 index 0000000000..e2a60bb465 --- /dev/null +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/DiskUse.java @@ -0,0 +1,78 @@ +/* + * Copyright 2025-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb.core.query; + +import org.jspecify.annotations.Nullable; +import org.springframework.util.StringUtils; + +/** + * Disk use indicates if the MongoDB server is allowed to write temporary files to disk during query/aggregation + * execution. MongoDB 6.0 server (and later) default for {@literal allowDiskUseByDefault} is {@literal true} on server + * side. + * + * @author Christoph Strobl + * @since 5.0 + */ +public enum DiskUse { + + /** + * Go with the server default value and do not specify any override. + */ + DEFAULT, + + /** + * Override server default value and explicitly allow disk writes. + */ + ALLOW, + + /** + * Override server default value and explicitly deny disk writes. + */ + DENY; + + /** + * Obtain the {@link DiskUse} corresponding to the given Boolean flag. {@literal null} is considered {@link #DEFAULT}, + * {@literal true} as {@link #ALLOW}, {@literal false} as {@link #DENY}. + * + * @param value can be {@literal null}. + * @return the {@link DiskUse} corresponding to the given value. + */ + public static DiskUse of(@Nullable Boolean value) { + return value != null ? (value ? ALLOW : DENY) : DEFAULT; + } + + /** + * Obtain the {@link DiskUse} referred to by the given value. Considers {@literal null} or empty Strings as + * {@link #DEFAULT}, {@literal true} as {@link #ALLOW}, {@literal false} as {@link #DENY} and delegates other values + * to {@link #valueOf(String)}. + * + * @param value can be {@literal null}. + * @return the {@link DiskUse} corresponding to the given value. + * @throws IllegalArgumentException if not matching {@link DiskUse} found. + */ + public static DiskUse of(@Nullable String value) { + + if (!StringUtils.hasText(value)) { + return DEFAULT; + } + + return switch (value) { + case "true" -> ALLOW; + case "false" -> DENY; + default -> valueOf(value.toUpperCase()); + }; + } +} diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Meta.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Meta.java index 5ec4af3989..530a030e9f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Meta.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Meta.java @@ -51,7 +51,7 @@ private enum MetaKey { private Map values = Collections.emptyMap(); private Set flags = Collections.emptySet(); private @Nullable Integer cursorBatchSize; - private @Nullable Boolean allowDiskUse; + private DiskUse diskUse = DiskUse.DEFAULT; public Meta() {} @@ -66,7 +66,7 @@ public Meta() {} this.values = new LinkedHashMap<>(source.values); this.flags = new LinkedHashSet<>(source.flags); this.cursorBatchSize = source.cursorBatchSize; - this.allowDiskUse = source.allowDiskUse; + this.diskUse = source.diskUse; } /** @@ -230,7 +230,7 @@ public Set getFlags() { */ @Nullable public Boolean getAllowDiskUse() { - return allowDiskUse; + return diskUse.equals(DiskUse.DEFAULT) ? null : diskUse.equals(DiskUse.ALLOW); } /** @@ -244,14 +244,18 @@ public Boolean getAllowDiskUse() { * @since 3.0 */ public void setAllowDiskUse(@Nullable Boolean allowDiskUse) { - this.allowDiskUse = allowDiskUse; + setDiskUse(DiskUse.of(allowDiskUse)); + } + + public void setDiskUse(DiskUse diskUse) { + this.diskUse = diskUse; } /** * @return */ public boolean hasValues() { - return !this.values.isEmpty() || !this.flags.isEmpty() || this.cursorBatchSize != null || this.allowDiskUse != null; + return !this.values.isEmpty() || !this.flags.isEmpty() || this.cursorBatchSize != null || !this.diskUse.equals(DiskUse.DEFAULT); } /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java index 47ce615fe3..8a1572de93 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java @@ -582,8 +582,13 @@ public Query comment(String comment) { */ @Contract("_ -> this") public Query allowDiskUse(boolean allowDiskUse) { + return diskUse(DiskUse.of(allowDiskUse)); + } + + @Contract("_ -> this") + public Query diskUse(DiskUse diskUse) { - meta.setAllowDiskUse(allowDiskUse); + meta.setDiskUse(diskUse); return this; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/Meta.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/Meta.java index 37109426f9..66c9bd786d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/Meta.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/Meta.java @@ -69,11 +69,17 @@ /** * When set to {@literal true}, aggregation stages can write data to disk. + * Valid arguments are: + *
+ *
""
DiskUse#DEFAULT
+ *
true|allow
DiskUse#ALLOW
+ *
false|deny
DiskUse#DENY
+ *
* * @return {@literal false} by default. * @since 3.0 - * @see Aggregation + * @see org.springframework.data.mongodb.core.query.DiskUse */ - boolean allowDiskUse() default false; + String allowDiskUse() default ""; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/QueryBlocks.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/QueryBlocks.java index 391e6cc07f..79e716cf18 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/QueryBlocks.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/aot/QueryBlocks.java @@ -26,6 +26,7 @@ import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.annotation.Collation; import org.springframework.data.mongodb.core.query.BasicQuery; +import org.springframework.data.mongodb.core.query.DiskUse; import org.springframework.data.mongodb.repository.Hint; import org.springframework.data.mongodb.repository.Meta; import org.springframework.data.mongodb.repository.query.MongoQueryExecution.PagedExecution; @@ -292,6 +293,12 @@ CodeBlock build() { if (StringUtils.hasText(comment)) { builder.addStatement("$L.comment($S)", queryVariableName, comment); } + + String allowDiskUse = metaAnnotation.getString("allowDiskUse"); + if (StringUtils.hasText(allowDiskUse)) { + DiskUse diskUse = DiskUse.of(allowDiskUse); + builder.addStatement("$L.diskUse($T.$L)", queryVariableName, DiskUse.class, diskUse.name()); + } } MergedAnnotation collationAnnotation = context.getAnnotation(Collation.class); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryMethod.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryMethod.java index a07d9fbee0..4ae0dcaf77 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryMethod.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryMethod.java @@ -27,6 +27,7 @@ import org.springframework.data.mongodb.core.annotation.Collation; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; +import org.springframework.data.mongodb.core.query.DiskUse; import org.springframework.data.mongodb.core.query.UpdateDefinition; import org.springframework.data.mongodb.repository.Aggregation; import org.springframework.data.mongodb.repository.Hint; @@ -272,8 +273,9 @@ public org.springframework.data.mongodb.core.query.Meta getQueryMetaAttributes() } } - if (meta.allowDiskUse()) { - metaAttributes.setAllowDiskUse(meta.allowDiskUse()); + DiskUse diskUse = DiskUse.of(meta.allowDiskUse()); + if (!diskUse.equals(DiskUse.DEFAULT)) { + metaAttributes.setAllowDiskUse(diskUse.equals(DiskUse.ALLOW)); } return metaAttributes; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/MongoRepositoryContributorUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/MongoRepositoryContributorUnitTests.java index c55d460cfc..2fdd7e6758 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/MongoRepositoryContributorUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/aot/MongoRepositoryContributorUnitTests.java @@ -15,8 +15,8 @@ */ package org.springframework.data.mongodb.repository.aot; -import static org.mockito.Mockito.*; -import static org.springframework.data.mongodb.test.util.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.springframework.data.mongodb.test.util.Assertions.assertThat; import example.aot.User; import example.aot.UserRepository; @@ -25,7 +25,6 @@ import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.Test; - import org.springframework.aot.generate.GeneratedFiles; import org.springframework.aot.test.generate.TestGenerationContext; import org.springframework.beans.factory.annotation.Autowired; @@ -42,8 +41,8 @@ * Unit tests for the {@link UserRepository} fragment sources via {@link MongoRepositoryContributor}. * * @author Mark Paluch + * @author Christoph Strobl */ - @SpringJUnitConfig(classes = MongoRepositoryContributorUnitTests.MongoRepositoryContributorConfiguration.class) class MongoRepositoryContributorUnitTests { @@ -63,7 +62,7 @@ MongoOperations mongoOperations() { @Autowired TestGenerationContext generationContext; - @Test // GH-4970 + @Test // GH-4970, GH-4667 void shouldConsiderMetaAnnotation() throws IOException { InputStreamSource aotFragment = generationContext.getGeneratedFiles().getGeneratedFile(GeneratedFiles.Kind.SOURCE, @@ -74,6 +73,7 @@ void shouldConsiderMetaAnnotation() throws IOException { assertThat(content).contains("filterQuery.maxTimeMsec(555)"); assertThat(content).contains("filterQuery.cursorBatchSize(1234)"); assertThat(content).contains("filterQuery.comment(\"foo\")"); + assertThat(content).contains("filterQuery.diskUse(DiskUse.DENY)"); } interface MetaUserRepository extends CrudRepository { @@ -81,7 +81,7 @@ interface MetaUserRepository extends CrudRepository { @Meta User findAllByLastname(String lastname); - @Meta(cursorBatchSize = 1234, comment = "foo", maxExecutionTimeMs = 555) + @Meta(cursorBatchSize = 1234, comment = "foo", maxExecutionTimeMs = 555, allowDiskUse = "false") User findWithMetaAllByLastname(String lastname); } 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 827168007e..c18d44a00e 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 @@ -349,7 +349,7 @@ private Class targetTypeOf(AggregationInvocation invocation) { private interface SampleRepository extends Repository { - @Meta(cursorBatchSize = 42, comment = "expensive-aggregation", allowDiskUse = true, maxExecutionTimeMs = 100) + @Meta(cursorBatchSize = 42, comment = "expensive-aggregation", allowDiskUse = "true", maxExecutionTimeMs = 100) @Aggregation({ RAW_GROUP_BY_LASTNAME_STRING, RAW_SORT_STRING }) PersonAggregate plainStringAggregation();