DATAES-552 - Polishing.

Update lookup to replace all matches & add some tests.

Original Pull Request: #267

src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java

@@ -19,12 +19,12 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.springframework.core.convert.support.GenericConversionService;
-import org.springframework.data.domain.Pageable;
 import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
 import org.springframework.data.elasticsearch.core.convert.DateTimeConverters;
 import org.springframework.data.elasticsearch.core.query.StringQuery;
 import org.springframework.data.repository.query.ParametersParameterAccessor;
 import org.springframework.util.Assert;
+import org.springframework.util.NumberUtils;
 
 /**
  * ElasticsearchStringQuery
@@ -83,13 +83,14 @@ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQue
 	}
 
 	private String replacePlaceholders(String input, ParametersParameterAccessor accessor) {
+
 		Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
 		String result = input;
 		while (matcher.find()) {
-			String group = matcher.group();
+
-			group = "\\" + group;
+			String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)";
-			int index = Integer.parseInt(matcher.group(1));
+			int index = NumberUtils.parseNumber(matcher.group(1), Integer.class);
-			result = result.replaceFirst(group, getParameterWithIndex(accessor, index));
+			result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index));
 		}
 		return result;
 	}

src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java

@@ -22,6 +22,7 @@ import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperatio
 import org.springframework.data.elasticsearch.core.query.StringQuery;
 import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.util.NumberUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -60,10 +61,10 @@ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsea
 		Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
 		String result = input;
 		while (matcher.find()) {
-			String group = matcher.group();
+
-			group = "\\" + group;
+			String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)";
-			int index = Integer.parseInt(matcher.group(1));
+			int index = NumberUtils.parseNumber(matcher.group(1), Integer.class);
-			result = result.replaceFirst(group, getParameterWithIndex(accessor, index));
+			result = result.replaceAll(placeholder, getParameterWithIndex(accessor, index));
 		}
 		return result;
 	}

src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2019 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.elasticsearch.repository.query;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.lang.reflect.Method;
+import java.util.Arrays;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.data.elasticsearch.annotations.Query;
+import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
+import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
+import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
+import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
+import org.springframework.data.elasticsearch.core.query.StringQuery;
+import org.springframework.data.elasticsearch.entities.Person;
+import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
+import org.springframework.data.repository.Repository;
+import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
+
+/**
+ * @author Christoph Strobl
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ElasticsearchStringQueryUnitTests {
+
+	@Mock ElasticsearchOperations operations;
+	ElasticsearchConverter converter;
+
+	@Before
+	public void setUp() {
+		converter = new MappingElasticsearchConverter(new SimpleElasticsearchMappingContext());
+	}
+
+	@Test // DATAES-552
+	public void shouldReplaceParametersCorrectly() throws Exception {
+
+		org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByName", "Luke");
+
+		assertThat(query).isInstanceOf(StringQuery.class);
+		assertThat(((StringQuery) query).getSource())
+				.isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }");
+	}
+
+	@Test // DATAES-552
+	public void shouldReplaceRepeatedParametersCorrectly() throws Exception {
+
+		org.springframework.data.elasticsearch.core.query.Query query = createQuery("findWithRepeatedPlaceholder", "zero",
+				"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven");
+
+		assertThat(query).isInstanceOf(StringQuery.class);
+		assertThat(((StringQuery) query).getSource())
+				.isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)");
+	}
+
+	private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args)
+			throws NoSuchMethodException {
+
+		Class<?>[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(size -> new Class[size]);
+		ElasticsearchQueryMethod queryMethod = getQueryMethod(methodName, argTypes);
+		ElasticsearchStringQuery elasticsearchStringQuery = queryForMethod(queryMethod);
+		return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
+	}
+
+	private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) {
+		return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery());
+	}
+
+	private ElasticsearchQueryMethod getQueryMethod(String name, Class<?>... parameters) throws NoSuchMethodException {
+
+		Method method = SampleRepository.class.getMethod(name, parameters);
+		return new ElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
+				new SpelAwareProxyProjectionFactory(), converter.getMappingContext());
+	}
+
+	private interface SampleRepository extends Repository<Person, String> {
+
+		@Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }")
+		Person findByName(String name);
+
+		@Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)")
+		Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5,
+				String arg6, String arg7, String arg8, String arg9, String arg10, String arg11);
+	}
+}

src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java

@@ -21,6 +21,7 @@ import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 
 import org.junit.Before;
 import org.junit.Ignore;
@@ -35,7 +36,6 @@ import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchC
 import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
 import org.springframework.data.elasticsearch.core.query.StringQuery;
 import org.springframework.data.elasticsearch.entities.Person;
-import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
 import org.springframework.data.repository.Repository;
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
@@ -90,25 +90,52 @@ public class ReactiveElasticsearchStringQueryUnitTests {
 	@Test // DATAES-552
 	public void shouldReplaceLotsOfParametersCorrectly() throws Exception {
 
-		ReactiveElasticsearchStringQuery elasticsearchStringQuery = createQueryForMethod("findWithQuiteSomeParameters",
+		org.springframework.data.elasticsearch.core.query.Query query = createQuery("findWithQuiteSomeParameters", "zero",
-				String.class, String.class, String.class, String.class, String.class, String.class, String.class, String.class,
+				"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven");
-				String.class, String.class, String.class, String.class);
+
-		StubParameterAccessor accesor = new StubParameterAccessor("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k",
-				"l");
-		org.springframework.data.elasticsearch.core.query.Query query = elasticsearchStringQuery.createQuery(accesor);
-		StringQuery reference = new StringQuery("name:(a, b, c, d, e, f, g, h, i, j, k, l)");
 		assertThat(query).isInstanceOf(StringQuery.class);
-		assertThat(((StringQuery) query).getSource()).isEqualTo(reference.getSource());
+		assertThat(((StringQuery) query).getSource())
+				.isEqualTo("name:(zero, one, two, three, four, five, six, seven, eight, nine, ten, eleven)");
+	}
+
+	@Test // DATAES-552
+	public void shouldReplaceRepeatedParametersCorrectly() throws Exception {
+
+		org.springframework.data.elasticsearch.core.query.Query query = createQuery("findWithRepeatedPlaceholder", "zero",
+				"one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven");
+
+		assertThat(query).isInstanceOf(StringQuery.class);
+		assertThat(((StringQuery) query).getSource())
+				.isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)");
+	}
+
+	private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args)
+			throws NoSuchMethodException {
+
+		Class<?>[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(size -> new Class[size]);
+		ReactiveElasticsearchQueryMethod queryMethod = getQueryMethod(methodName, argTypes);
+		ReactiveElasticsearchStringQuery elasticsearchStringQuery = queryForMethod(queryMethod);
+		
+		return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
+	}
+
+	private ReactiveElasticsearchStringQuery queryForMethod(ReactiveElasticsearchQueryMethod queryMethod) {
+		return new ReactiveElasticsearchStringQuery(queryMethod, operations, PARSER,
+				QueryMethodEvaluationContextProvider.DEFAULT);
+	}
+
+	private ReactiveElasticsearchQueryMethod getQueryMethod(String name, Class<?>... parameters)
+			throws NoSuchMethodException {
+
+		Method method = SampleRepository.class.getMethod(name, parameters);
+		return new ReactiveElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
+				new SpelAwareProxyProjectionFactory(), converter.getMappingContext());
 	}
 
 	private ReactiveElasticsearchStringQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
 
-		Method method = SampleRepository.class.getMethod(name, parameters);
+		ReactiveElasticsearchQueryMethod queryMethod = getQueryMethod(name, parameters);
-		ProjectionFactory factory = new SpelAwareProxyProjectionFactory();
+		return queryForMethod(queryMethod);
-		ReactiveElasticsearchQueryMethod queryMethod = new ReactiveElasticsearchQueryMethod(method,
-				new DefaultRepositoryMetadata(SampleRepository.class), factory, converter.getMappingContext());
-		return new ReactiveElasticsearchStringQuery(queryMethod, operations, PARSER,
-				QueryMethodEvaluationContextProvider.DEFAULT);
 	}
 
 	private interface SampleRepository extends Repository<Person, String> {
@@ -122,5 +149,9 @@ public class ReactiveElasticsearchStringQueryUnitTests {
 		@Query(value = "name:(?0, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11)")
 		Person findWithQuiteSomeParameters(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5,
 				String arg6, String arg7, String arg8, String arg9, String arg10, String arg11);
+
+		@Query(value = "name:(?0, ?11, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?0, ?1)")
+		Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String arg3, String arg4, String arg5,
+				String arg6, String arg7, String arg8, String arg9, String arg10, String arg11);
 	}
 }