Retain parameter type when binding parameters in annotated Query/Aggregation.

This commit ensures the parameter type is preserved when binding parameters used within the value of the Query or Aggregation annotation

Closes: #4089

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/EvaluationContextExpressionEvaluator.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2022 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.util.json;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
+
+import org.springframework.data.mapping.model.SpELExpressionEvaluator;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.ExpressionParser;
+import org.springframework.expression.spel.standard.SpelExpression;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.StandardEvaluationContext;
+import org.springframework.lang.Nullable;
+
+/**
+ * @author Christoph Strobl
+ * @since 3.3.5
+ */
+class EvaluationContextExpressionEvaluator implements SpELExpressionEvaluator {
+
+	ValueProvider valueProvider;
+	ExpressionParser expressionParser;
+	Supplier<EvaluationContext> evaluationContext;
+
+	public EvaluationContextExpressionEvaluator(ValueProvider valueProvider, ExpressionParser expressionParser,
+			Supplier<EvaluationContext> evaluationContext) {
+
+		this.valueProvider = valueProvider;
+		this.expressionParser = expressionParser;
+		this.evaluationContext = evaluationContext;
+	}
+
+	@Nullable
+	@Override
+	public <T> T evaluate(String expression) {
+		return evaluateExpression(expression, Collections.emptyMap());
+	}
+
+	public EvaluationContext getEvaluationContext(String expressionString) {
+		return evaluationContext != null ? evaluationContext.get() : new StandardEvaluationContext();
+	}
+
+	public SpelExpression getParsedExpression(String expressionString) {
+		return (SpelExpression) (expressionParser != null ? expressionParser : new SpelExpressionParser())
+				.parseExpression(expressionString);
+	}
+
+	public <T> T evaluateExpression(String expressionString, Map<String, Object> variables) {
+
+		SpelExpression expression = getParsedExpression(expressionString);
+		EvaluationContext ctx = getEvaluationContext(expressionString);
+		variables.entrySet().forEach(entry -> ctx.setVariable(entry.getKey(), entry.getValue()));
+
+		Object result = expression.getValue(ctx, Object.class);
+		return (T) result;
+	}
+}

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingContext.java

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.mongodb.util.json;
 
