Skip to content

Commit 6167f63

Browse files
committed
Polishing.
Simplify reactive composition. Switch to eager operator evaluation. See #4712 Original pull request: #4717
1 parent 6437640 commit 6167f63

File tree

3 files changed

+36
-31
lines changed

3 files changed

+36
-31
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@
1717

1818
import reactor.core.publisher.Flux;
1919
import reactor.core.publisher.Mono;
20-
import reactor.util.function.Tuple2;
2120

2221
import java.util.ArrayList;
2322
import java.util.List;
2423

2524
import org.bson.Document;
2625
import org.bson.codecs.configuration.CodecRegistry;
2726
import org.reactivestreams.Publisher;
27+
2828
import org.springframework.core.convert.converter.Converter;
2929
import org.springframework.data.mapping.model.EntityInstantiators;
3030
import org.springframework.data.mapping.model.SpELExpressionEvaluator;
@@ -231,7 +231,6 @@ private boolean isTailable(MongoQueryMethod method) {
231231
return method.getTailableAnnotation() != null;
232232
}
233233

234-
235234
Query applyQueryMetaAttributesWhenPresent(Query query) {
236235

237236
if (method.hasQueryMetaAttributes()) {
@@ -290,7 +289,8 @@ Query applyHintIfPresent(Query query) {
290289
}
291290

292291
/**
293-
* If present apply the {@link com.mongodb.ReadPreference} from the {@link org.springframework.data.mongodb.repository.ReadPreference} annotation.
292+
* If present apply the {@link com.mongodb.ReadPreference} from the
293+
* {@link org.springframework.data.mongodb.repository.ReadPreference} annotation.
294294
*
295295
* @param query must not be {@literal null}.
296296
* @return never {@literal null}.
@@ -339,8 +339,8 @@ protected Mono<UpdateDefinition> createUpdate(MongoParameterAccessor accessor) {
339339

340340
String updateJson = updateSource.update();
341341
return getParameterBindingCodec() //
342-
.flatMap(codec -> expressionEvaluator(updateJson, accessor, codec)) //
343-
.map(it -> decode(it.getT1(), updateJson, accessor, it.getT2())) //
342+
.flatMap(codec -> expressionEvaluator(updateJson, accessor, codec) //
343+
.map(evaluator -> decode(evaluator, updateJson, accessor, codec))) //
344344
.map(BasicUpdate::fromDocument);
345345
}
346346
if (!ObjectUtils.isEmpty(updateSource.pipeline())) {
@@ -376,16 +376,17 @@ protected Mono<List<AggregationOperation>> parseAggregationPipeline(String[] pip
376376
private Mono<AggregationOperation> computePipelineStage(String source, MongoParameterAccessor accessor,
377377
ParameterBindingDocumentCodec codec) {
378378

379-
return expressionEvaluator(source, accessor, codec).map(
380-
it -> new StringAggregationOperation(source, AbstractReactiveMongoQuery.this.getQueryMethod().getDomainClass(), bsonString -> AbstractReactiveMongoQuery.this.decode(it.getT1(), bsonString, accessor, it.getT2())));
379+
return expressionEvaluator(source, accessor, codec).map(evaluator -> new StringAggregationOperation(source,
380+
AbstractReactiveMongoQuery.this.getQueryMethod().getDomainClass(),
381+
bsonString -> AbstractReactiveMongoQuery.this.decode(evaluator, bsonString, accessor, codec)));
381382
}
382383

383-
private Mono<Tuple2<SpELExpressionEvaluator, ParameterBindingDocumentCodec>> expressionEvaluator(String source,
384-
MongoParameterAccessor accessor, ParameterBindingDocumentCodec codec) {
384+
private Mono<SpELExpressionEvaluator> expressionEvaluator(String source, MongoParameterAccessor accessor,
385+
ParameterBindingDocumentCodec codec) {
385386

386387
ExpressionDependencies dependencies = codec.captureExpressionDependencies(source, accessor::getBindableValue,
387388
expressionParser);
388-
return getSpelEvaluatorFor(dependencies, accessor).zipWith(Mono.just(codec));
389+
return getSpelEvaluatorFor(dependencies, accessor);
389390
}
390391

391392
private Document decode(SpELExpressionEvaluator expressionEvaluator, String source, MongoParameterAccessor accessor,

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringAggregationOperation.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@
2222
import org.bson.Document;
2323
import org.springframework.data.mongodb.core.aggregation.AggregationOperation;
2424
import org.springframework.data.mongodb.core.aggregation.AggregationOperationContext;
25+
import org.springframework.lang.Nullable;
2526

2627
/**
28+
* String-based aggregation operation for a repository query method.
29+
*
2730
* @author Christoph Strobl
31+
* @since 4.2.7
2832
*/
2933
class StringAggregationOperation implements AggregationOperation {
3034

@@ -33,12 +37,16 @@ class StringAggregationOperation implements AggregationOperation {
3337
private final String source;
3438
private final Class<?> domainType;
3539
private final Function<String, Document> bindFunction;
40+
private final @Nullable String operator;
3641

3742
StringAggregationOperation(String source, Class<?> domainType, Function<String, Document> bindFunction) {
3843

3944
this.source = source;
4045
this.domainType = domainType;
4146
this.bindFunction = bindFunction;
47+
48+
Matcher matcher = OPERATOR_PATTERN.matcher(source);
49+
this.operator = matcher.find() ? matcher.group() : null;
4250
}
4351

4452
@Override
@@ -48,11 +56,6 @@ public Document toDocument(AggregationOperationContext context) {
4856

4957
@Override
5058
public String getOperator() {
51-
52-
Matcher matcher = OPERATOR_PATTERN.matcher(source);
53-
if (matcher.find()) {
54-
return matcher.group();
55-
}
56-
return AggregationOperation.super.getOperator();
59+
return operator != null ? operator : AggregationOperation.super.getOperator();
5760
}
5861
}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedAggregationOperationUnitTests.java

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.springframework.data.mongodb.repository.query;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.assertj.core.api.Assertions.*;
1919

2020
import org.assertj.core.api.Assertions;
2121
import org.bson.Document;
@@ -24,27 +24,28 @@
2424
import org.junit.jupiter.params.provider.ValueSource;
2525

2626
/**
27+
* Unit tests for {@link StringBasedAggregation}.
28+
*
2729
* @author Christoph Strobl
2830
*/
2931
public class StringBasedAggregationOperationUnitTests {
3032

31-
@ParameterizedTest // GH-4712
32-
@ValueSource(strings = { "$project", "'$project'", "\"$project\"" })
33-
void extractsAggregationOperatorFromAggregationStringWithoutBindingParameters(String operator) {
33+
@ParameterizedTest // GH-4712
34+
@ValueSource(strings = { "$project", "'$project'", "\"$project\"" })
35+
void extractsAggregationOperatorFromAggregationStringWithoutBindingParameters(String operator) {
3436

35-
StringAggregationOperation agg = new StringAggregationOperation("{ %s : { 'fn' : 1 } }".formatted(operator),
36-
Object.class, (it) -> Assertions.fail("o_O Parameter binding"));
37+
StringAggregationOperation agg = new StringAggregationOperation("{ %s : { 'fn' : 1 } }".formatted(operator),
38+
Object.class, (it) -> Assertions.fail("o_O Parameter binding"));
3739

38-
assertThat(agg.getOperator()).isEqualTo("$project");
39-
}
40+
assertThat(agg.getOperator()).isEqualTo("$project");
41+
}
4042

41-
@Test
42-
// GH-4712
43-
void fallbackToParameterBindingIfAggregationOperatorCannotBeExtractedFromAggregationStringWithoutBindingParameters() {
43+
@Test // GH-4712
44+
void fallbackToParameterBindingIfAggregationOperatorCannotBeExtractedFromAggregationStringWithoutBindingParameters() {
4445

45-
StringAggregationOperation agg = new StringAggregationOperation("{ happy-madison : { 'fn' : 1 } }", Object.class,
46-
(it) -> new Document("$project", ""));
46+
StringAggregationOperation agg = new StringAggregationOperation("{ happy-madison : { 'fn' : 1 } }", Object.class,
47+
(it) -> new Document("$project", ""));
4748

48-
assertThat(agg.getOperator()).isEqualTo("$project");
49-
}
49+
assertThat(agg.getOperator()).isEqualTo("$project");
50+
}
5051
}

0 commit comments

Comments
 (0)