DATAMONGO-995 - Improve support of quote handling for custom query parameters.

Introduced ParameterBindingParser which exposes parameter references in query strings as ParameterBindings. This allows us to detect whether a parameter reference in a query string is already quoted avoiding wrongly double-quoting the parameter value.

Original pull request: #185.
Related ticket: DATAMONGO-420.

Author: Thomas Darimont

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

@@ -15,8 +15,6 @@
  */
 package org.springframework.data.mongodb.repository.query;
 
-import static java.util.regex.Pattern.*;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -30,6 +28,7 @@
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.util.StringUtils;
 
+import com.mongodb.DBObject;
 import com.mongodb.util.JSON;
 
 /**
@@ -188,20 +187,13 @@ private static enum ParameterBindingParser {
 
 		INSTANCE;
 
-		private static final Pattern PARAMETER_BINDING_PATTERN;
+		private static final String PARAMETER_PREFIX = "_param_";
+		private static final String PARSEABLE_PARAMETER = "\"" + PARAMETER_PREFIX + "$1\"";
+		private static final Pattern PARAMETER_BINDING_PATTERN = Pattern.compile("\\?(\\d+)");
+		private static final Pattern PARSEABLE_BINDING_PATTERN = Pattern.compile("\"?" + PARAMETER_PREFIX + "(\\d+)\"?");
 
 		private final static int PARAMETER_INDEX_GROUP = 1;
 
-		static {
-
-			StringBuilder builder = new StringBuilder();
-			builder.append("\\?(\\d+)"); // position parameter and parameter index
-			builder.append("[^,'\"]*"); // followed by non quotes, non field separators
-			builder.append("[,\"'}]?");
-
-			PARAMETER_BINDING_PATTERN = Pattern.compile(builder.toString(), CASE_INSENSITIVE);
-		}
-
 		/**
 		 * Returns a list of {@link ParameterBinding}s found in the given {@code input} or an
 		 * {@link Collections#emptyList()}.
@@ -217,19 +209,60 @@ public List<ParameterBinding> parseParameterBindingsFrom(String input) {
 
 			List<ParameterBinding> bindings = new ArrayList<ParameterBinding>();
 
+			String parseableInput = makeParameterReferencesParseable(input);
+
+			collectParameterReferencesIntoBindings(bindings, JSON.parse(parseableInput));
+
+			return bindings;
+		}
+
+		private String makeParameterReferencesParseable(String input) {
+
 			Matcher matcher = PARAMETER_BINDING_PATTERN.matcher(input);
+			String parseableInput = matcher.replaceAll(PARSEABLE_PARAMETER);
 
-			while (matcher.find()) {
+			return parseableInput;
+		}
 
-				String group = matcher.group();
+		private void collectParameterReferencesIntoBindings(List<ParameterBinding> bindings, Object value) {
 
-				boolean parameterIsQuoted = group.endsWith("'") || group.endsWith("\"");
-				int parameterIndex = Integer.parseInt(matcher.group(PARAMETER_INDEX_GROUP));
+			if (value instanceof String) {
 
-				bindings.add(new ParameterBinding(parameterIndex, parameterIsQuoted));
-			}
+				String string = ((String) value).trim();
 
-			return bindings;
+				Matcher valueMatcher = PARSEABLE_BINDING_PATTERN.matcher(string);
+				while (valueMatcher.find()) {
+					int paramIndex = Integer.parseInt(valueMatcher.group(PARAMETER_INDEX_GROUP));
+					boolean quoted = (string.startsWith("'") && string.endsWith("'"))
+							|| (string.startsWith("\"") && string.endsWith("\""));
+					bindings.add(new ParameterBinding(paramIndex, quoted));
+				}
+
+			} else if (value instanceof Pattern) {
+
+				String string = ((Pattern) value).toString().trim();
+
+				Matcher valueMatcher = PARSEABLE_BINDING_PATTERN.matcher(string);
+				while (valueMatcher.find()) {
+					int paramIndex = Integer.parseInt(valueMatcher.group(PARAMETER_INDEX_GROUP));
+
+					/*
+					 * The pattern is used as a direct parameter replacement, e.g. 'field': ?1, 
+					 * therefore we treat it as not quoted to remain backwards compatible.
+					 */
+					boolean quoted = !string.equals(PARAMETER_PREFIX + paramIndex);
+
+					bindings.add(new ParameterBinding(paramIndex, quoted));
+				}
+
+			} else if (value instanceof DBObject) {
+
+				DBObject dbo = (DBObject) value;
+
+				for (String field : dbo.keySet()) {
+					collectParameterReferencesIntoBindings(bindings, dbo.get(field));
+				}
+			}
 		}
 	}
 

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