+import java.util.Map;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -58,13 +59,7 @@ public class ParameterBindingContext {
 	 */
 	public ParameterBindingContext(ValueProvider valueProvider, ExpressionParser expressionParser,
 			Supplier<EvaluationContext> evaluationContext) {
-
+		this(valueProvider, new EvaluationContextExpressionEvaluator(valueProvider, expressionParser, evaluationContext));
-		this(valueProvider, new SpELExpressionEvaluator() {
-			@Override
-			public <T> T evaluate(String expressionString) {
-				return (T) expressionParser.parseExpression(expressionString).getValue(evaluationContext.get(), Object.class);
-			}
-		});
 	}
 
 	/**
@@ -87,20 +82,20 @@ public class ParameterBindingContext {
 	 * @return
 	 * @since 3.1
 	 */
-	public static ParameterBindingContext forExpressions(ValueProvider valueProvider,
+	public static ParameterBindingContext forExpressions(ValueProvider valueProvider, ExpressionParser expressionParser,
-			ExpressionParser expressionParser, Function<ExpressionDependencies, EvaluationContext> contextFunction) {
+			Function<ExpressionDependencies, EvaluationContext> contextFunction) {
 
-		return new ParameterBindingContext(valueProvider, new SpELExpressionEvaluator() {
+		return new ParameterBindingContext(valueProvider,
-			@Override
+				new EvaluationContextExpressionEvaluator(valueProvider, expressionParser, null) {
-			public <T> T evaluate(String expressionString) {
 
-				Expression expression = expressionParser.parseExpression(expressionString);
+					@Override
-				ExpressionDependencies dependencies = ExpressionDependencies.discover(expression);
+					public EvaluationContext getEvaluationContext(String expressionString) {
-				EvaluationContext evaluationContext = contextFunction.apply(dependencies);
 
-				return (T) expression.getValue(evaluationContext, Object.class);
+						Expression expression = getParsedExpression(expressionString);
-			}
+						ExpressionDependencies dependencies = ExpressionDependencies.discover(expression);
-		});
+						return contextFunction.apply(dependencies);
+					}
+				});
 	}
 
 	@Nullable
@@ -113,6 +108,16 @@ public class ParameterBindingContext {
 		return expressionEvaluator.evaluate(expressionString);
 	}
 
+	@Nullable
+	public Object evaluateExpression(String expressionString, Map<String, Object> variables) {
+
+		if (expressionEvaluator instanceof EvaluationContextExpressionEvaluator) {
+			return ((EvaluationContextExpressionEvaluator) expressionEvaluator).evaluateExpression(expressionString,
+					variables);
+		}
+		return expressionEvaluator.evaluate(expressionString);
+	}
+
 	public ValueProvider getValueProvider() {
 		return valueProvider;
 	}

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReader.java

@@ -20,8 +20,12 @@ import static java.lang.String.*;
 import java.text.DateFormat;
 import java.text.ParsePosition;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.TimeZone;
@@ -64,6 +68,7 @@ public class ParameterBindingJsonReader extends AbstractBsonReader {
 	private static final Pattern ENTIRE_QUERY_BINDING_PATTERN = Pattern.compile("^\\?(\\d+)$|^[\\?:]#\\{.*\\}$");
 	private static final Pattern PARAMETER_BINDING_PATTERN = Pattern.compile("\\?(\\d+)");
 	private static final Pattern EXPRESSION_BINDING_PATTERN = Pattern.compile("[\\?:]#\\{.*\\}");
+	private static final Pattern SPEL_PARAMETER_BINDING_PATTERN = Pattern.compile("('\\?(\\d+)'|\\?(\\d+))");
 
 	private final ParameterBindingContext bindingContext;
 
@@ -372,14 +377,24 @@ public class ParameterBindingJsonReader extends AbstractBsonReader {
 				String binding = regexMatcher.group();
 				String expression = binding.substring(3, binding.length() - 1);
 
-				Matcher inSpelMatcher = PARAMETER_BINDING_PATTERN.matcher(expression);
+				Matcher inSpelMatcher = SPEL_PARAMETER_BINDING_PATTERN.matcher(expression); // ?0 '?0'
+				Map<String, Object> innerSpelVariables = new HashMap<>();
+
 				while (inSpelMatcher.find()) {
 
-					int index = computeParameterIndex(inSpelMatcher.group());
+					String group = inSpelMatcher.group();
-					expression = expression.replace(inSpelMatcher.group(), getBindableValueForIndex(index).toString());
+					int index = computeParameterIndex(group);
+					Object value = getBindableValueForIndex(index);
+					String varName = "__QVar" + innerSpelVariables.size();
+					expression = expression.replace(group, "#" + varName);
+					if(group.startsWith("'")) { // retain the string semantic
+						innerSpelVariables.put(varName, nullSafeToString(value));
+					} else {
+						innerSpelVariables.put(varName, value);
+					}
 				}
 
-				Object value = evaluateExpression(expression);
+				Object value = evaluateExpression(expression, innerSpelVariables);
 				bindableValue.setValue(value);
 				bindableValue.setType(bsonTypeForValue(value));
 				return bindableValue;
@@ -408,14 +423,24 @@ public class ParameterBindingJsonReader extends AbstractBsonReader {
 			String binding = regexMatcher.group();
 			String expression = binding.substring(3, binding.length() - 1);
 
-			Matcher inSpelMatcher = PARAMETER_BINDING_PATTERN.matcher(expression);
+			Matcher inSpelMatcher = SPEL_PARAMETER_BINDING_PATTERN.matcher(expression);
+			Map<String, Object> innerSpelVariables = new HashMap<>();
+
 			while (inSpelMatcher.find()) {
 
-				int index = computeParameterIndex(inSpelMatcher.group());
+				String group = inSpelMatcher.group();
-				expression = expression.replace(inSpelMatcher.group(), getBindableValueForIndex(index).toString());
+				int index = computeParameterIndex(group);
+				Object value = getBindableValueForIndex(index);
+				String varName = "__QVar" + innerSpelVariables.size();
+				expression = expression.replace(group, "#" + varName);
+				if(group.startsWith("'")) { // retain the string semantic
+					innerSpelVariables.put(varName, nullSafeToString(value));
+				} else {
+					innerSpelVariables.put(varName, value);
+				}
 			}
 
-			computedValue = computedValue.replace(binding, nullSafeToString(evaluateExpression(expression)));
+			computedValue = computedValue.replace(binding, nullSafeToString(evaluateExpression(expression, innerSpelVariables)));
 
 			bindableValue.setValue(computedValue);
 			bindableValue.setType(BsonType.STRING);
@@ -452,7 +477,7 @@ public class ParameterBindingJsonReader extends AbstractBsonReader {
 	}
 
 	private static int computeParameterIndex(String parameter) {
-		return NumberUtils.parseNumber(parameter.replace("?", ""), Integer.class);
+		return NumberUtils.parseNumber(parameter.replace("?", "").replace("'", ""), Integer.class);
 	}
 
 	private Object getBindableValueForIndex(int index) {
@@ -504,7 +529,12 @@ public class ParameterBindingJsonReader extends AbstractBsonReader {
 
 	@Nullable
 	private Object evaluateExpression(String expressionString) {
-		return bindingContext.evaluateExpression(expressionString);
+		return bindingContext.evaluateExpression(expressionString, Collections.emptyMap());
+	}
+
+	@Nullable
+	private Object evaluateExpression(String expressionString, Map<String,Object> variables) {
+		return bindingContext.evaluateExpression(expressionString, variables);
 	}
 
 	// Spring Data Customization END

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/ParameterBindingJsonReaderUnitTests.java

@@ -25,14 +25,15 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Date;
 import java.util.List;
+import java.util.UUID;
 
-import org.bson.BsonBinary;
 import org.bson.Document;
 import org.bson.codecs.DecoderContext;
 import org.junit.jupiter.api.Test;
 import org.springframework.data.spel.EvaluationContextProvider;
 import org.springframework.data.spel.ExpressionDependencies;
 import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.ParseException;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -369,11 +370,11 @@ class ParameterBindingJsonReaderUnitTests {
 				new SpelExpressionParser());
 	}
 
-	@Test // GH-3871
+	@Test // GH-3871, GH-4089
-	public void bindEntireQueryUsingSpelExpressionWhenEvaluationResultIsJsonString() {
+	public void bindEntireQueryUsingSpelExpressionWhenEvaluationResultIsDocument() {
 
 		Object[] args = new Object[] { "expected", "unexpected" };
-		String json = "?#{ true ? \"{ 'name': ?0 }\" : \"{ 'name' : ?1 }\" }";
+		String json = "?#{ true ? { 'name': ?0 } : { 'name' : ?1 } }";
 		StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
 				.getEvaluationContext(args);
 
@@ -384,25 +385,27 @@ class ParameterBindingJsonReaderUnitTests {
 		assertThat(target).isEqualTo(new Document("name", "expected"));
 	}
 
-	@Test // GH-3871
+	@Test // GH-3871, GH-4089
-	public void throwsExceptionWhenbindEntireQueryUsingSpelExpressionResultsInInvalidJsonString() {
+	public void throwsExceptionWhenBindEntireQueryUsingSpelExpressionIsMalFormatted() {
 
 		Object[] args = new Object[] { "expected", "unexpected" };
-		String json = "?#{ true ? \"{ 'name': ?0 { }\" : \"{ 'name' : ?1 }\" }";
+		String json = "?#{ true ? { 'name': ?0 { } } : { 'name' : ?1 } }";
 		StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
 				.getEvaluationContext(args);
 
-		ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
+		assertThatExceptionOfType(ParseException.class).isThrownBy(() -> {
-				new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
+			ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json,
+					new ParameterBindingContext((index) -> args[index], new SpelExpressionParser(), evaluationContext));
 
-		assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build()));
+			new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
+		});
 	}
 
-	@Test // GH-3871
+	@Test // GH-3871, GH-4089
 	public void bindEntireQueryUsingSpelExpressionWhenEvaluationResultIsJsonStringContainingUUID() {
 
-		Object[] args = new Object[] { "UUID('cfbca728-4e39-4613-96bc-f920b5c37e16')", "unexpected" };
+		Object[] args = new Object[] { UUID.fromString("cfbca728-4e39-4613-96bc-f920b5c37e16"), "unexpected" };
-		String json = "?#{ true ? \"{ 'name': ?0 }\" : \"{ 'name' : ?1 }\" }";
+		String json = "?#{ true ? { 'name': ?0 } : { 'name' : ?1 } }";
 		StandardEvaluationContext evaluationContext = (StandardEvaluationContext) EvaluationContextProvider.DEFAULT
 				.getEvaluationContext(args);
 
@@ -411,7 +414,7 @@ class ParameterBindingJsonReaderUnitTests {
 
 		Document target = new ParameterBindingDocumentCodec().decode(reader, DecoderContext.builder().build());
 
-		assertThat(target.get("name")).isInstanceOf(BsonBinary.class);
+		assertThat(target.get("name")).isInstanceOf(UUID.class);
 	}
 
 	@Test // GH-3871
@@ -481,6 +484,69 @@ class ParameterBindingJsonReaderUnitTests {
 		assertThat(target).isEqualTo(new Document("parent", null));
 	}
 
+
+	@Test // GH-4089
+	void retainsSpelArgumentTypeViaArgumentIndex() {
+
+		String source = "new java.lang.Object()";
+		Document target = parse("{ arg0 : ?#{[0]} }", source);
+		assertThat(target.get("arg0")).isEqualTo(source);
+	}
+
+	@Test // GH-4089
+	void retainsSpelArgumentTypeViaParameterPlaceholder() {
+
+		String source = "new java.lang.Object()";
+		Document target = parse("{ arg0 : :#{?0} }", source);
+		assertThat(target.get("arg0")).isEqualTo(source);
+	}
+
+	@Test // GH-4089
+	void errorsOnNonDocument() {
+
+		String source = "new java.lang.Object()";
+		assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> parse(":#{?0}", source));
+	}
+
+	@Test // GH-4089
+	void bindsFullDocument() {
+
+		Document source = new Document();
+		assertThat(parse(":#{?0}", source)).isSameAs(source);
+	}
+
+	@Test // GH-4089
+	void enforcesStringSpelArgumentTypeViaParameterPlaceholderWhenQuoted() {
+
+		Integer source = 10;
+		Document target = parse("{ arg0 : :#{'?0'} }", source);
+		assertThat(target.get("arg0")).isEqualTo("10");
+	}
+
+	@Test // GH-4089
+	void enforcesSpelArgumentTypeViaParameterPlaceholderWhenQuoted() {
+
+		String source = "new java.lang.Object()";
+		Document target = parse("{ arg0 : :#{'?0'} }", source);
+		assertThat(target.get("arg0")).isEqualTo(source);
+	}
+
+	@Test // GH-4089
+	void retainsSpelArgumentTypeViaParameterPlaceholderWhenValueContainsSingleQuotes() {
+
+		String source = "' + new java.lang.Object() + '";
+		Document target = parse("{ arg0 : :#{?0} }", source);
+		assertThat(target.get("arg0")).isEqualTo(source);
+	}
+
+	@Test // GH-4089
+	void retainsSpelArgumentTypeViaParameterPlaceholderWhenValueContainsDoubleQuotes() {
+
+		String source = "\\\" + new java.lang.Object() + \\\"";
+		Document target = parse("{ arg0 : :#{?0} }", source);
+		assertThat(target.get("arg0")).isEqualTo(source);
+	}
+
 	private static Document parse(String json, Object... args) {
 
 		ParameterBindingJsonReader reader = new ParameterBindingJsonReader(json, args);