diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java index 00294e298..a0d8e8780 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java @@ -19,9 +19,9 @@ import reactor.core.publisher.Mono; import org.bson.Document; import org.reactivestreams.Publisher; - import org.springframework.core.convert.converter.Converter; import org.springframework.data.mapping.model.EntityInstantiators; +import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.ReactiveFindOperation.FindWithProjection; import org.springframework.data.mongodb.core.ReactiveFindOperation.FindWithQuery; @@ -36,9 +36,9 @@ import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.repository.query.ResultProcessor; +import org.springframework.data.spel.ExpressionDependencies; import org.springframework.data.util.TypeInformation; import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -141,7 +141,6 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery { protected Publisher doExecute(ReactiveMongoQueryMethod method, ResultProcessor processor, ConvertingParameterAccessor accessor, @Nullable Class typeToRead) { - return createQuery(accessor).flatMapMany(it -> { Query query = it; @@ -296,4 +295,22 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery { * @since 2.0.4 */ protected abstract boolean isLimiting(); + + /** + * Obtain a {@link Mono publisher} emitting the {@link SpELExpressionEvaluator} suitable to evaluate expressions + * backed by the given dependencies. + * + * @param dependencies must not be {@literal null}. + * @param accessor must not be {@literal null}. + * @return a {@link Mono} emitting the {@link SpELExpressionEvaluator} when ready. + */ + protected Mono getSpelEvaluatorFor(ExpressionDependencies dependencies, + ConvertingParameterAccessor accessor) { + + return evaluationContextProvider + .getEvaluationContextLater(getQueryMethod().getParameters(), accessor.getValues(), dependencies) + .map(evaluationContext -> (SpELExpressionEvaluator) new DefaultSpELExpressionEvaluator(expressionParser, + evaluationContext)) + .defaultIfEmpty(DefaultSpELExpressionEvaluator.unsupported()); + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/DefaultSpELExpressionEvaluator.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/DefaultSpELExpressionEvaluator.java index ab4dcf8c8..cd154b5bd 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/DefaultSpELExpressionEvaluator.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/DefaultSpELExpressionEvaluator.java @@ -39,6 +39,7 @@ class DefaultSpELExpressionEvaluator implements SpELExpressionEvaluator { * Return a {@link SpELExpressionEvaluator} that does not support expression evaluation. * * @return a {@link SpELExpressionEvaluator} that does not support expression evaluation. + * @since 3.1 */ public static SpELExpressionEvaluator unsupported() { return NoOpExpressionEvaluator.INSTANCE; diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactivePartTreeMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactivePartTreeMongoQuery.java index cb0f4cb61..8ac8dbad1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactivePartTreeMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactivePartTreeMongoQuery.java @@ -19,7 +19,6 @@ import reactor.core.publisher.Mono; import org.bson.Document; import org.bson.json.JsonParseException; - import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.ReactiveMongoOperations; @@ -34,7 +33,6 @@ import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.repository.query.ReturnedType; import org.springframework.data.repository.query.parser.PartTree; import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.StringUtils; /** @@ -85,10 +83,27 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery { */ @Override protected Mono createQuery(ConvertingParameterAccessor accessor) { + return Mono.fromSupplier(() -> createQueryInternal(accessor, false)); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#createCountQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor) + */ + @Override + protected Mono createCountQuery(ConvertingParameterAccessor accessor) { + return Mono.fromSupplier(() -> createQueryInternal(accessor, true)); + } + + private Query createQueryInternal(ConvertingParameterAccessor accessor, boolean isCountQuery) { - MongoQueryCreator creator = new MongoQueryCreator(tree, accessor, context, isGeoNearQuery); + MongoQueryCreator creator = new MongoQueryCreator(tree, accessor, context, isCountQuery ? false : isGeoNearQuery); Query query = creator.createQuery(); + if (isCountQuery) { + return query; + } + if (tree.isLimiting()) { query.limit(tree.getMaxResults()); } @@ -108,7 +123,7 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery { returnedType.getInputProperties().forEach(query.fields()::include); } - return Mono.just(query); + return query; } try { @@ -116,23 +131,13 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery { BasicQuery result = new BasicQuery(query.getQueryObject(), Document.parse(fieldSpec)); result.setSortObject(query.getSortObject()); - return Mono.just(result); - + return result; } catch (JsonParseException o_O) { throw new IllegalStateException(String.format("Invalid query or field specification in %s!", getQueryMethod()), o_O); } } - /* - * (non-Javadoc) - * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#createCountQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor) - */ - @Override - protected Mono createCountQuery(ConvertingParameterAccessor accessor) { - return Mono.fromSupplier(() -> new MongoQueryCreator(tree, accessor, context, false).createQuery()); - } - /* * (non-Javadoc) * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isCountQuery() diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java index 6ebdc61a2..df9608eae 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java @@ -20,12 +20,9 @@ import reactor.core.publisher.Mono; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import org.bson.Document; import org.reactivestreams.Publisher; - -import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.aggregation.Aggregation; import org.springframework.data.mongodb.core.aggregation.AggregationOperation; @@ -40,7 +37,6 @@ import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationCo import org.springframework.data.repository.query.ResultProcessor; import org.springframework.data.spel.ExpressionDependencies; import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.ClassUtils; /** @@ -135,16 +131,10 @@ public class ReactiveStringBasedAggregation extends AbstractReactiveMongoQuery { for (String source : method.getAnnotatedAggregation()) { - Optional dependencies = CODEC.getExpressionDependencies(source, + ExpressionDependencies dependencies = CODEC.captureExpressionDependencies(source, accessor::getBindableValue, expressionParser); - Mono evaluator = dependencies.map( - it -> evaluationContextProvider.getEvaluationContextLater(method.getParameters(), accessor.getValues(), it)) - .map(evaluationContext -> evaluationContext - .map(it -> (SpELExpressionEvaluator) new DefaultSpELExpressionEvaluator(expressionParser, it))) - .orElseGet(() -> Mono.just(DefaultSpELExpressionEvaluator.unsupported())); - - Mono stage = evaluator.map(it -> { + Mono stage = getSpelEvaluatorFor(dependencies, accessor).map(it -> { ParameterBindingContext bindingContext = new ParameterBindingContext(accessor::getBindableValue, it); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQuery.java index 360a09575..1ab7762d3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQuery.java @@ -17,13 +17,9 @@ package org.springframework.data.mongodb.repository.query; import reactor.core.publisher.Mono; -import java.util.Optional; - import org.bson.Document; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.ReactiveMongoOperations; import org.springframework.data.mongodb.core.query.BasicQuery; @@ -145,17 +141,11 @@ public class ReactiveStringBasedMongoQuery extends AbstractReactiveMongoQuery { private Mono getBindingContext(ConvertingParameterAccessor accessor, ExpressionParser expressionParser, String json) { - Optional dependencies = CODEC.getExpressionDependencies(json, accessor::getBindableValue, + ExpressionDependencies dependencies = CODEC.captureExpressionDependencies(json, accessor::getBindableValue, expressionParser); - Mono evaluator = dependencies - .map(it -> evaluationContextProvider.getEvaluationContextLater(getQueryMethod().getParameters(), - accessor.getValues(), it)) - .map(evaluationContext -> evaluationContext - .map(it -> (SpELExpressionEvaluator) new DefaultSpELExpressionEvaluator(expressionParser, it))) - .orElseGet(() -> Mono.just(DefaultSpELExpressionEvaluator.unsupported())); - - return evaluator.map(it -> new ParameterBindingContext(accessor::getBindableValue, it)); + return getSpelEvaluatorFor(dependencies, accessor) + .map(it -> new ParameterBindingContext(accessor::getBindableValue, it)); } /* diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java index e09a7a1cd..bb6864233 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java @@ -15,13 +15,10 @@ */ package org.springframework.data.mongodb.repository.query; -import java.util.Optional; - import org.bson.Document; import org.bson.codecs.configuration.CodecRegistry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.query.BasicQuery; @@ -143,15 +140,11 @@ public class StringBasedMongoQuery extends AbstractMongoQuery { private ParameterBindingContext getBindingContext(ConvertingParameterAccessor accessor, ExpressionParser expressionParser, String json) { - Optional dependencies = codec.getExpressionDependencies(json, accessor::getBindableValue, + ExpressionDependencies dependencies = codec.captureExpressionDependencies(json, accessor::getBindableValue, expressionParser); - SpELExpressionEvaluator evaluator = dependencies - .map(it -> evaluationContextProvider.getEvaluationContext(getQueryMethod().getParameters(), - accessor.getValues(), it)) - .map(evaluationContext -> (SpELExpressionEvaluator) new DefaultSpELExpressionEvaluator(expressionParser, - evaluationContext)) - .orElse(DefaultSpELExpressionEvaluator.unsupported()); + SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(expressionParser, evaluationContextProvider + .getEvaluationContext(getQueryMethod().getParameters(), accessor.getValues(), dependencies)); return new ParameterBindingContext(accessor::getBindableValue, evaluator); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/CachingExpressionParser.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/CachingExpressionParser.java index 318bb6874..a30833b4d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/CachingExpressionParser.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/CachingExpressionParser.java @@ -35,7 +35,7 @@ class CachingExpressionParser implements ExpressionParser { private final ExpressionParser delegate; private final Map cache = new ConcurrentHashMap<>(); - public CachingExpressionParser(ExpressionParser delegate) { + CachingExpressionParser(ExpressionParser delegate) { this.delegate = delegate; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingDocumentCodec.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingDocumentCodec.java index 69a0fec2c..566ff603a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingDocumentCodec.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingDocumentCodec.java @@ -40,7 +40,6 @@ import org.bson.Transformer; import org.bson.codecs.*; import org.bson.codecs.configuration.CodecRegistry; import org.bson.json.JsonParseException; - import org.springframework.data.mapping.model.SpELExpressionEvaluator; import org.springframework.data.spel.EvaluationContextProvider; import org.springframework.data.spel.ExpressionDependencies; @@ -187,43 +186,23 @@ public class ParameterBindingDocumentCodec implements CollectibleCodec * * @param json * @param expressionParser - * @return a {@link Optional} containing merged {@link ExpressionDependencies} object if expressions were found, - * otherwise {@link Optional#empty()}. + * @return merged {@link ExpressionDependencies} object if expressions were found, otherwise + * {@link ExpressionDependencies#none()}. * @since 3.1 */ - public Optional getExpressionDependencies(@Nullable String json, ValueProvider valueProvider, + public ExpressionDependencies captureExpressionDependencies(@Nullable String json, ValueProvider valueProvider, ExpressionParser expressionParser) { if (StringUtils.isEmpty(json)) { - return Optional.empty(); + return ExpressionDependencies.none(); } - List dependencies = new ArrayList<>(); - - ParameterBindingContext context = new ParameterBindingContext(valueProvider, new SpELExpressionEvaluator() { - @Override - public T evaluate(String expression) { - - dependencies.add(ExpressionDependencies.discover(expressionParser.parseExpression(expression))); - - return (T) new Object(); - } - }); - - ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json, context); - this.decode(reader, DecoderContext.builder().build()); + DependencyCapturingExpressionEvaluator expressionEvaluator = new DependencyCapturingExpressionEvaluator( + expressionParser); + this.decode(new ParameterBindingJsonReader(json, new ParameterBindingContext(valueProvider, expressionEvaluator)), + DecoderContext.builder().build()); - if (dependencies.isEmpty()) { - return Optional.empty(); - } - - ExpressionDependencies result = ExpressionDependencies.empty(); - - for (ExpressionDependencies dependency : dependencies) { - result = result.mergeWith(dependency); - } - - return Optional.of(result); + return expressionEvaluator.getCapturedDependencies(); } @SuppressWarnings({ "rawtypes", "unchecked" }) @@ -400,4 +379,32 @@ public class ParameterBindingDocumentCodec implements CollectibleCodec return list; } + /** + * @author Christoph Strobl + * @since 3.1 + */ + static class DependencyCapturingExpressionEvaluator implements SpELExpressionEvaluator { + + private static final Object PLACEHOLDER = new Object(); + + private final ExpressionParser expressionParser; + private final List dependencies = new ArrayList<>(); + + DependencyCapturingExpressionEvaluator(ExpressionParser expressionParser) { + this.expressionParser = expressionParser; + } + + @Nullable + @Override + public T evaluate(String expression) { + + dependencies.add(ExpressionDependencies.discover(expressionParser.parseExpression(expression))); + return (T) PLACEHOLDER; + } + + ExpressionDependencies getCapturedDependencies() { + return ExpressionDependencies.merged(dependencies); + } + + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java index cd41e8ffa..a1ec0957c 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java @@ -1356,4 +1356,11 @@ public abstract class AbstractPersonRepositoryIntegrationTests { this.alicia.getEmail(), this.alicia.getAge(), Sex.FEMALE, this.alicia.createdAt, alicia.getSkills(), "street", "zipCode", "city", alicia.getUniqueId(), credentials.username, credentials.password)).isNotNull(); } + + @Test // DATAMONGO-1894 + void spelExpressionArgumentsGetReevaluatedOnEveryInvocation() { + + assertThat(repository.findWithSpelByFirstnameForSpELExpressionWithParameterIndexOnly("Dave")).containsExactly(dave); + assertThat(repository.findWithSpelByFirstnameForSpELExpressionWithParameterIndexOnly("Carter")).containsExactly(carter); + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java index bc774538f..3403f304d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java @@ -25,7 +25,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.List; -import java.util.Optional; import org.bson.Document; import org.bson.codecs.DecoderContext; @@ -268,10 +267,10 @@ class ParameterBindingJsonReaderUnitTests { String json = "{ $and : [?#{ [0] == null ? { '$where' : 'true' } : { 'v1' : { '$in' : {[0]} } } }]}"; - Optional expressionDependencies = new ParameterBindingDocumentCodec() - .getExpressionDependencies(json, it -> new Object(), new SpelExpressionParser()); + ExpressionDependencies expressionDependencies = new ParameterBindingDocumentCodec() + .captureExpressionDependencies(json, it -> new Object(), new SpelExpressionParser()); - assertThat(expressionDependencies).contains(ExpressionDependencies.empty()); + assertThat(expressionDependencies).isEqualTo(ExpressionDependencies.none()); } @Test // DATAMONGO-1894 @@ -279,8 +278,8 @@ class ParameterBindingJsonReaderUnitTests { String json = "{ hello: ?#{hasRole('foo')} }"; - Optional expressionDependencies = new ParameterBindingDocumentCodec() - .getExpressionDependencies(json, it -> new Object(), new SpelExpressionParser()); + ExpressionDependencies expressionDependencies = new ParameterBindingDocumentCodec() + .captureExpressionDependencies(json, it -> new Object(), new SpelExpressionParser()); assertThat(expressionDependencies).isNotEmpty(); assertThat(expressionDependencies.get()).hasSize(1);