From a23dc3b86ec74c26d859d9f48a34e850c1deb149 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 30 Nov 2022 13:40:13 +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 b78554647f..3b04395a67 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-4238-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index 1b2a1390e6..567a71316b 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-4238-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index 8db8d798fb..aeacf70e4f 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-4238-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index 9a57f7eb52..663d4037aa 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-4238-SNAPSHOT ../pom.xml From 839cb5f9a80dbb47606cc09395f2268e96d21acd Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 30 Nov 2022 15:54:29 +0100 Subject: [PATCH 2/2] Accept index names as hint for aggregations. --- .../data/mongodb/core/MongoTemplate.java | 32 ++++++++-- .../mongodb/core/ReactiveMongoTemplate.java | 11 +++- .../core/aggregation/AggregationOptions.java | 58 ++++++++++++++++++- .../mongodb/core/MongoTemplateUnitTests.java | 12 ++++ .../core/ReactiveMongoTemplateUnitTests.java | 13 +++++ 5 files changed, 116 insertions(+), 10 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 d579e6d034..177783c70b 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 @@ -30,7 +30,6 @@ import org.apache.commons.logging.LogFactory; import org.bson.Document; import org.bson.conversions.Bson; - import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -634,7 +633,8 @@ public MongoCollection createCollection(String collectionName, } @Override - public MongoCollection createView(String name, Class source, AggregationPipeline pipeline, @Nullable ViewOptions options) { + public MongoCollection createView(String name, Class source, AggregationPipeline pipeline, + @Nullable ViewOptions options) { return createView(name, getCollectionName(source), queryOperations.createAggregation(Aggregation.newAggregation(source, pipeline.getOperations()), source), @@ -642,7 +642,8 @@ public MongoCollection createView(String name, Class source, Aggreg } @Override - public MongoCollection createView(String name, String source, AggregationPipeline pipeline, @Nullable ViewOptions options) { + public MongoCollection createView(String name, String source, AggregationPipeline pipeline, + @Nullable ViewOptions options) { return createView(name, source, queryOperations.createAggregation(Aggregation.newAggregation(pipeline.getOperations()), (Class) null), @@ -654,7 +655,8 @@ private MongoCollection createView(String name, String source, Aggrega return doCreateView(name, source, aggregation.getAggregationPipeline(), options); } - protected MongoCollection doCreateView(String name, String source, List pipeline, @Nullable ViewOptions options) { + protected MongoCollection doCreateView(String name, String source, List pipeline, + @Nullable ViewOptions options) { CreateViewOptions viewOptions = new CreateViewOptions(); if (options != null) { @@ -2065,7 +2067,16 @@ protected AggregationResults doAggregate(Aggregation aggregation, String } options.getComment().ifPresent(aggregateIterable::comment); - options.getHint().ifPresent(aggregateIterable::hint); + if (options.getHintObject().isPresent()) { + Object hintObject = options.getHintObject().get(); + if (hintObject instanceof String hintString) { + aggregateIterable = aggregateIterable.hintString(hintString); + } else if (hintObject instanceof Document hintDocument) { + aggregateIterable = aggregateIterable.hint(hintDocument); + } else { + throw new IllegalStateException("Unable to read hint of type %s".formatted(hintObject.getClass())); + } + } if (options.hasExecutionTimeLimit()) { aggregateIterable = aggregateIterable.maxTime(options.getMaxTime().toMillis(), TimeUnit.MILLISECONDS); @@ -2124,7 +2135,16 @@ protected Stream aggregateStream(Aggregation aggregation, String collecti } options.getComment().ifPresent(cursor::comment); - options.getHint().ifPresent(cursor::hint); + if (options.getHintObject().isPresent()) { + Object hintObject = options.getHintObject().get(); + if (hintObject instanceof String hintString) { + cursor = cursor.hintString(hintString); + } else if (hintObject instanceof Document hintDocument) { + cursor = cursor.hint(hintDocument); + } else { + throw new IllegalStateException("Unable to read hint of type %s".formatted(hintObject.getClass())); + } + } Class domainType = aggregation instanceof TypedAggregation ? ((TypedAggregation) aggregation).getInputType() : null; 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 ac58269750..d2dd4f6f60 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 @@ -938,7 +938,16 @@ private Flux aggregateAndMap(MongoCollection collection, List operations.forType(inputType).getCollation()) // .map(Collation::toMongoCollation) // 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 e4ef7a7e49..11e42e8682 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 @@ -20,6 +20,7 @@ import org.bson.Document; import org.springframework.data.mongodb.core.query.Collation; +import org.springframework.data.mongodb.util.BsonUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -53,7 +54,7 @@ public class AggregationOptions { private final Optional cursor; private final Optional collation; private final Optional comment; - private final Optional hint; + private final Optional hint; private Duration maxTime = Duration.ZERO; private ResultOptions resultOptions = ResultOptions.READ; private DomainTypeMapping domainTypeMapping = DomainTypeMapping.RELAXED; @@ -113,7 +114,7 @@ public AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Docum * @since 3.1 */ private AggregationOptions(boolean allowDiskUse, boolean explain, @Nullable Document cursor, - @Nullable Collation collation, @Nullable String comment, @Nullable Document hint) { + @Nullable Collation collation, @Nullable String comment, @Nullable Object hint) { this.allowDiskUse = allowDiskUse; this.explain = explain; @@ -242,6 +243,44 @@ public Optional getComment() { * @since 3.1 */ public Optional getHint() { + return hint.map(it -> { + if (it instanceof Document doc) { + return doc; + } + if (it instanceof String hintString) { + if (BsonUtils.isJsonDocument(hintString)) { + return BsonUtils.parse(hintString, null); + } + } + throw new IllegalStateException("Unable to read hint of type %s".formatted(it.getClass())); + }); + } + + /** + * Get the hint (indexName) used to to fulfill the aggregation. + * + * @return never {@literal null}. + * @since 4.1 + */ + public Optional getHintAsString() { + return hint.map(it -> { + if (it instanceof String hintString) { + return hintString; + } + if (it instanceof Document doc) { + return BsonUtils.toJson(doc); + } + throw new IllegalStateException("Unable to read hint of type %s".formatted(it.getClass())); + }); + } + + /** + * Get the hint used to to fulfill the aggregation. + * + * @return never {@literal null}. + * @since 4.1 + */ + public Optional getHintObject() { return hint; } @@ -361,7 +400,7 @@ public static class Builder { private @Nullable Document cursor; private @Nullable Collation collation; private @Nullable String comment; - private @Nullable Document hint; + private @Nullable Object hint; private @Nullable Duration maxTime; private @Nullable ResultOptions resultOptions; private @Nullable DomainTypeMapping domainTypeMapping; @@ -454,6 +493,19 @@ public Builder hint(@Nullable Document hint) { return this; } + /** + * Define a hint that is used by query optimizer to to fulfill the aggregation. + * + * @param indexName can be {@literal null}. + * @return this. + * @since 4.1 + */ + public Builder hint(@Nullable String indexName) { + + this.hint = indexName; + return this; + } + /** * Set the time limit for processing. * diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index e5581249e8..3006bb8320 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -204,6 +204,8 @@ void beforeEach() { when(aggregateIterable.map(any())).thenReturn(aggregateIterable); when(aggregateIterable.maxTime(anyLong(), any())).thenReturn(aggregateIterable); when(aggregateIterable.into(any())).thenReturn(Collections.emptyList()); + when(aggregateIterable.hint(any())).thenReturn(aggregateIterable); + when(aggregateIterable.hintString(any())).thenReturn(aggregateIterable); when(distinctIterable.collation(any())).thenReturn(distinctIterable); when(distinctIterable.map(any())).thenReturn(distinctIterable); when(distinctIterable.into(any())).thenReturn(Collections.emptyList()); @@ -497,6 +499,16 @@ void aggregateShouldHonorOptionsHint() { verify(aggregateIterable).hint(hint); } + @Test // GH-4238 + void aggregateShouldHonorOptionsHintString() { + + AggregationOptions options = AggregationOptions.builder().hint("index-1").build(); + + template.aggregate(newAggregation(Aggregation.unwind("foo")).withOptions(options), "collection-1", Wrapper.class); + + verify(aggregateIterable).hintString("index-1"); + } + @Test // GH-3542 void aggregateShouldUseRelaxedMappingByDefault() { 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 01e827751c..af7d7105dd 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 @@ -23,6 +23,8 @@ import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import org.springframework.data.mongodb.core.MongoTemplateUnitTests.Wrapper; +import org.springframework.data.mongodb.core.aggregation.Aggregation; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -666,6 +668,17 @@ void aggregateShouldHonorOptionsHint() { verify(aggregatePublisher).hint(hint); } + @Test // GH-4238 + void aggregateShouldHonorOptionsHintString() { + + AggregationOptions options = AggregationOptions.builder().hint("index-1").build(); + + template.aggregate(newAggregation(Sith.class, project("id")).withOptions(options), AutogenerateableId.class, + Document.class).subscribe(); + + verify(aggregatePublisher).hintString("index-1"); + } + @Test // DATAMONGO-2390 void aggregateShouldNoApplyZeroOrNegativeMaxTime() {