DATAMONGO-1894 - Polishing.

Remove superfluous Optional wrappers and unify SpEL dependency resolution.

Original Pull Request: #874

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<Object> 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<SpELExpressionEvaluator> getSpelEvaluatorFor(ExpressionDependencies dependencies,
+			ConvertingParameterAccessor accessor) {
+
+		return evaluationContextProvider
+				.getEvaluationContextLater(getQueryMethod().getParameters(), accessor.getValues(), dependencies)
+				.map(evaluationContext -> (SpELExpressionEvaluator) new DefaultSpELExpressionEvaluator(expressionParser,
+						evaluationContext))
+				.defaultIfEmpty(DefaultSpELExpressionEvaluator.unsupported());
+	}
 }

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;

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<Query> 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<Query> 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<Query> createCountQuery(ConvertingParameterAccessor accessor) {
-		return Mono.fromSupplier(() -> new MongoQueryCreator(tree, accessor, context, false).createQuery());
-	}
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isCountQuery()

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<ExpressionDependencies> dependencies = CODEC.getExpressionDependencies(source,
+			ExpressionDependencies dependencies = CODEC.captureExpressionDependencies(source,
 					accessor::getBindableValue, expressionParser);
 
-			Mono<SpELExpressionEvaluator> 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<AggregationOperation> stage = evaluator.map(it -> {
+			Mono<AggregationOperation> stage = getSpelEvaluatorFor(dependencies, accessor).map(it -> {
 
 				ParameterBindingContext bindingContext = new ParameterBindingContext(accessor::getBindableValue, it);
 

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<ParameterBindingContext> getBindingContext(ConvertingParameterAccessor accessor,
 			ExpressionParser expressionParser, String json) {
 
-		Optional<ExpressionDependencies> dependencies = CODEC.getExpressionDependencies(json, accessor::getBindableValue,
+		ExpressionDependencies dependencies = CODEC.captureExpressionDependencies(json, accessor::getBindableValue,
 				expressionParser);
 
-		Mono<SpELExpressionEvaluator> 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));
 	}
 
 	/*

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<ExpressionDependencies> 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);
 	}

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<String, Expression> cache = new ConcurrentHashMap<>();
 
-	public CachingExpressionParser(ExpressionParser delegate) {
+	CachingExpressionParser(ExpressionParser delegate) {
 		this.delegate = delegate;
 	}
 

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<Document>
 	 *
 	 * @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<ExpressionDependencies> 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<ExpressionDependencies> dependencies = new ArrayList<>();
-
-		ParameterBindingContext context = new ParameterBindingContext(valueProvider, new SpELExpressionEvaluator() {
-			@Override
-			public <T> 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<Document>
 		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<ExpressionDependencies> dependencies = new ArrayList<>();
+
+		DependencyCapturingExpressionEvaluator(ExpressionParser expressionParser) {
+			this.expressionParser = expressionParser;
+		}
+
+		@Nullable
+		@Override
+		public <T> T evaluate(String expression) {
+
+			dependencies.add(ExpressionDependencies.discover(expressionParser.parseExpression(expression)));
+			return (T) PLACEHOLDER;
+		}
+
+		ExpressionDependencies getCapturedDependencies() {
+			return ExpressionDependencies.merged(dependencies);
+		}
+
+	}
 }

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);
+	}
 }

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> 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> 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);