@@ -170,9 +170,9 @@ public void shouldSupportFindByParameterizedCriteriaAndFields() throws Exception
 
 		ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, new Object[] {
 				new BasicDBObject("firstname", "first").append("lastname", "last"), Collections.singletonMap("lastname", 1) });
-
 		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByParameterizedCriteriaAndFields", DBObject.class,
 				Map.class);
+
 		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
 
 		assertThat(query.getQueryObject(),
@@ -187,13 +187,74 @@ public void shouldSupportFindByParameterizedCriteriaAndFields() throws Exception
 	public void shouldSupportRespectExistingQuotingInFindByTitleBeginsWithExplicitQuoting() throws Exception {
 
 		ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, new Object[] { "fun" });
-
 		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByTitleBeginsWithExplicitQuoting", String.class);
+
 		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
 
 		assertThat(query.getQueryObject(), is(new BasicQuery("{title: {$regex: '^fun', $options: 'i'}}").getQueryObject()));
 	}
 
+	/**
+	 * @see DATAMONGO-995, DATAMONGO-420
+	 */
+	@Test
+	public void shouldParseQueryWithParametersInExpression() throws Exception {
+
+		ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, new Object[] { 1, 2, 3, 4 });
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByQueryWithParametersInExpression", int.class,
+				int.class, int.class, int.class);
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor);
+
+		assertThat(query.getQueryObject(), is(new BasicQuery(
+				"{$where: 'return this.date.getUTCMonth() == 3 && this.date.getUTCDay() == 4;'}").getQueryObject()));
+	}
+
+	/**
+	 * @see DATAMONGO-995, DATAMONGO-420
+	 */
+	@Test
+	public void bindsSimplePropertyAlreadyQuotedCorrectly() throws Exception {
+
+		ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "Matthews");
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class);
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
+		org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{'lastname' : 'Matthews'}");
+
+		assertThat(query.getQueryObject(), is(reference.getQueryObject()));
+	}
+
+	/**
+	 * @see DATAMONGO-995, DATAMONGO-420
+	 */
+	@Test
+	public void bindsSimplePropertyAlreadyQuotedWithRegexCorrectly() throws Exception {
+
+		ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "^Mat.*");
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastnameQuoted", String.class);
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
+		org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{'lastname' : '^Mat.*'}");
+
+		assertThat(query.getQueryObject(), is(reference.getQueryObject()));
+	}
+
+	/**
+	 * @see DATAMONGO-995, DATAMONGO-420
+	 */
+	@Test
+	public void bindsSimplePropertyWithRegexCorrectly() throws Exception {
+
+		StringBasedMongoQuery mongoQuery = createQueryForMethod("findByLastname", String.class);
+		ConvertingParameterAccessor accesor = StubParameterAccessor.getAccessor(converter, "^Mat.*");
+
+		org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accesor);
+		org.springframework.data.mongodb.core.query.Query reference = new BasicQuery("{'lastname' : '^Mat.*'}");
+
+		assertThat(query.getQueryObject(), is(reference.getQueryObject()));
+	}
+
 	private StringBasedMongoQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
 
 		Method method = SampleRepository.class.getMethod(name, parameters);
@@ -206,6 +267,9 @@ private interface SampleRepository {
 		@Query("{ 'lastname' : ?0 }")
 		Person findByLastname(String lastname);
 
+		@Query("{ 'lastname' : '?0' }")
+		Person findByLastnameQuoted(String lastname);
+
 		@Query("{ 'address' : ?0 }")
 		Person findByAddress(Address address);
 
@@ -226,5 +290,8 @@ private interface SampleRepository {
 
 		@Query("{'title': { $regex : '^?0', $options : 'i'}}")
 		List<DBObject> findByTitleBeginsWithExplicitQuoting(String title);
+
+		@Query(value = "{$where: 'return this.date.getUTCMonth() == ?2 && this.date.getUTCDay() == ?3;'}")
+		List<DBObject> findByQueryWithParametersInExpression(int param1, int param2, int param3, int param4);
 	}
 }