From 1794cd99cb043d259e50861c41d960945e3995d7 Mon Sep 17 00:00:00 2001 From: Niklas Herder Date: Sat, 3 Jul 2021 13:42:56 +0200 Subject: [PATCH] Support collection parameters in @Query methods. Original Pull Request #1856 Closes #1858 (cherry picked from commit 6f84a1c589998ffb47b8a0f97446073ecaa396b8) (cherry picked from commit 254948d1c9ca3e517f26906c6fe79c6b368e6921) (cherry picked from commit 979c164135dc3140d48c64f9bb1cf403a4f17ce8) (cherry picked from commit d6cfc20a69898d7c135377d84809051de3e09d83) --- .../repository/support/StringQueryUtil.java | 32 ++++++- .../ElasticsearchStringQueryUnitTests.java | 91 +++++++++++++++---- 2 files changed, 100 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java index 2e063dc45..9814a91fa 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java @@ -15,8 +15,10 @@ */ package org.springframework.data.elasticsearch.repository.support; +import java.util.Collection; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.elasticsearch.core.convert.DateTimeConverters; @@ -27,6 +29,7 @@ import org.springframework.util.NumberUtils; /** * @author Peter-Josef Meisch + * @author Niklas Herder */ final public class StringQueryUtil { @@ -70,6 +73,28 @@ final public class StringQueryUtil { // noinspection ConstantConditions if (parameter != null) { + parameterValue = convert(parameter); + } + + return parameterValue; + + } + + private static String convert(Object parameter) { + if (Collection.class.isAssignableFrom(parameter.getClass())) { + Collection collectionParam = (Collection) parameter; + StringBuilder sb = new StringBuilder("["); + sb.append(collectionParam.stream().map(o -> { + if (o instanceof String) { + return "\"" + convert(o) + "\""; + } else { + return convert(o); + } + }).collect(Collectors.joining(","))); + sb.append("]"); + return sb.toString(); + } else { + String parameterValue = "null"; if (conversionService.canConvert(parameter.getClass(), String.class)) { String converted = conversionService.convert(parameter, String.class); @@ -79,11 +104,10 @@ final public class StringQueryUtil { } else { parameterValue = parameter.toString(); } + + parameterValue = parameterValue.replaceAll("\"", Matcher.quoteReplacement("\\\"")); + return parameterValue; } - - parameterValue = parameterValue.replaceAll("\"", Matcher.quoteReplacement("\\\"")); - return parameterValue; - } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java index 784b17310..f5dd7f736 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTests.java @@ -24,6 +24,7 @@ import lombok.NoArgsConstructor; import lombok.Setter; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -50,10 +51,12 @@ import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.lang.Nullable; /** * @author Christoph Strobl * @author Peter-Josef Meisch + * @author Niklas Herder */ @RunWith(MockitoJUnitRunner.class) public class ElasticsearchStringQueryUnitTests { @@ -97,7 +100,39 @@ public class ElasticsearchStringQueryUnitTests { .isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}"); } - private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args) + @Test // #1858 + public void shouldOnlyEscapeStringQueryParameters() throws Exception { + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByAge", Integer.valueOf(30)); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()).isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'age' : 30 } } } }"); + + } + + @Test // #1858 + public void shouldOnlyEscapeStringCollectionQueryParameters() throws Exception { + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByAgeIn", + new ArrayList<>(Arrays.asList(30, 35, 40))); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'age' : [30,35,40] } } } }"); + + } + + @Test // #1858 + public void shouldEscapeStringsInCollectionsQueryParameters() throws Exception { + + final List another_string = Arrays.asList("hello \"Stranger\"", "Another string"); + List params = new ArrayList<>(another_string); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameIn", params); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()).isEqualTo( + "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); + } + + private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, Object... args) throws NoSuchMethodException { Class[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(size -> new Class[size]); @@ -117,24 +152,55 @@ public class ElasticsearchStringQueryUnitTests { new SpelAwareProxyProjectionFactory(), converter.getMappingContext()); } + private interface SampleRepository extends Repository { + + @Query("{ 'bool' : { 'must' : { 'term' : { 'age' : ?0 } } } }") + List findByAge(Integer age); + + @Query("{ 'bool' : { 'must' : { 'term' : { 'age' : ?0 } } } }") + List findByAgeIn(ArrayList age); + + @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }") + Person findByName(String name); + + @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : ?0 } } } }") + Person findByNameIn(ArrayList names); + + @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); + + @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") + List findByPrefix(String prefix); + } + /** * @author Rizwan Idrees * @author Mohsin Husen * @author Artur Konczak + * @author Niklas Herder */ @Document(indexName = "test-index-person-query-unittest", type = "user", shards = 1, replicas = 0, refreshInterval = "-1") static class Person { - @Id private String id; + @Nullable public int age; + @Nullable @Id private String id; + @Nullable private String name; + @Nullable @Field(type = FieldType.Nested) private List car; + @Nullable @Field(type = FieldType.Nested, includeInParent = true) private List books; - private String name; + @Nullable + public int getAge() { + return age; + } - @Field(type = FieldType.Nested) private List car; - - @Field(type = FieldType.Nested, includeInParent = true) private List books; + public void setAge(int age) { + this.age = age; + } + @Nullable public String getId() { return id; } @@ -168,19 +234,6 @@ public class ElasticsearchStringQueryUnitTests { } } - private interface SampleRepository extends Repository { - - @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); - - @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") - List findByPrefix(String prefix); - } - /** * @author Rizwan Idrees * @author Mohsin Husen