From 7105c29b32052a2331e6203911e44cd43c48fad1 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Sat, 13 Jan 2024 22:25:21 +0800 Subject: [PATCH 01/11] Add support for SpEL in @Query --- .../elasticsearch-repository-queries.adoc | 2 +- .../query/ElasticsearchStringQuery.java | 54 ++++++- ...sticsearchCollectionToStringConverter.java | 79 +++++++++++ .../ElasticsearchRepositoryFactory.java | 27 +++- ...omMethodRepositoryELCIntegrationTests.java | 9 ++ ...ustomMethodRepositoryIntegrationTests.java | 134 ++++++++++++++++++ .../repositories/custommethod/QueryParam.java | 25 ++++ .../ElasticsearchStringQueryUnitTests.java | 74 +++++++++- .../utils/IndexNameProvider.java | 2 +- 9 files changed, 396 insertions(+), 10 deletions(-) create mode 100644 src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java diff --git a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc index 2b725a820..a39804280 100644 --- a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc +++ b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc @@ -316,7 +316,7 @@ Repository methods can be defined to have the following return types for returni .Declare query on the method using the `@Query` annotation. ==== -The arguments passed to the method can be inserted into placeholders in the query string. the placeholders are of the form `?0`, `?1`, `?2` etc. for the first, second, third parameter and so on. +The arguments passed to the method can be inserted into placeholders in the query string. The placeholders are of the form `?0`, `?1`, `?2` etc. for the first, second, third parameter and so on. [source,java] ---- interface BookRepository extends ElasticsearchRepository { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index 9807e6a54..2a9a64c37 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -17,11 +17,22 @@ import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.query.BaseQuery; -import org.springframework.data.elasticsearch.core.query.Query; import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; +import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.expression.ParserContext; +import org.springframework.expression.TypeConverter; +import org.springframework.expression.common.LiteralExpression; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + /** * ElasticsearchStringQuery * @@ -30,16 +41,27 @@ * @author Mark Paluch * @author Taylor Ono * @author Peter-Josef Meisch + * @author Haibo Liu */ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQuery { private final String queryString; + private static final SpelExpressionParser PARSER = new SpelExpressionParser(); + private static final Map QUERY_EXPRESSIONS = new ConcurrentHashMap<>(); + private final QueryMethodEvaluationContextProvider evaluationContextProvider; + private final TypeConverter typeConverter; public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations, - String queryString) { + String queryString, QueryMethodEvaluationContextProvider evaluationContextProvider, + TypeConverter typeConverter) { super(queryMethod, elasticsearchOperations); Assert.notNull(queryString, "Query cannot be empty"); + Assert.notNull(evaluationContextProvider, "ExpressionEvaluationContextProvider must not be null"); + Assert.notNull(typeConverter, "TypeConverter must not be null"); + this.queryString = queryString; + this.evaluationContextProvider = evaluationContextProvider; + this.typeConverter = typeConverter; } @Override @@ -58,12 +80,36 @@ protected boolean isExistsQuery() { } protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor parameterAccessor) { - String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService()) .replacePlaceholders(this.queryString, parameterAccessor); - var query = new StringQuery(queryString); + var query = new StringQuery(parseSpEL(queryString, parameterAccessor)); query.addSort(parameterAccessor.getSort()); return query; } + + private String parseSpEL(String queryString, ElasticsearchParametersParameterAccessor parameterAccessor) { + Expression expr = getQueryExpression(queryString); + if (expr != null) { + EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), + parameterAccessor.getValues()); + if (context instanceof StandardEvaluationContext standardEvaluationContext) { + standardEvaluationContext.setTypeConverter(typeConverter); + } + + String parsed = expr.getValue(context, String.class); + Assert.notNull(parsed, "Query parsed by SpEL should not be null"); + return parsed; + } + return queryString; + } + + @Nullable + private Expression getQueryExpression(String queryString) { + return QUERY_EXPRESSIONS.computeIfAbsent(queryString, f -> { + Expression expr = PARSER.parseExpression(queryString, ParserContext.TEMPLATE_EXPRESSION); + return expr instanceof LiteralExpression ? null : expr; + }); + + } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java new file mode 100644 index 000000000..8a4d38e17 --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java @@ -0,0 +1,79 @@ +/* + * Copyright 2024 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.support; + +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.GenericConverter; +import org.springframework.lang.Nullable; + +import java.util.Collection; +import java.util.Collections; +import java.util.Set; +import java.util.StringJoiner; +import java.util.regex.Matcher; + +/** + * Convert a collection into string for value part of the elasticsearch query. + *

+ * The string should be wrapped with square brackets, with each element quoted therefore escaped if quotes exist in the + * original element. + *

+ * eg: The value part of an elasticsearch terms query should looks like {@code ["hello \"Stranger\"","Another string"]}, + * and the whole query string may be + * {@code { 'bool' : { 'must' : { 'terms' : { 'name' : ["hello \"Stranger\"","Another string"] } } } }} + * + * @author Haibo Liu + */ +public class ElasticsearchCollectionToStringConverter implements GenericConverter { + + private static final String DELIMITER = ","; + + private final ConversionService conversionService; + + public ElasticsearchCollectionToStringConverter(ConversionService conversionService) { + this.conversionService = conversionService; + } + + @Override + public Set getConvertibleTypes() { + return Collections.singleton(new ConvertiblePair(Collection.class, String.class)); + } + + @Override + @Nullable + public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + if (source == null) { + return null; + } + Collection sourceCollection = (Collection) source; + if (sourceCollection.isEmpty()) { + return "[]"; + } + StringJoiner sb = new StringJoiner(DELIMITER, "[", "]"); + for (Object sourceElement : sourceCollection) { + Object targetElement = this.conversionService.convert( + sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); + sb.add("\"" + escape(targetElement) + "\""); + } + return sb.toString(); + } + + private String escape(@Nullable Object target) { + // escape the quotes in the string, because the string should already be quoted manually + return String.valueOf(target).replaceAll("\"", Matcher.quoteReplacement("\\\"")); + } +} diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java index 7b5f82771..b084461bd 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java @@ -15,6 +15,9 @@ */ package org.springframework.data.elasticsearch.repository.support; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.converter.ConverterRegistry; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.repository.ElasticsearchRepository; import org.springframework.data.elasticsearch.repository.query.ElasticsearchPartQuery; @@ -34,6 +37,8 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.expression.TypeConverter; +import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -54,6 +59,7 @@ * @author Sascha Woo * @author Peter-Josef Meisch * @author Ezequiel Antúnez Camacho + * @author Haibo Liu */ public class ElasticsearchRepositoryFactory extends RepositoryFactorySupport { @@ -96,11 +102,24 @@ private static boolean isQueryDslRepository(Class repositoryInterface) { @Override protected Optional getQueryLookupStrategy(@Nullable Key key, QueryMethodEvaluationContextProvider evaluationContextProvider) { - return Optional.of(new ElasticsearchQueryLookupStrategy()); + return Optional.of(new ElasticsearchQueryLookupStrategy(evaluationContextProvider)); } private class ElasticsearchQueryLookupStrategy implements QueryLookupStrategy { + QueryMethodEvaluationContextProvider evaluationContextProvider; + private TypeConverter typeConverter; + + ElasticsearchQueryLookupStrategy(QueryMethodEvaluationContextProvider evaluationContextProvider) { + this.evaluationContextProvider = evaluationContextProvider; + + // register elasticsearch custom type converter for conversion service + ConversionService conversionService = new DefaultConversionService(); + ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; + converterRegistry.addConverter(new ElasticsearchCollectionToStringConverter(conversionService)); + typeConverter = new StandardTypeConverter(conversionService); + } + /* * (non-Javadoc) * @see org.springframework.data.repository.query.QueryLookupStrategy#resolveQuery(java.lang.reflect.Method, org.springframework.data.repository.core.RepositoryMetadata, org.springframework.data.projection.ProjectionFactory, org.springframework.data.repository.core.NamedQueries) @@ -115,9 +134,11 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata, if (namedQueries.hasQuery(namedQueryName)) { String namedQuery = namedQueries.getQuery(namedQueryName); - return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, namedQuery); + return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, namedQuery, + evaluationContextProvider, typeConverter); } else if (queryMethod.hasAnnotatedQuery()) { - return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, queryMethod.getAnnotatedQuery()); + return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, queryMethod.getAnnotatedQuery(), + evaluationContextProvider, typeConverter); } return new ElasticsearchPartQuery(queryMethod, elasticsearchOperations); } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java index c7b5615ed..21780e3cd 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java @@ -25,6 +25,7 @@ /** * @author Peter-Josef Meisch + * @author Haibo Liu * @since 4.4 */ @ContextConfiguration(classes = { CustomMethodRepositoryELCIntegrationTests.Config.class }) @@ -40,5 +41,13 @@ static class Config { IndexNameProvider indexNameProvider() { return new IndexNameProvider("custom-method-repository"); } + + /** + * a normal bean referenced by SpEL in query + */ + @Bean + QueryParam queryParam() { + return new QueryParam("abc"); + } } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index d21f00114..965dff5b8 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -1522,6 +1522,65 @@ void shouldReturnSearchHitsForStringQuery() { assertThat(searchHits.getTotalHits()).isEqualTo(20); } + @Test + void shouldReturnSearchHitsForStringQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + // when + SearchHits searchHits = repository.queryByStringSpEL("abc"); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + + @Test + void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + QueryParam param = new QueryParam("abc"); + // when + SearchHits searchHits = repository.queryByParameterPropertySpEL(param); + repository.queryByBeanPropertySpEL(); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + + @Test + void shouldReturnSearchHitsForBeanQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + // when + SearchHits searchHits = repository.queryByBeanPropertySpEL(); + repository.queryByBeanPropertySpEL(); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + + @Test + void shouldReturnSearchHitsForCollectionQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + // when + SearchHits searchHits = repository.queryByCollectionSpEL(List.of("abc")); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + + @Test + void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + QueryParam param = new QueryParam("abc"); + // when + SearchHits searchHits = repository.queryByParameterPropertyCollectionSpEL(List.of(param)); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + @Test // DATAES-372 void shouldReturnHighlightsOnAnnotatedMethod() { List entities = createSampleEntities("abc", 2); @@ -1940,6 +1999,81 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByString(String type); + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "type":"#{#type}" + } + } + ] + } + } + """) + SearchHits queryByStringSpEL(String type); + + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "type":"#{#entity.q}" + } + } + ] + } + } + """) + SearchHits queryByParameterPropertySpEL(QueryParam entity); + + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "type":"#{@queryParam.q}" + } + } + ] + } + } + """) + SearchHits queryByBeanPropertySpEL(); + + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "type": #{#types} + } + } + ] + } + } + """) + SearchHits queryByCollectionSpEL(Collection types); + + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "type": #{#entities.![q]} + } + } + ] + } + } + """) + SearchHits queryByParameterPropertyCollectionSpEL(Collection entities); + @Query(""" { "bool":{ diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java new file mode 100644 index 000000000..9b1dbc401 --- /dev/null +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java @@ -0,0 +1,25 @@ +/* + * Copyright 2024 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.repositories.custommethod; + +/** + * Used as a parameter referenced by SpEL in query method. + * + * @param q content + * @author Haibo Liu + */ +public record QueryParam(String q) { +} 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 b7e9e84a3..ee7deb639 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 @@ -32,6 +32,9 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.converter.ConverterRegistry; +import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; @@ -42,15 +45,21 @@ import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.query.StringQuery; +import org.springframework.data.elasticsearch.repositories.custommethod.QueryParam; +import org.springframework.data.elasticsearch.repository.support.ElasticsearchCollectionToStringConverter; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; +import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.expression.TypeConverter; +import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.lang.Nullable; /** * @author Christoph Strobl * @author Peter-Josef Meisch * @author Niklas Herder + * @author Haibo Liu */ @ExtendWith(MockitoExtension.class) public class ElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase { @@ -83,6 +92,51 @@ public void shouldReplaceRepeatedParametersCorrectly() throws Exception { .isEqualTo("name:(zero, eleven, one, two, three, four, five, six, seven, eight, nine, ten, eleven, zero, one)"); } + @Test + public void shouldReplaceParametersSpEL() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", "Luke"); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); + } + + @Test + public void shouldUseParameterPropertySpEL() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByParameterPropertySpEL", + new QueryParam("Luke")); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); + } + + @Test + public void shouldReplaceCollectionSpEL() 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("findByNamesSpEL", params); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()).isEqualTo( + "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); + } + + @Test + public void shouldReplaceCollectionParametersSpEL() throws Exception { + + final List another_string = List.of(new QueryParam("hello \"Stranger\""), new QueryParam("Another string")); + List params = new ArrayList<>(another_string); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesParameterSpEL", params); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()).isEqualTo( + "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); + } + @Test // #1790 @DisplayName("should escape Strings in query parameters") void shouldEscapeStringsInQueryParameters() throws Exception { @@ -166,7 +220,13 @@ void shouldUseConverterOnParameters() throws NoSuchMethodException { } private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) { - return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery()); + ConversionService conversionService = new DefaultConversionService(); + ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; + converterRegistry.addConverter(new ElasticsearchCollectionToStringConverter(conversionService)); + TypeConverter typeConverter = new StandardTypeConverter(conversionService); + + return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery(), + QueryMethodEvaluationContextProvider.DEFAULT, typeConverter); } private ElasticsearchQueryMethod getQueryMethod(String name, Class... parameters) throws NoSuchMethodException { @@ -187,9 +247,21 @@ private interface SampleRepository extends Repository { @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }") Person findByName(String name); + @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{#name}' } } } }") + Person findByNameSpEL(String name); + + @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{#param.q}' } } } }") + Person findByParameterPropertySpEL(QueryParam param); + @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : ?0 } } } }") Person findByNameIn(ArrayList names); + @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : #{#names} } } } }") + Person findByNamesSpEL(ArrayList names); + + @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : #{#names.![q]} } } } }") + Person findByNamesParameterSpEL(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); diff --git a/src/test/java/org/springframework/data/elasticsearch/utils/IndexNameProvider.java b/src/test/java/org/springframework/data/elasticsearch/utils/IndexNameProvider.java index 130c35c9a..0b19964af 100644 --- a/src/test/java/org/springframework/data/elasticsearch/utils/IndexNameProvider.java +++ b/src/test/java/org/springframework/data/elasticsearch/utils/IndexNameProvider.java @@ -16,7 +16,7 @@ package org.springframework.data.elasticsearch.utils; /** - * Class providing an index name with a prefix and a index number + * Class providing an index name with a prefix and an index number * * @author Peter-Josef Meisch */ From d205d252b3000aa09eafcf3efc08a9b5ec55cdb9 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Sat, 13 Jan 2024 23:09:48 +0800 Subject: [PATCH 02/11] Add doc --- .../elasticsearch-repository-queries.adoc | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc index a39804280..ec176b9f2 100644 --- a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc +++ b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc @@ -361,3 +361,174 @@ would make an https://www.elastic.co/guide/en/elasticsearch/reference/current/qu } ---- ==== + +[[elasticsearch.query-methods.at-query.spel]] +=== Using SpEL Expressions + +.Declare query on the method using the `@Query` annotation with SpEL expression. +==== +https://docs.spring.io/spring-framework/reference/core/expressions.html[SpEL expression] is also supported when defining query in `@Query`. + + +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "name":"#{#name}" + } + } + ] + } + } + """) + Page findByName(String name,Pageable pageable); +} +---- + +If for example the function is called with the parameter _John_, it would produce the following query body: + +[source,json] +---- +{ + "bool":{ + "must":[ + { + "term":{ + "name":"John" + } + } + ] + } +} +---- +==== + +.accessing entity property. +==== +Supposing that we have the following entity class as query parameter type: +[source,java] +---- +public record QueryParam(String q) { +} +---- + +It's easy to access the parameter entity by `#` symbol, then reference the property `q` with a simple `.`: + +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "name":"#{#entity.q}" + } + } + ] + } + } + """) + Page findByName(QueryParam entity,Pageable pageable); +} +---- + +We can pass `new QueryParam("John")` as the parameter now, and it will produce the same query string as above. +==== + +.accessing bean property. +==== +https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/bean-references.html[Bean property] is also supported as entity property. Given that there is a bean named `queryParam` of type `QueryParam`, we can access the bean with symbol `@` rather than `#`: +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "name":"#{@queryParam.q}" + } + } + ] + } + } + """) + Page findByName(QueryParam entity,Pageable pageable); +} +---- +==== + +.SpEL and `Collection` param. +==== +`Collection` parameter is also supported and is as easy to use as normal `String`, such as the following `terms` query: + +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "name": #{#names} + } + } + ] + } + } + """) + Page findByName(Collection names,Pageable pageable); +} +---- +A collection of `names` like `List.of("name1", "name2")` will produce the following terms query: +[source,json] +---- +{ + "bool":{ + "must":[ + { + "terms":{ + "name": ["name1", "name2"] + } + } + ] + } +} +---- +==== + +.access property in the `Collection` param. +==== +https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/collection-projection.html[SpEL Collection Projection] is convenient to use when entities in the `Collection` parameter is not plain `String`: + +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "name": #{#entities.![q]} + } + } + ] + } + } + """) + Page findByName(Collection entities,Pageable pageable); +} +---- +This will extract all the `q` property values as a new `Collection` from `QueryParam` collection, thus takes the same effect as above. +==== + From d506b04a6587f7684d714283b3c6ec924590fda4 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Sun, 14 Jan 2024 16:03:03 +0800 Subject: [PATCH 03/11] Fix string quatation escape for elasticsearch value in query --- .../query/ElasticsearchStringQuery.java | 50 ++++++++++++++---- ...archCollectionValueToStringConverter.java} | 25 ++++----- .../ElasticsearchRepositoryFactory.java | 16 ++---- ...ticsearchStringValueToStringConverter.java | 52 +++++++++++++++++++ ...asticsearchValueSpELConversionService.java | 42 +++++++++++++++ ...ustomMethodRepositoryIntegrationTests.java | 10 ++++ .../ElasticsearchStringQueryUnitTests.java | 47 ++++++++++------- 7 files changed, 188 insertions(+), 54 deletions(-) rename src/main/java/org/springframework/data/elasticsearch/repository/support/{ElasticsearchCollectionToStringConverter.java => ElasticsearchCollectionValueToStringConverter.java} (75%) create mode 100644 src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java create mode 100644 src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index 2a9a64c37..d874f6890 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -18,15 +18,20 @@ import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.query.BaseQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; +import org.springframework.data.elasticsearch.repository.support.ElasticsearchCollectionValueToStringConverter; +import org.springframework.data.elasticsearch.repository.support.ElasticsearchValueSpELConversionService; import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ParserContext; import org.springframework.expression.TypeConverter; +import org.springframework.expression.common.CompositeStringExpression; import org.springframework.expression.common.LiteralExpression; +import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -45,23 +50,22 @@ */ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQuery { - private final String queryString; private static final SpelExpressionParser PARSER = new SpelExpressionParser(); private static final Map QUERY_EXPRESSIONS = new ConcurrentHashMap<>(); + + private final String queryString; private final QueryMethodEvaluationContextProvider evaluationContextProvider; - private final TypeConverter typeConverter; + private final TypeConverter elasticsearchSpELTypeConverter; public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations, - String queryString, QueryMethodEvaluationContextProvider evaluationContextProvider, - TypeConverter typeConverter) { + String queryString, QueryMethodEvaluationContextProvider evaluationContextProvider) { super(queryMethod, elasticsearchOperations); Assert.notNull(queryString, "Query cannot be empty"); Assert.notNull(evaluationContextProvider, "ExpressionEvaluationContextProvider must not be null"); - Assert.notNull(typeConverter, "TypeConverter must not be null"); this.queryString = queryString; this.evaluationContextProvider = evaluationContextProvider; - this.typeConverter = typeConverter; + this.elasticsearchSpELTypeConverter = new StandardTypeConverter(ElasticsearchValueSpELConversionService.CONVERSION_SERVICE_LAZY); } @Override @@ -94,22 +98,50 @@ private String parseSpEL(String queryString, ElasticsearchParametersParameterAcc EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), parameterAccessor.getValues()); if (context instanceof StandardEvaluationContext standardEvaluationContext) { - standardEvaluationContext.setTypeConverter(typeConverter); + standardEvaluationContext.setTypeConverter(elasticsearchSpELTypeConverter); } - String parsed = expr.getValue(context, String.class); + String parsed = parseExpressions(expr, context); Assert.notNull(parsed, "Query parsed by SpEL should not be null"); return parsed; } return queryString; } + /** + * {@link Expression#getValue(EvaluationContext, Class)} is not used because the value part in SpEL should be converted + * by {@link org.springframework.data.elasticsearch.repository.support.ElasticsearchStringValueToStringConverter} or + * {@link ElasticsearchCollectionValueToStringConverter} to + * escape the quotations, but other literal parts in SpEL expression should not be processed with this converter. + * So we just get the string value from {@link LiteralExpression} directly rather than + * {@link LiteralExpression#getValue(EvaluationContext, Class)}. + */ + private String parseExpressions(Expression rootExpr, EvaluationContext context) { + StringBuilder parsed = new StringBuilder(); + if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { + Expression[] expressions = compositeStringExpression.getExpressions(); + + for (Expression exp : expressions) { + if (exp instanceof LiteralExpression literalExpression) { + // get the string literal directly + parsed.append(literalExpression.getExpressionString()); + } else if (exp instanceof SpelExpression spelExpression) { + // evaluate the value + parsed.append(spelExpression.getValue(context, String.class)); + } else { + // then it should be another composite expression + parsed.append(parseExpressions(exp, context)); + } + } + } + return parsed.toString(); + } + @Nullable private Expression getQueryExpression(String queryString) { return QUERY_EXPRESSIONS.computeIfAbsent(queryString, f -> { Expression expr = PARSER.parseExpression(queryString, ParserContext.TEMPLATE_EXPRESSION); return expr instanceof LiteralExpression ? null : expr; }); - } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java similarity index 75% rename from src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java rename to src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java index 8a4d38e17..0ee584eb9 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java @@ -24,27 +24,25 @@ import java.util.Collections; import java.util.Set; import java.util.StringJoiner; -import java.util.regex.Matcher; /** * Convert a collection into string for value part of the elasticsearch query. *

- * The string should be wrapped with square brackets, with each element quoted therefore escaped if quotes exist in the - * original element. + * The string should be wrapped with square brackets, with each element quoted therefore + * escaped(by {@link ElasticsearchStringValueToStringConverter}) if quotations exist in the original element. *

* eg: The value part of an elasticsearch terms query should looks like {@code ["hello \"Stranger\"","Another string"]}, - * and the whole query string may be - * {@code { 'bool' : { 'must' : { 'terms' : { 'name' : ["hello \"Stranger\"","Another string"] } } } }} + * for query {@code { 'bool' : { 'must' : { 'terms' : { 'name' : ["hello \"Stranger\"","Another string"] } } } }} * * @author Haibo Liu */ -public class ElasticsearchCollectionToStringConverter implements GenericConverter { +public class ElasticsearchCollectionValueToStringConverter implements GenericConverter { private static final String DELIMITER = ","; private final ConversionService conversionService; - public ElasticsearchCollectionToStringConverter(ConversionService conversionService) { + public ElasticsearchCollectionValueToStringConverter(ConversionService conversionService) { this.conversionService = conversionService; } @@ -57,7 +55,7 @@ public Set getConvertibleTypes() { @Nullable public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { if (source == null) { - return null; + return "[]"; } Collection sourceCollection = (Collection) source; if (sourceCollection.isEmpty()) { @@ -67,13 +65,12 @@ public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDe for (Object sourceElement : sourceCollection) { Object targetElement = this.conversionService.convert( sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); - sb.add("\"" + escape(targetElement) + "\""); + if (sourceElement instanceof String) { + sb.add("\"" + targetElement + "\""); + } else { + sb.add(String.valueOf(targetElement)); + } } return sb.toString(); } - - private String escape(@Nullable Object target) { - // escape the quotes in the string, because the string should already be quoted manually - return String.valueOf(target).replaceAll("\"", Matcher.quoteReplacement("\\\"")); - } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java index b084461bd..2752b5464 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchRepositoryFactory.java @@ -15,9 +15,6 @@ */ package org.springframework.data.elasticsearch.repository.support; -import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.converter.ConverterRegistry; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.repository.ElasticsearchRepository; import org.springframework.data.elasticsearch.repository.query.ElasticsearchPartQuery; @@ -107,17 +104,10 @@ protected Optional getQueryLookupStrategy(@Nullable Key key private class ElasticsearchQueryLookupStrategy implements QueryLookupStrategy { - QueryMethodEvaluationContextProvider evaluationContextProvider; - private TypeConverter typeConverter; + private final QueryMethodEvaluationContextProvider evaluationContextProvider; ElasticsearchQueryLookupStrategy(QueryMethodEvaluationContextProvider evaluationContextProvider) { this.evaluationContextProvider = evaluationContextProvider; - - // register elasticsearch custom type converter for conversion service - ConversionService conversionService = new DefaultConversionService(); - ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; - converterRegistry.addConverter(new ElasticsearchCollectionToStringConverter(conversionService)); - typeConverter = new StandardTypeConverter(conversionService); } /* @@ -135,10 +125,10 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata, if (namedQueries.hasQuery(namedQueryName)) { String namedQuery = namedQueries.getQuery(namedQueryName); return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, namedQuery, - evaluationContextProvider, typeConverter); + evaluationContextProvider); } else if (queryMethod.hasAnnotatedQuery()) { return new ElasticsearchStringQuery(queryMethod, elasticsearchOperations, queryMethod.getAnnotatedQuery(), - evaluationContextProvider, typeConverter); + evaluationContextProvider); } return new ElasticsearchPartQuery(queryMethod, elasticsearchOperations); } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java new file mode 100644 index 000000000..da87bf86e --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java @@ -0,0 +1,52 @@ +/* + * Copyright 2024 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.support; + +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.GenericConverter; +import org.springframework.lang.Nullable; + +import java.util.Collections; +import java.util.Set; +import java.util.regex.Matcher; + +/** + * Values in elasticsearch query may contain quotations and should be escaped when converting. + * However, this converter should only be used in this situation, rather than common string to string conversions. + * + * @author Haibo Liu + */ +public class ElasticsearchStringValueToStringConverter implements GenericConverter { + + @Override + public Set getConvertibleTypes() { + return Collections.singleton(new ConvertiblePair(String.class, String.class)); + } + + @Override + @Nullable + public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + if (source == null) { + return "null"; + } + return escape(source); + } + + private String escape(@Nullable Object source) { + // escape the quotes in the string, because the string should already be quoted manually + return String.valueOf(source).replaceAll("\"", Matcher.quoteReplacement("\\\"")); + } +} diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java new file mode 100644 index 000000000..1686ef231 --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java @@ -0,0 +1,42 @@ +/* + * Copyright 2024 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.support; + +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.converter.ConverterRegistry; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.data.util.Lazy; + +/** + * To supply a {@link ConversionService} with custom converter to handle SpEL value in elasticsearch query. + * This service should only be used in this situation. + * + * @author Haibo Liu + */ +public class ElasticsearchValueSpELConversionService { + + public static final Lazy CONVERSION_SERVICE_LAZY = Lazy.of( + ElasticsearchValueSpELConversionService::buildElasticsearchConversionService); + + private static ConversionService buildElasticsearchConversionService() { + // register elasticsearch custom type converter for conversion service + ConversionService conversionService = new DefaultConversionService(); + ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; + converterRegistry.addConverter(new ElasticsearchCollectionValueToStringConverter(conversionService)); + converterRegistry.addConverter(new ElasticsearchStringValueToStringConverter()); + return conversionService; + } +} diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index 965dff5b8..34886f065 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -1569,6 +1569,16 @@ void shouldReturnSearchHitsForCollectionQuerySpEL() { assertThat(searchHits.getTotalHits()).isEqualTo(20); } + @Test + void shouldReturnSearchHitsForEmptyCollectionQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + // when + SearchHits searchHits = repository.queryByCollectionSpEL(List.of()); + assertThat(searchHits.getTotalHits()).isEqualTo(0); + } + @Test void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { List entities = createSampleEntities("abc", 20); 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 ee7deb639..85eb592c6 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 @@ -32,9 +32,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.core.convert.ConversionService; -import org.springframework.core.convert.converter.ConverterRegistry; -import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; @@ -46,13 +43,10 @@ import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.repositories.custommethod.QueryParam; -import org.springframework.data.elasticsearch.repository.support.ElasticsearchCollectionToStringConverter; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; -import org.springframework.expression.TypeConverter; -import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.lang.Nullable; /** @@ -99,7 +93,17 @@ public void shouldReplaceParametersSpEL() throws Exception { assertThat(query).isInstanceOf(StringQuery.class); assertThat(((StringQuery) query).getSource()) - .isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); + .isEqualTo("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"Luke\" } } } }"); + } + + @Test + public void shouldReplaceParametersSpELWithQuotes() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", "hello \"world\""); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()) + .isEqualTo("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"hello \\\"world\\\"\" } } } }"); } @Test @@ -116,8 +120,8 @@ public void shouldUseParameterPropertySpEL() throws Exception { @Test public void shouldReplaceCollectionSpEL() throws Exception { - final List another_string = Arrays.asList("hello \"Stranger\"", "Another string"); - List params = new ArrayList<>(another_string); + final List anotherString = Arrays.asList("hello \"Stranger\"", "Another string"); + List params = new ArrayList<>(anotherString); org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); assertThat(query).isInstanceOf(StringQuery.class); @@ -125,11 +129,23 @@ public void shouldReplaceCollectionSpEL() throws Exception { "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); } + @Test + public void shouldReplaceEmptyCollectionSpEL() throws Exception { + + final List anotherString = List.of(); + List params = new ArrayList<>(anotherString); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + + assertThat(query).isInstanceOf(StringQuery.class); + assertThat(((StringQuery) query).getSource()).isEqualTo( + "{ 'bool' : { 'must' : { 'terms' : { 'name' : [] } } } }"); + } + @Test public void shouldReplaceCollectionParametersSpEL() throws Exception { - final List another_string = List.of(new QueryParam("hello \"Stranger\""), new QueryParam("Another string")); - List params = new ArrayList<>(another_string); + final List anotherString = List.of(new QueryParam("hello \"Stranger\""), new QueryParam("Another string")); + List params = new ArrayList<>(anotherString); org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesParameterSpEL", params); assertThat(query).isInstanceOf(StringQuery.class); @@ -220,13 +236,8 @@ void shouldUseConverterOnParameters() throws NoSuchMethodException { } private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) { - ConversionService conversionService = new DefaultConversionService(); - ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; - converterRegistry.addConverter(new ElasticsearchCollectionToStringConverter(conversionService)); - TypeConverter typeConverter = new StandardTypeConverter(conversionService); - return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery(), - QueryMethodEvaluationContextProvider.DEFAULT, typeConverter); + QueryMethodEvaluationContextProvider.DEFAULT); } private ElasticsearchQueryMethod getQueryMethod(String name, Class... parameters) throws NoSuchMethodException { @@ -247,7 +258,7 @@ private interface SampleRepository extends Repository { @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }") Person findByName(String name); - @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{#name}' } } } }") + @Query("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"#{#name}\" } } } }") Person findByNameSpEL(String name); @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{#param.q}' } } } }") From 9d3883485c5cfca2bf19f2babbaa07e81a1c71ba Mon Sep 17 00:00:00 2001 From: puppylpg Date: Sun, 14 Jan 2024 16:57:10 +0800 Subject: [PATCH 04/11] modify SpEL expression procession --- .../query/ElasticsearchStringQuery.java | 25 ++++++++++--------- ...ticsearchStringValueToStringConverter.java | 2 +- ...asticsearchValueSpELConversionService.java | 1 - 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index d874f6890..0fb5e97d5 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -112,27 +112,28 @@ private String parseSpEL(String queryString, ElasticsearchParametersParameterAcc * {@link Expression#getValue(EvaluationContext, Class)} is not used because the value part in SpEL should be converted * by {@link org.springframework.data.elasticsearch.repository.support.ElasticsearchStringValueToStringConverter} or * {@link ElasticsearchCollectionValueToStringConverter} to - * escape the quotations, but other literal parts in SpEL expression should not be processed with this converter. + * escape the quotations, but other literal parts in SpEL expression should not be processed with these converters. * So we just get the string value from {@link LiteralExpression} directly rather than * {@link LiteralExpression#getValue(EvaluationContext, Class)}. */ private String parseExpressions(Expression rootExpr, EvaluationContext context) { StringBuilder parsed = new StringBuilder(); - if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { + if (rootExpr instanceof LiteralExpression literalExpression) { + // get the string literal directly + parsed.append(literalExpression.getExpressionString()); + } else if (rootExpr instanceof SpelExpression spelExpression) { + // evaluate the value + parsed.append(spelExpression.getValue(context, String.class)); + } else if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { + // then it should be another composite expression Expression[] expressions = compositeStringExpression.getExpressions(); for (Expression exp : expressions) { - if (exp instanceof LiteralExpression literalExpression) { - // get the string literal directly - parsed.append(literalExpression.getExpressionString()); - } else if (exp instanceof SpelExpression spelExpression) { - // evaluate the value - parsed.append(spelExpression.getValue(context, String.class)); - } else { - // then it should be another composite expression - parsed.append(parseExpressions(exp, context)); - } + parsed.append(parseExpressions(exp, context)); } + } else { + // no more + parsed.append(rootExpr.getValue(context, String.class)); } return parsed.toString(); } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java index da87bf86e..54974a0ea 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java @@ -25,7 +25,7 @@ /** * Values in elasticsearch query may contain quotations and should be escaped when converting. - * However, this converter should only be used in this situation, rather than common string to string conversions. + * However, the converter should only be used in this situation, rather than common string to string conversions. * * @author Haibo Liu */ diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java index 1686ef231..904486d52 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java @@ -22,7 +22,6 @@ /** * To supply a {@link ConversionService} with custom converter to handle SpEL value in elasticsearch query. - * This service should only be used in this situation. * * @author Haibo Liu */ From b72145ec6cd131a4eb4f81561e88533c3dc0e6a6 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Mon, 15 Jan 2024 17:40:18 +0800 Subject: [PATCH 05/11] Add test case with annotation --- .../query/ElasticsearchQueryMethod.java | 4 +-- .../core/SearchTemplateIntegrationTests.java | 6 ++-- ...ustomMethodRepositoryIntegrationTests.java | 30 +++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java index 5f290ea93..c1bef5990 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java @@ -69,9 +69,9 @@ */ public class ElasticsearchQueryMethod extends QueryMethod { - // the following 2 variables exits in the base class, but are private. We need them for + // the following 2 variables exists in the base class, but are private. We need them for // correct handling of return types (SearchHits), so we have our own values here. - // Alas this means that we have to copy code that initializes these variables and in the + // Also, this means that we have to copy code that initializes these variables and in the // base class uses them in order to use our variables protected final Method method; protected final Class unwrappedReturnType; diff --git a/src/test/java/org/springframework/data/elasticsearch/core/SearchTemplateIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/core/SearchTemplateIntegrationTests.java index d0f410470..a02684db2 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/SearchTemplateIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/SearchTemplateIntegrationTests.java @@ -323,9 +323,9 @@ record Person( // @Document(indexName = "#{@indexNameProvider.indexName()}-student") record Student( // - @Nullable @Id String id, // - @Field(type = FieldType.Text) String firstName, // - @Field(type = FieldType.Text) String lastName // + @Nullable @Id String id, // + @Field(type = FieldType.Text) String firstName, // + @Field(type = FieldType.Text) String lastName // ) { } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index 34886f065..d1b7325ad 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -63,6 +63,7 @@ import org.springframework.data.geo.Distance; import org.springframework.data.geo.Metrics; import org.springframework.data.geo.Point; +import org.springframework.data.repository.query.Param; import org.springframework.lang.Nullable; /** @@ -1591,6 +1592,19 @@ void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { assertThat(searchHits.getTotalHits()).isEqualTo(20); } + @Test + void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpELWithParamAnnotation() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + QueryParam param = new QueryParam("abc"); + // when + SearchHits searchHits = repository.queryByParameterPropertyCollectionSpELWithParamAnnotation( + List.of(param)); + + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + @Test // DATAES-372 void shouldReturnHighlightsOnAnnotatedMethod() { List entities = createSampleEntities("abc", 2); @@ -2084,6 +2098,22 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByParameterPropertyCollectionSpEL(Collection entities); + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "type": #{#e.![q]} + } + } + ] + } + } + """) + SearchHits queryByParameterPropertyCollectionSpELWithParamAnnotation( + @Param("e") Collection entities); + @Query(""" { "bool":{ From f88384c9fb5d526c651b5d5d8984f2072c03b594 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Wed, 17 Jan 2024 10:39:50 +0800 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: Peter-Josef Meisch --- .../repository/query/ElasticsearchQueryMethod.java | 4 ++-- .../elasticsearch/repositories/custommethod/QueryParam.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java index c1bef5990..2f98e8276 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchQueryMethod.java @@ -69,9 +69,9 @@ */ public class ElasticsearchQueryMethod extends QueryMethod { - // the following 2 variables exists in the base class, but are private. We need them for + // the following 2 variables exist in the base class, but are private. We need them for // correct handling of return types (SearchHits), so we have our own values here. - // Also, this means that we have to copy code that initializes these variables and in the + // This means that we have to copy code that initializes these variables and in the // base class uses them in order to use our variables protected final Method method; protected final Class unwrappedReturnType; diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java index 9b1dbc401..7a99f45f2 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java @@ -16,7 +16,7 @@ package org.springframework.data.elasticsearch.repositories.custommethod; /** - * Used as a parameter referenced by SpEL in query method. + * Used as a parameter referenced by SpEL in query method tests. * * @param q content * @author Haibo Liu From 7a35531f6921e03d1682d3b6efd31bd9161b3e2a Mon Sep 17 00:00:00 2001 From: puppylpg Date: Wed, 17 Jan 2024 14:35:19 +0800 Subject: [PATCH 07/11] doc and format --- .../elasticsearch-repository-queries.adoc | 64 +++++--- .../query/ElasticsearchStringQuery.java | 7 +- ...earchCollectionValueToStringConverter.java | 20 ++- ...ticsearchStringValueToStringConverter.java | 5 +- ...asticsearchValueSpELConversionService.java | 9 +- ...omMethodRepositoryELCIntegrationTests.java | 4 +- ...ustomMethodRepositoryIntegrationTests.java | 22 +-- .../{QueryParam.java => QueryParameter.java} | 4 +- .../ElasticsearchStringQueryUnitTests.java | 152 +++++++++++++++--- 9 files changed, 218 insertions(+), 69 deletions(-) rename src/main/java/org/springframework/data/elasticsearch/repository/support/{ => spel}/ElasticsearchCollectionValueToStringConverter.java (90%) rename src/main/java/org/springframework/data/elasticsearch/repository/support/{ => spel}/ElasticsearchStringValueToStringConverter.java (92%) rename src/main/java/org/springframework/data/elasticsearch/repository/support/{ => spel}/ElasticsearchValueSpELConversionService.java (85%) rename src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/{QueryParam.java => QueryParameter.java} (91%) diff --git a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc index ec176b9f2..b558e13bb 100644 --- a/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc +++ b/src/main/antora/modules/ROOT/pages/elasticsearch/repositories/elasticsearch-repository-queries.adoc @@ -379,14 +379,14 @@ interface BookRepository extends ElasticsearchRepository { "must":[ { "term":{ - "name":"#{#name}" + "name": "#{#name}" } } ] } } """) - Page findByName(String name,Pageable pageable); + Page findByName(String name, Pageable pageable); } ---- @@ -399,7 +399,7 @@ If for example the function is called with the parameter _John_, it would produc "must":[ { "term":{ - "name":"John" + "name": "John" } } ] @@ -408,16 +408,16 @@ If for example the function is called with the parameter _John_, it would produc ---- ==== -.accessing entity property. +.accessing parameter property. ==== -Supposing that we have the following entity class as query parameter type: +Supposing that we have the following class as query parameter type: [source,java] ---- -public record QueryParam(String q) { +public record QueryParameter(String value) { } ---- -It's easy to access the parameter entity by `#` symbol, then reference the property `q` with a simple `.`: +It's easy to access the parameter by `#` symbol, then reference the property `value` with a simple `.`: [source,java] ---- @@ -428,23 +428,23 @@ interface BookRepository extends ElasticsearchRepository { "must":[ { "term":{ - "name":"#{#entity.q}" + "name": "#{#parameter.value}" } } ] } } """) - Page findByName(QueryParam entity,Pageable pageable); + Page findByName(QueryParameter parameter, Pageable pageable); } ---- -We can pass `new QueryParam("John")` as the parameter now, and it will produce the same query string as above. +We can pass `new QueryParameter("John")` as the parameter now, and it will produce the same query string as above. ==== .accessing bean property. ==== -https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/bean-references.html[Bean property] is also supported as entity property. Given that there is a bean named `queryParam` of type `QueryParam`, we can access the bean with symbol `@` rather than `#`: +https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/bean-references.html[Bean property] is also supported to access. Given that there is a bean named `queryParameter` of type `QueryParameter`, we can access the bean with symbol `@` rather than `#`, and there is no need to declare a parameter of type `QueryParameter` in the query method: [source,java] ---- interface BookRepository extends ElasticsearchRepository { @@ -454,14 +454,14 @@ interface BookRepository extends ElasticsearchRepository { "must":[ { "term":{ - "name":"#{@queryParam.q}" + "name": "#{@queryParameter.value}" } } ] } } """) - Page findByName(QueryParam entity,Pageable pageable); + Page findByName(Pageable pageable); } ---- ==== @@ -486,9 +486,12 @@ interface BookRepository extends ElasticsearchRepository { } } """) - Page findByName(Collection names,Pageable pageable); + Page findByName(Collection names, Pageable pageable); } ---- + +NOTE: collection values should not be quoted when declaring the elasticsearch json query. + A collection of `names` like `List.of("name1", "name2")` will produce the following terms query: [source,json] ---- @@ -508,7 +511,7 @@ A collection of `names` like `List.of("name1", "name2")` will produce the follow .access property in the `Collection` param. ==== -https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/collection-projection.html[SpEL Collection Projection] is convenient to use when entities in the `Collection` parameter is not plain `String`: +https://docs.spring.io/spring-framework/reference/core/expressions/language-ref/collection-projection.html[SpEL Collection Projection] is convenient to use when values in the `Collection` parameter is not plain `String`: [source,java] ---- @@ -519,16 +522,41 @@ interface BookRepository extends ElasticsearchRepository { "must":[ { "terms":{ - "name": #{#entities.![q]} + "name": #{#parameters.![value]} } } ] } } """) - Page findByName(Collection entities,Pageable pageable); + Page findByName(Collection parameters, Pageable pageable); } ---- -This will extract all the `q` property values as a new `Collection` from `QueryParam` collection, thus takes the same effect as above. +This will extract all the `value` property values as a new `Collection` from `QueryParameter` collection, thus takes the same effect as above. ==== +.alter parameter name by using `@Param` +==== +When accessing the parameter by SpEL, it's also useful to alter the parameter name to another one by `@Param` annotation in Sping Data: + +[source,java] +---- +interface BookRepository extends ElasticsearchRepository { + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "name": #{#another.![value]} + } + } + ] + } + } + """) + Page findByName(@Param("another") Collection parameters, Pageable pageable); +} +---- + +==== diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index 0fb5e97d5..c83b48eab 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -18,8 +18,9 @@ import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.query.BaseQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; -import org.springframework.data.elasticsearch.repository.support.ElasticsearchCollectionValueToStringConverter; -import org.springframework.data.elasticsearch.repository.support.ElasticsearchValueSpELConversionService; +import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchCollectionValueToStringConverter; +import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchStringValueToStringConverter; +import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchValueSpELConversionService; import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.expression.EvaluationContext; @@ -110,7 +111,7 @@ private String parseSpEL(String queryString, ElasticsearchParametersParameterAcc /** * {@link Expression#getValue(EvaluationContext, Class)} is not used because the value part in SpEL should be converted - * by {@link org.springframework.data.elasticsearch.repository.support.ElasticsearchStringValueToStringConverter} or + * by {@link ElasticsearchStringValueToStringConverter} or * {@link ElasticsearchCollectionValueToStringConverter} to * escape the quotations, but other literal parts in SpEL expression should not be processed with these converters. * So we just get the string value from {@link LiteralExpression} directly rather than diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java similarity index 90% rename from src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java rename to src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java index 0ee584eb9..2eb6db39d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchCollectionValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.elasticsearch.repository.support; +package org.springframework.data.elasticsearch.repository.support.spel; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; @@ -31,9 +31,23 @@ * The string should be wrapped with square brackets, with each element quoted therefore * escaped(by {@link ElasticsearchStringValueToStringConverter}) if quotations exist in the original element. *

- * eg: The value part of an elasticsearch terms query should looks like {@code ["hello \"Stranger\"","Another string"]}, - * for query {@code { 'bool' : { 'must' : { 'terms' : { 'name' : ["hello \"Stranger\"","Another string"] } } } }} + * eg: The value part of an elasticsearch terms query should looks like {@code ["hello \"Stranger\"","Another string"]} + * for query + * 

+ * {@code
+ *  {
+ *    "bool":{
+ *      "must":{
+ *        "terms":{
+ *          "name": ["hello \"Stranger\"", "Another string"]
+ *        }
+ *      }
+ *    }
+ *  }
+ * }
+ *
* + * @since 5.3 * @author Haibo Liu */ public class ElasticsearchCollectionValueToStringConverter implements GenericConverter { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java similarity index 92% rename from src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java rename to src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java index 54974a0ea..5c16c7226 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchStringValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.elasticsearch.repository.support; +package org.springframework.data.elasticsearch.repository.support.spel; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.GenericConverter; @@ -25,8 +25,9 @@ /** * Values in elasticsearch query may contain quotations and should be escaped when converting. - * However, the converter should only be used in this situation, rather than common string to string conversions. + * Note that the converter should only be used in this situation, rather than common string to string conversions. * + * @since 5.3 * @author Haibo Liu */ public class ElasticsearchStringValueToStringConverter implements GenericConverter { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchValueSpELConversionService.java similarity index 85% rename from src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java rename to src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchValueSpELConversionService.java index 904486d52..e75709b2b 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/ElasticsearchValueSpELConversionService.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchValueSpELConversionService.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.data.elasticsearch.repository.support; +package org.springframework.data.elasticsearch.repository.support.spel; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.ConverterRegistry; @@ -21,16 +21,17 @@ import org.springframework.data.util.Lazy; /** - * To supply a {@link ConversionService} with custom converter to handle SpEL value in elasticsearch query. + * To supply a {@link ConversionService} with custom converters to handle SpEL values in elasticsearch query. * + * @since 5.3 * @author Haibo Liu */ public class ElasticsearchValueSpELConversionService { public static final Lazy CONVERSION_SERVICE_LAZY = Lazy.of( - ElasticsearchValueSpELConversionService::buildElasticsearchConversionService); + ElasticsearchValueSpELConversionService::buildSpELConversionService); - private static ConversionService buildElasticsearchConversionService() { + private static ConversionService buildSpELConversionService() { // register elasticsearch custom type converter for conversion service ConversionService conversionService = new DefaultConversionService(); ConverterRegistry converterRegistry = (ConverterRegistry) conversionService; diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java index 21780e3cd..b00a8f043 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryELCIntegrationTests.java @@ -46,8 +46,8 @@ IndexNameProvider indexNameProvider() { * a normal bean referenced by SpEL in query */ @Bean - QueryParam queryParam() { - return new QueryParam("abc"); + QueryParameter queryParameter() { + return new QueryParameter("abc"); } } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index d1b7325ad..8f7a56663 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -1539,7 +1539,7 @@ void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { List entities = createSampleEntities("abc", 20); repository.saveAll(entities); - QueryParam param = new QueryParam("abc"); + QueryParameter param = new QueryParameter("abc"); // when SearchHits searchHits = repository.queryByParameterPropertySpEL(param); repository.queryByBeanPropertySpEL(); @@ -1585,7 +1585,7 @@ void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { List entities = createSampleEntities("abc", 20); repository.saveAll(entities); - QueryParam param = new QueryParam("abc"); + QueryParameter param = new QueryParameter("abc"); // when SearchHits searchHits = repository.queryByParameterPropertyCollectionSpEL(List.of(param)); @@ -1597,7 +1597,7 @@ void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpELWithParamAnnot List entities = createSampleEntities("abc", 20); repository.saveAll(entities); - QueryParam param = new QueryParam("abc"); + QueryParameter param = new QueryParameter("abc"); // when SearchHits searchHits = repository.queryByParameterPropertyCollectionSpELWithParamAnnotation( List.of(param)); @@ -2029,7 +2029,7 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByParameterPropertySpEL(QueryParam entity); + SearchHits queryByParameterPropertySpEL(QueryParameter parameter); @Query(""" { @@ -2059,7 +2059,7 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByParameterPropertyCollectionSpEL(Collection entities); + SearchHits queryByParameterPropertyCollectionSpEL(Collection parameters); @Query(""" { @@ -2104,7 +2104,7 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByParameterPropertyCollectionSpELWithParamAnnotation( - @Param("e") Collection entities); + @Param("e") Collection parameters); @Query(""" { diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParameter.java similarity index 91% rename from src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java rename to src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParameter.java index 7a99f45f2..86b842b58 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParam.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/QueryParameter.java @@ -18,8 +18,8 @@ /** * Used as a parameter referenced by SpEL in query method tests. * - * @param q content + * @param value content * @author Haibo Liu */ -public record QueryParam(String q) { +public record QueryParameter(String value) { } 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 85eb592c6..5c921724d 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 @@ -32,6 +32,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.skyscreamer.jsonassert.JSONAssert; +import org.skyscreamer.jsonassert.JSONCompareMode; import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; @@ -42,7 +44,7 @@ import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.query.StringQuery; -import org.springframework.data.elasticsearch.repositories.custommethod.QueryParam; +import org.springframework.data.elasticsearch.repositories.custommethod.QueryParameter; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; @@ -90,43 +92,84 @@ public void shouldReplaceRepeatedParametersCorrectly() throws Exception { public void shouldReplaceParametersSpEL() throws Exception { org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", "Luke"); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "Luke" + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()) - .isEqualTo("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"Luke\" } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test public void shouldReplaceParametersSpELWithQuotes() throws Exception { - org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", "hello \"world\""); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", + "hello \"world\""); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "hello \\"world\\"" + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()) - .isEqualTo("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"hello \\\"world\\\"\" } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test public void shouldUseParameterPropertySpEL() throws Exception { org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByParameterPropertySpEL", - new QueryParam("Luke")); + new QueryParameter("Luke")); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "Luke" + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()) - .isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test public void shouldReplaceCollectionSpEL() throws Exception { - final List anotherString = Arrays.asList("hello \"Stranger\"", "Another string"); + final List anotherString = List.of("hello \"Stranger\"", "Another string"); List params = new ArrayList<>(anotherString); org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": ["hello \\"Stranger\\"", "Another string"] + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()).isEqualTo( - "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test @@ -135,22 +178,43 @@ public void shouldReplaceEmptyCollectionSpEL() throws Exception { final List anotherString = List.of(); List params = new ArrayList<>(anotherString); org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": [] + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()).isEqualTo( - "{ 'bool' : { 'must' : { 'terms' : { 'name' : [] } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test public void shouldReplaceCollectionParametersSpEL() throws Exception { - final List anotherString = List.of(new QueryParam("hello \"Stranger\""), new QueryParam("Another string")); - List params = new ArrayList<>(anotherString); + final List anotherString = List.of(new QueryParameter("hello \"Stranger\""), + new QueryParameter("Another string")); + List params = new ArrayList<>(anotherString); org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesParameterSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": ["hello \\"Stranger\\"", "Another string"] + } + } + } + } + """; assertThat(query).isInstanceOf(StringQuery.class); - assertThat(((StringQuery) query).getSource()).isEqualTo( - "{ 'bool' : { 'must' : { 'terms' : { 'name' : [\"hello \\\"Stranger\\\"\",\"Another string\"] } } } }"); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } @Test // #1790 @@ -258,20 +322,60 @@ private interface SampleRepository extends Repository { @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }") Person findByName(String name); - @Query("{ \"bool\" : { \"must\" : { \"term\" : { \"name\" : \"#{#name}\" } } } }") + @Query(""" + { + "bool":{ + "must":{ + "term":{ + "name": "#{#name}" + } + } + } + } + """) Person findByNameSpEL(String name); - @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{#param.q}' } } } }") - Person findByParameterPropertySpEL(QueryParam param); + @Query(""" + { + "bool":{ + "must":{ + "term":{ + "name": "#{#param.value}" + } + } + } + } + """) + Person findByParameterPropertySpEL(QueryParameter param); @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : ?0 } } } }") Person findByNameIn(ArrayList names); - @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : #{#names} } } } }") + @Query(""" + { + "bool":{ + "must":{ + "term":{ + "name": #{#names} + } + } + } + } + """) Person findByNamesSpEL(ArrayList names); - @Query("{ 'bool' : { 'must' : { 'terms' : { 'name' : #{#names.![q]} } } } }") - Person findByNamesParameterSpEL(ArrayList names); + @Query(""" + { + "bool":{ + "must":{ + "terms":{ + "name": #{#names.![value]} + } + } + } + } + """) + Person findByNamesParameterSpEL(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, From 8eac2ea8c237b18d38ef22c7a37a88176851f28e Mon Sep 17 00:00:00 2001 From: puppylpg Date: Wed, 17 Jan 2024 16:46:54 +0800 Subject: [PATCH 08/11] reject null value for SpEL --- .../query/ElasticsearchStringQuery.java | 11 ++- ...earchCollectionValueToStringConverter.java | 12 +-- ...ticsearchStringValueToStringConverter.java | 2 +- ...ustomMethodRepositoryIntegrationTests.java | 50 ++++++++++- .../ElasticsearchStringQueryUnitTests.java | 89 ++++++++++++++++++- 5 files changed, 152 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index c83b48eab..f8f982967 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -16,6 +16,7 @@ package org.springframework.data.elasticsearch.repository.query; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; +import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.core.query.BaseQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchCollectionValueToStringConverter; @@ -124,9 +125,15 @@ private String parseExpressions(Expression rootExpr, EvaluationContext context) parsed.append(literalExpression.getExpressionString()); } else if (rootExpr instanceof SpelExpression spelExpression) { // evaluate the value - parsed.append(spelExpression.getValue(context, String.class)); + String value = spelExpression.getValue(context, String.class); + if (value == null) { + throw new ConversionException(String.format( + "Parameter value can't be null for SpEL expression '%s' in method '%s' when querying elasticsearch", + spelExpression.getExpressionString(), queryMethod.method.getName())); + } + parsed.append(value); } else if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { - // then it should be another composite expression + // parse one by one for composite expression Expression[] expressions = compositeStringExpression.getExpressions(); for (Expression exp : expressions) { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java index 2eb6db39d..1a0adb1e3 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchCollectionValueToStringConverter.java @@ -20,15 +20,12 @@ import org.springframework.core.convert.converter.GenericConverter; import org.springframework.lang.Nullable; -import java.util.Collection; -import java.util.Collections; -import java.util.Set; -import java.util.StringJoiner; +import java.util.*; /** * Convert a collection into string for value part of the elasticsearch query. *

- * The string should be wrapped with square brackets, with each element quoted therefore + * If the value is type {@link String}, it should be wrapped with square brackets, with each element quoted therefore * escaped(by {@link ElasticsearchStringValueToStringConverter}) if quotations exist in the original element. *

* eg: The value part of an elasticsearch terms query should looks like {@code ["hello \"Stranger\"","Another string"]} @@ -77,6 +74,11 @@ public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDe } StringJoiner sb = new StringJoiner(DELIMITER, "[", "]"); for (Object sourceElement : sourceCollection) { + // ignore the null value in collection + if (Objects.isNull(sourceElement)) { + continue; + } + Object targetElement = this.conversionService.convert( sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); if (sourceElement instanceof String) { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java index 5c16c7226..07ad446b1 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/ElasticsearchStringValueToStringConverter.java @@ -41,7 +41,7 @@ public Set getConvertibleTypes() { @Nullable public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { if (source == null) { - return "null"; + return null; } return escape(source); } diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index 8f7a56663..8595b88af 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.data.elasticsearch.repositories.custommethod; import static org.assertj.core.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.springframework.data.elasticsearch.annotations.FieldType.*; import static org.springframework.data.elasticsearch.utils.IdGenerator.*; @@ -52,6 +53,7 @@ import org.springframework.data.elasticsearch.core.SearchHit; import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.SearchPage; +import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.core.geo.GeoBox; import org.springframework.data.elasticsearch.core.geo.GeoPoint; import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates; @@ -1534,6 +1536,18 @@ void shouldReturnSearchHitsForStringQuerySpEL() { assertThat(searchHits.getTotalHits()).isEqualTo(20); } + @Test + void shouldRaiseExceptionForNullStringQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + ConversionException thrown = assertThrows(ConversionException.class, () -> repository.queryByStringSpEL(null)); + + assertThat(thrown.getMessage()) + .isEqualTo("Parameter value can't be null for SpEL expression '#type' in method 'queryByStringSpEL'" + + " when querying elasticsearch"); + } + @Test void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { List entities = createSampleEntities("abc", 20); @@ -1571,7 +1585,19 @@ void shouldReturnSearchHitsForCollectionQuerySpEL() { } @Test - void shouldReturnSearchHitsForEmptyCollectionQuerySpEL() { + void shouldRaiseExceptionForNullCollectionQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + ConversionException thrown = assertThrows(ConversionException.class, () -> repository.queryByCollectionSpEL(null)); + + assertThat(thrown.getMessage()) + .isEqualTo("Parameter value can't be null for SpEL expression '#types' in method 'queryByCollectionSpEL'" + + " when querying elasticsearch"); + } + + @Test + void shouldNotReturnSearchHitsForEmptyCollectionQuerySpEL() { List entities = createSampleEntities("abc", 20); repository.saveAll(entities); @@ -1580,6 +1606,28 @@ void shouldReturnSearchHitsForEmptyCollectionQuerySpEL() { assertThat(searchHits.getTotalHits()).isEqualTo(0); } + @Test + void shouldNotReturnSearchHitsForCollectionQueryWithOnlyNullValuesSpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + List params = new ArrayList<>(); + params.add(null); + // when + SearchHits searchHits = repository.queryByCollectionSpEL(params); + assertThat(searchHits.getTotalHits()).isEqualTo(0); + } + + @Test + void shouldIgnoreNullValuesInCollectionQuerySpEL() { + List entities = createSampleEntities("abc", 20); + repository.saveAll(entities); + + // when + SearchHits searchHits = repository.queryByCollectionSpEL(Arrays.asList("abc", null)); + assertThat(searchHits.getTotalHits()).isEqualTo(20); + } + @Test void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { List entities = createSampleEntities("abc", 20); 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 5c921724d..91fec839b 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 @@ -160,7 +160,7 @@ public void shouldReplaceCollectionSpEL() throws Exception { { "bool":{ "must":{ - "term":{ + "terms":{ "name": ["hello \\"Stranger\\"", "Another string"] } } @@ -172,6 +172,28 @@ public void shouldReplaceCollectionSpEL() throws Exception { JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } + @Test + public void shouldReplaceNonStringCollectionSpEL() throws Exception { + + final List ages = List.of(1, 2, 3); + List params = new ArrayList<>(ages); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByAgesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": [1, 2, 3] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + @Test public void shouldReplaceEmptyCollectionSpEL() throws Exception { @@ -182,7 +204,31 @@ public void shouldReplaceEmptyCollectionSpEL() throws Exception { { "bool":{ "must":{ - "term":{ + "terms":{ + "name": [] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldBeEmptyWithNullValuesInCollectionSpEL() throws Exception { + + final List anotherString = List.of(); + List params = new ArrayList<>(anotherString); + // add a null value + params.add(null); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ "name": [] } } @@ -194,6 +240,30 @@ public void shouldReplaceEmptyCollectionSpEL() throws Exception { JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); } + @Test + public void shouldIgnoreNullValuesInCollectionSpEL() throws Exception { + + final List anotherString = List.of("abc"); + List params = new ArrayList<>(anotherString); + // add a null value + params.add(null); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": ["abc"] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + @Test public void shouldReplaceCollectionParametersSpEL() throws Exception { @@ -355,7 +425,7 @@ private interface SampleRepository extends Repository { { "bool":{ "must":{ - "term":{ + "terms":{ "name": #{#names} } } @@ -364,6 +434,19 @@ private interface SampleRepository extends Repository { """) Person findByNamesSpEL(ArrayList names); + @Query(""" + { + "bool":{ + "must":{ + "terms":{ + "name": #{#ages} + } + } + } + } + """) + Person findByAgesSpEL(ArrayList ages); + @Query(""" { "bool":{ From 767a36df3e14c200787404187f7e31af35618d18 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Thu, 18 Jan 2024 02:18:36 +0800 Subject: [PATCH 09/11] Add support for SpEL in reactive query --- ...tReactiveElasticsearchRepositoryQuery.java | 3 +- ...sticsearchParametersParameterAccessor.java | 2 +- .../query/ElasticsearchStringQuery.java | 86 +----- .../ReactiveElasticsearchStringQuery.java | 16 +- .../ReactivePartTreeElasticsearchQuery.java | 2 +- .../spel/QueryStringSpELEvaluator.java | 127 ++++++++ ...ustomMethodRepositoryIntegrationTests.java | 5 +- .../ElasticsearchStringQueryUnitTests.java | 4 +- ...tiveElasticsearchStringQueryUnitTests.java | 283 +++++++++++++++++- .../query/StubParameterAccessor.java | 108 ------- ...icsearchRepositoryELCIntegrationTests.java | 10 + ...asticsearchRepositoryIntegrationTests.java | 227 ++++++++++++++ 12 files changed, 658 insertions(+), 215 deletions(-) create mode 100644 src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java delete mode 100644 src/test/java/org/springframework/data/elasticsearch/repository/query/StubParameterAccessor.java diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/AbstractReactiveElasticsearchRepositoryQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/AbstractReactiveElasticsearchRepositoryQuery.java index 516ea146b..02826cbe5 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/AbstractReactiveElasticsearchRepositoryQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/AbstractReactiveElasticsearchRepositoryQuery.java @@ -43,6 +43,7 @@ * * @author Christoph Strobl * @author Peter-Josef Meisch + * @author Haibo Liu * @since 3.2 */ abstract class AbstractReactiveElasticsearchRepositoryQuery implements RepositoryQuery { @@ -112,7 +113,7 @@ private Object execute(ElasticsearchParametersParameterAccessor parameterAccesso * @param accessor must not be {@literal null}. * @return */ - protected abstract BaseQuery createQuery(ElasticsearchParameterAccessor accessor); + protected abstract BaseQuery createQuery(ElasticsearchParametersParameterAccessor accessor); private ReactiveElasticsearchQueryExecution getExecution(ElasticsearchParameterAccessor accessor, Converter resultProcessing) { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchParametersParameterAccessor.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchParametersParameterAccessor.java index c4343475f..92f4992c4 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchParametersParameterAccessor.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchParametersParameterAccessor.java @@ -21,7 +21,7 @@ * @author Christoph Strobl * @since 3.2 */ -class ElasticsearchParametersParameterAccessor extends ParametersParameterAccessor +public class ElasticsearchParametersParameterAccessor extends ParametersParameterAccessor implements ElasticsearchParameterAccessor { private final Object[] values; diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index f8f982967..b50aa012e 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -16,30 +16,13 @@ package org.springframework.data.elasticsearch.repository.query; import org.springframework.data.elasticsearch.core.ElasticsearchOperations; -import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.core.query.BaseQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; -import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchCollectionValueToStringConverter; -import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchStringValueToStringConverter; -import org.springframework.data.elasticsearch.repository.support.spel.ElasticsearchValueSpELConversionService; import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; +import org.springframework.data.elasticsearch.repository.support.spel.QueryStringSpELEvaluator; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.Expression; -import org.springframework.expression.ParserContext; -import org.springframework.expression.TypeConverter; -import org.springframework.expression.common.CompositeStringExpression; -import org.springframework.expression.common.LiteralExpression; -import org.springframework.expression.spel.standard.SpelExpression; -import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.expression.spel.support.StandardEvaluationContext; -import org.springframework.expression.spel.support.StandardTypeConverter; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - /** * ElasticsearchStringQuery * @@ -52,12 +35,8 @@ */ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQuery { - private static final SpelExpressionParser PARSER = new SpelExpressionParser(); - private static final Map QUERY_EXPRESSIONS = new ConcurrentHashMap<>(); - private final String queryString; private final QueryMethodEvaluationContextProvider evaluationContextProvider; - private final TypeConverter elasticsearchSpELTypeConverter; public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations, String queryString, QueryMethodEvaluationContextProvider evaluationContextProvider) { @@ -67,7 +46,6 @@ public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, Elasticsea this.queryString = queryString; this.evaluationContextProvider = evaluationContextProvider; - this.elasticsearchSpELTypeConverter = new StandardTypeConverter(ElasticsearchValueSpELConversionService.CONVERSION_SERVICE_LAZY); } @Override @@ -89,68 +67,10 @@ protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor paramet String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService()) .replacePlaceholders(this.queryString, parameterAccessor); - var query = new StringQuery(parseSpEL(queryString, parameterAccessor)); + QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, evaluationContextProvider); + var query = new StringQuery(evaluator.evaluate(queryMethod)); query.addSort(parameterAccessor.getSort()); return query; } - private String parseSpEL(String queryString, ElasticsearchParametersParameterAccessor parameterAccessor) { - Expression expr = getQueryExpression(queryString); - if (expr != null) { - EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), - parameterAccessor.getValues()); - if (context instanceof StandardEvaluationContext standardEvaluationContext) { - standardEvaluationContext.setTypeConverter(elasticsearchSpELTypeConverter); - } - - String parsed = parseExpressions(expr, context); - Assert.notNull(parsed, "Query parsed by SpEL should not be null"); - return parsed; - } - return queryString; - } - - /** - * {@link Expression#getValue(EvaluationContext, Class)} is not used because the value part in SpEL should be converted - * by {@link ElasticsearchStringValueToStringConverter} or - * {@link ElasticsearchCollectionValueToStringConverter} to - * escape the quotations, but other literal parts in SpEL expression should not be processed with these converters. - * So we just get the string value from {@link LiteralExpression} directly rather than - * {@link LiteralExpression#getValue(EvaluationContext, Class)}. - */ - private String parseExpressions(Expression rootExpr, EvaluationContext context) { - StringBuilder parsed = new StringBuilder(); - if (rootExpr instanceof LiteralExpression literalExpression) { - // get the string literal directly - parsed.append(literalExpression.getExpressionString()); - } else if (rootExpr instanceof SpelExpression spelExpression) { - // evaluate the value - String value = spelExpression.getValue(context, String.class); - if (value == null) { - throw new ConversionException(String.format( - "Parameter value can't be null for SpEL expression '%s' in method '%s' when querying elasticsearch", - spelExpression.getExpressionString(), queryMethod.method.getName())); - } - parsed.append(value); - } else if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { - // parse one by one for composite expression - Expression[] expressions = compositeStringExpression.getExpressions(); - - for (Expression exp : expressions) { - parsed.append(parseExpressions(exp, context)); - } - } else { - // no more - parsed.append(rootExpr.getValue(context, String.class)); - } - return parsed.toString(); - } - - @Nullable - private Expression getQueryExpression(String queryString) { - return QUERY_EXPRESSIONS.computeIfAbsent(queryString, f -> { - Expression expr = PARSER.parseExpression(queryString, ParserContext.TEMPLATE_EXPRESSION); - return expr instanceof LiteralExpression ? null : expr; - }); - } } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java index 6ac03a8ce..c1b49425f 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java @@ -16,19 +16,23 @@ package org.springframework.data.elasticsearch.repository.query; import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations; +import org.springframework.data.elasticsearch.core.query.BaseQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.repository.support.StringQueryUtil; +import org.springframework.data.elasticsearch.repository.support.spel.QueryStringSpELEvaluator; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.expression.spel.standard.SpelExpressionParser; /** * @author Christoph Strobl * @author Taylor Ono + * @author Haibo Liu * @since 3.2 */ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsearchRepositoryQuery { private final String query; + private final QueryMethodEvaluationContextProvider evaluationContextProvider; public ReactiveElasticsearchStringQuery(ReactiveElasticsearchQueryMethod queryMethod, ReactiveElasticsearchOperations operations, SpelExpressionParser expressionParser, @@ -43,14 +47,16 @@ public ReactiveElasticsearchStringQuery(String query, ReactiveElasticsearchQuery super(queryMethod, operations); this.query = query; + this.evaluationContextProvider = evaluationContextProvider; } @Override - protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) { - String queryString = new StringQueryUtil( - getElasticsearchOperations().getElasticsearchConverter().getConversionService()).replacePlaceholders(this.query, - parameterAccessor); - return new StringQuery(queryString); + protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor parameterAccessor) { + String queryString = new StringQueryUtil(getElasticsearchOperations().getElasticsearchConverter().getConversionService()) + .replacePlaceholders(this.query, parameterAccessor); + + QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, evaluationContextProvider); + return new StringQuery(evaluator.evaluate(queryMethod)); } @Override diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactivePartTreeElasticsearchQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactivePartTreeElasticsearchQuery.java index bcd33e888..1fd7f1308 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactivePartTreeElasticsearchQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactivePartTreeElasticsearchQuery.java @@ -40,7 +40,7 @@ public ReactivePartTreeElasticsearchQuery(ReactiveElasticsearchQueryMethod query } @Override - protected BaseQuery createQuery(ElasticsearchParameterAccessor accessor) { + protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor accessor) { CriteriaQuery query = new ElasticsearchQueryCreator(tree, accessor, getMappingContext()).createQuery(); if (tree.isLimiting()) { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java new file mode 100644 index 000000000..263db8a60 --- /dev/null +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java @@ -0,0 +1,127 @@ +/* + * Copyright 2024 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.support.spel; + +import org.springframework.data.elasticsearch.core.convert.ConversionException; +import org.springframework.data.elasticsearch.repository.query.ElasticsearchParametersParameterAccessor; +import org.springframework.data.repository.query.QueryMethod; +import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.expression.ParserContext; +import org.springframework.expression.TypeConverter; +import org.springframework.expression.common.CompositeStringExpression; +import org.springframework.expression.common.LiteralExpression; +import org.springframework.expression.spel.standard.SpelExpression; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.StandardTypeConverter; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * To evaluate the SpEL expressions of the query string. + * + * @author Haibo Liu + * @since 5.3 + */ +public class QueryStringSpELEvaluator { + + private static final SpelExpressionParser PARSER = new SpelExpressionParser(); + private static final Map QUERY_EXPRESSIONS = new ConcurrentHashMap<>(); + + private final String queryString; + private final ElasticsearchParametersParameterAccessor parameterAccessor; + private final QueryMethodEvaluationContextProvider evaluationContextProvider; + private final TypeConverter elasticsearchSpELTypeConverter; + + public QueryStringSpELEvaluator(String queryString, ElasticsearchParametersParameterAccessor parameterAccessor, + QueryMethodEvaluationContextProvider evaluationContextProvider) { + this.queryString = queryString; + this.parameterAccessor = parameterAccessor; + this.evaluationContextProvider = evaluationContextProvider; + this.elasticsearchSpELTypeConverter = new StandardTypeConverter(ElasticsearchValueSpELConversionService.CONVERSION_SERVICE_LAZY); + } + + /** + * Evaluate the SpEL parts of the query string. + * + * @param queryMethod the query method + * @return a plain string with values evaluated + */ + public String evaluate(QueryMethod queryMethod) { + Expression expr = getQueryExpression(queryString); + if (expr != null) { + EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), + parameterAccessor.getValues()); + if (context instanceof StandardEvaluationContext standardEvaluationContext) { + standardEvaluationContext.setTypeConverter(elasticsearchSpELTypeConverter); + } + + String parsed = parseExpressions(expr, context, queryMethod); + Assert.notNull(parsed, "Query parsed by SpEL should not be null"); + return parsed; + } + return queryString; + } + + /** + * {@link Expression#getValue(EvaluationContext, Class)} is not used because the value part in SpEL should be converted + * by {@link ElasticsearchStringValueToStringConverter} or + * {@link ElasticsearchCollectionValueToStringConverter} to + * escape the quotations, but other literal parts in SpEL expression should not be processed with these converters. + * So we just get the string value from {@link LiteralExpression} directly rather than + * {@link LiteralExpression#getValue(EvaluationContext, Class)}. + */ + private String parseExpressions(Expression rootExpr, EvaluationContext context, QueryMethod queryMethod) { + StringBuilder parsed = new StringBuilder(); + if (rootExpr instanceof LiteralExpression literalExpression) { + // get the string literal directly + parsed.append(literalExpression.getExpressionString()); + } else if (rootExpr instanceof SpelExpression spelExpression) { + // evaluate the value + String value = spelExpression.getValue(context, String.class); + if (value == null) { + throw new ConversionException(String.format( + "Parameter value can't be null for SpEL expression '%s' in method '%s' when querying elasticsearch", + spelExpression.getExpressionString(), queryMethod.getName())); + } + parsed.append(value); + } else if (rootExpr instanceof CompositeStringExpression compositeStringExpression) { + // parse one by one for composite expression + Expression[] expressions = compositeStringExpression.getExpressions(); + + for (Expression exp : expressions) { + parsed.append(parseExpressions(exp, context, queryMethod)); + } + } else { + // no more + parsed.append(rootExpr.getValue(context, String.class)); + } + return parsed.toString(); + } + + @Nullable + private Expression getQueryExpression(String queryString) { + return QUERY_EXPRESSIONS.computeIfAbsent(queryString, f -> { + Expression expr = PARSER.parseExpression(queryString, ParserContext.TEMPLATE_EXPRESSION); + return expr instanceof LiteralExpression ? null : expr; + }); + } +} diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index 8595b88af..6e5aef693 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -1556,7 +1556,6 @@ void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { QueryParameter param = new QueryParameter("abc"); // when SearchHits searchHits = repository.queryByParameterPropertySpEL(param); - repository.queryByBeanPropertySpEL(); assertThat(searchHits.getTotalHits()).isEqualTo(20); } @@ -1568,7 +1567,6 @@ void shouldReturnSearchHitsForBeanQuerySpEL() { // when SearchHits searchHits = repository.queryByBeanPropertySpEL(); - repository.queryByBeanPropertySpEL(); assertThat(searchHits.getTotalHits()).isEqualTo(20); } @@ -1603,6 +1601,7 @@ void shouldNotReturnSearchHitsForEmptyCollectionQuerySpEL() { // when SearchHits searchHits = repository.queryByCollectionSpEL(List.of()); + assertThat(searchHits.getTotalHits()).isEqualTo(0); } @@ -1615,6 +1614,7 @@ void shouldNotReturnSearchHitsForCollectionQueryWithOnlyNullValuesSpEL() { params.add(null); // when SearchHits searchHits = repository.queryByCollectionSpEL(params); + assertThat(searchHits.getTotalHits()).isEqualTo(0); } @@ -1625,6 +1625,7 @@ void shouldIgnoreNullValuesInCollectionQuerySpEL() { // when SearchHits searchHits = repository.queryByCollectionSpEL(Arrays.asList("abc", null)); + assertThat(searchHits.getTotalHits()).isEqualTo(20); } 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 91fec839b..60821db50 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 @@ -183,7 +183,7 @@ public void shouldReplaceNonStringCollectionSpEL() throws Exception { "bool":{ "must":{ "terms":{ - "name": [1, 2, 3] + "age": [1, 2, 3] } } } @@ -439,7 +439,7 @@ private interface SampleRepository extends Repository { "bool":{ "must":{ "terms":{ - "name": #{#ages} + "age": #{#ages} } } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java index f400f3904..e649fe792 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQueryUnitTests.java @@ -22,6 +22,7 @@ import reactor.core.publisher.Mono; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -29,12 +30,13 @@ import java.util.Map; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.skyscreamer.jsonassert.JSONAssert; +import org.skyscreamer.jsonassert.JSONCompareMode; import org.springframework.data.annotation.Id; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; @@ -45,6 +47,7 @@ import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations; import org.springframework.data.elasticsearch.core.SearchHit; import org.springframework.data.elasticsearch.core.query.StringQuery; +import org.springframework.data.elasticsearch.repositories.custommethod.QueryParameter; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; @@ -55,6 +58,7 @@ /** * @author Christoph Strobl * @author Peter-Josef Meisch + * @author Haibo Liu */ @ExtendWith(MockitoExtension.class) public class ReactiveElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase { @@ -71,10 +75,7 @@ public void setUp() { @Test // DATAES-519 public void bindsSimplePropertyCorrectly() throws Exception { - ReactiveElasticsearchStringQuery elasticsearchStringQuery = createQueryForMethod("findByName", String.class); - StubParameterAccessor accessor = new StubParameterAccessor("Luke"); - - org.springframework.data.elasticsearch.core.query.Query query = elasticsearchStringQuery.createQuery(accessor); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByName", "Luke"); StringQuery reference = new StringQuery("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); assertThat(query).isInstanceOf(StringQuery.class); @@ -82,20 +83,214 @@ public void bindsSimplePropertyCorrectly() throws Exception { } @Test // DATAES-519 - @Disabled("TODO: fix spel query integration") public void bindsExpressionPropertyCorrectly() throws Exception { - ReactiveElasticsearchStringQuery elasticsearchStringQuery = createQueryForMethod("findByNameWithExpression", - String.class); - StubParameterAccessor accessor = new StubParameterAccessor("Luke"); - - org.springframework.data.elasticsearch.core.query.Query query = elasticsearchStringQuery.createQuery(accessor); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameWithExpression", "Luke"); StringQuery reference = new StringQuery("{ 'bool' : { 'must' : { 'term' : { 'name' : 'Luke' } } } }"); assertThat(query).isInstanceOf(StringQuery.class); assertThat(((StringQuery) query).getSource()).isEqualTo(reference.getSource()); } + @Test + public void shouldReplaceParametersSpEL() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", "Luke"); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "Luke" + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldReplaceParametersSpELWithQuotes() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNameSpEL", + "hello \"world\""); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "hello \\"world\\"" + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldUseParameterPropertySpEL() throws Exception { + + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByParameterPropertySpEL", + new QueryParameter("Luke")); + String expected = """ + { + "bool":{ + "must":{ + "term":{ + "name": "Luke" + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldReplaceCollectionSpEL() throws Exception { + + final List anotherString = List.of("hello \"Stranger\"", "Another string"); + List params = new ArrayList<>(anotherString); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": ["hello \\"Stranger\\"", "Another string"] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldReplaceNonStringCollectionSpEL() throws Exception { + + final List ages = List.of(1, 2, 3); + List params = new ArrayList<>(ages); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByAgesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "age": [1, 2, 3] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldReplaceEmptyCollectionSpEL() throws Exception { + + final List anotherString = List.of(); + List params = new ArrayList<>(anotherString); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": [] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldBeEmptyWithNullValuesInCollectionSpEL() throws Exception { + + final List anotherString = List.of(); + List params = new ArrayList<>(anotherString); + // add a null value + params.add(null); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": [] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldIgnoreNullValuesInCollectionSpEL() throws Exception { + + final List anotherString = List.of("abc"); + List params = new ArrayList<>(anotherString); + // add a null value + params.add(null); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": ["abc"] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + + @Test + public void shouldReplaceCollectionParametersSpEL() throws Exception { + + final List anotherString = List.of(new QueryParameter("hello \"Stranger\""), + new QueryParameter("Another string")); + List params = new ArrayList<>(anotherString); + org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByNamesParameterSpEL", params); + String expected = """ + { + "bool":{ + "must":{ + "terms":{ + "name": ["hello \\"Stranger\\"", "Another string"] + } + } + } + } + """; + + assertThat(query).isInstanceOf(StringQuery.class); + JSONAssert.assertEquals(((StringQuery) query).getSource(), expected, JSONCompareMode.NON_EXTENSIBLE); + } + @Test // DATAES-552 public void shouldReplaceLotsOfParametersCorrectly() throws Exception { @@ -205,7 +400,59 @@ private interface SampleRepository extends Repository { @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?0' } } } }") Mono findByName(String name); - @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '?#{[0]}' } } } }") + @Query(""" + { + "bool":{ + "must":{ + "term":{ + "name": "#{#name}" + } + } + } + } + """) + Mono findByNameSpEL(String name); + + @Query(""" + { + "bool":{ + "must":{ + "terms":{ + "name": #{#names} + } + } + } + } + """) + Flux findByNamesSpEL(List names); + + @Query(""" + { + "bool":{ + "must":{ + "term":{ + "name": "#{#param.value}" + } + } + } + } + """) + Flux findByParameterPropertySpEL(QueryParameter param); + + @Query(""" + { + "bool":{ + "must":{ + "terms":{ + "name": #{#names.![value]} + } + } + } + } + """) + Flux findByNamesParameterSpEL(List names); + + @Query("{ 'bool' : { 'must' : { 'term' : { 'name' : '#{[0]}' } } } }") Flux findByNameWithExpression(String param0); @Query(value = "name:(?0, ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11)") @@ -228,6 +475,18 @@ Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String @Query("{ 'bool' : { 'must' : { 'terms' : { 'ages' : ?0 } } } }") Flux findByAges(List ages); + @Query(""" + { + "bool":{ + "must":{ + "terms":{ + "age": #{#ages} + } + } + } + } + """) + Flux findByAgesSpEL(List ages); } /** diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/query/StubParameterAccessor.java b/src/test/java/org/springframework/data/elasticsearch/repository/query/StubParameterAccessor.java deleted file mode 100644 index 12177a283..000000000 --- a/src/test/java/org/springframework/data/elasticsearch/repository/query/StubParameterAccessor.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2019-2024 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 java.util.Arrays; -import java.util.Iterator; -import java.util.Optional; - -import org.springframework.data.domain.Pageable; -import org.springframework.data.domain.ScrollPosition; -import org.springframework.data.domain.Sort; -import org.springframework.data.repository.query.ParameterAccessor; - -/** - * Simple {@link ParameterAccessor} that returns the given parameters unfiltered. - * - * @author Christoph Strobl - * @author Peter-Josef Meisch - */ -class StubParameterAccessor implements ElasticsearchParameterAccessor { - - private final Object[] values; - - StubParameterAccessor(Object... values) { - this.values = values; - } - - @Override - public ScrollPosition getScrollPosition() { - return null; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#getPageable() - */ - @Override - public Pageable getPageable() { - return Pageable.unpaged(); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#getBindableValue(int) - */ - @Override - public Object getBindableValue(int index) { - return values[index]; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#hasBindableNullValue() - */ - @Override - public boolean hasBindableNullValue() { - return false; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#getSort() - */ - @Override - public Sort getSort() { - return Sort.unsorted(); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#iterator() - */ - @Override - public Iterator iterator() { - return Arrays.asList(values).iterator(); - } - - /* - * (non-Javadoc) - * @see org.springframework.data.elasticsearch.repository.query.ElasticsearchParameterAccessor#getValues() - */ - @Override - public Object[] getValues() { - return this.values; - } - - /* - * (non-Javadoc) - * @see org.springframework.data.repository.query.ParameterAccessor#findDynamicProjection() - */ - @Override - public Class findDynamicProjection() { - return null; - } -} diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryELCIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryELCIntegrationTests.java index eaef4cba4..b5aec9c92 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryELCIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryELCIntegrationTests.java @@ -19,12 +19,14 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.data.elasticsearch.junit.jupiter.ReactiveElasticsearchTemplateConfiguration; +import org.springframework.data.elasticsearch.repositories.custommethod.QueryParameter; import org.springframework.data.elasticsearch.repository.config.EnableReactiveElasticsearchRepositories; import org.springframework.data.elasticsearch.utils.IndexNameProvider; import org.springframework.test.context.ContextConfiguration; /** * @author Peter-Josef Meisch + * @author Haibo Liu * @since 4.4 */ @ContextConfiguration(classes = { SimpleReactiveElasticsearchRepositoryELCIntegrationTests.Config.class }) @@ -39,6 +41,14 @@ static class Config { IndexNameProvider indexNameProvider() { return new IndexNameProvider("simple-reactive-repository"); } + + /** + * a normal bean referenced by SpEL in query + */ + @Bean + QueryParameter queryParameter() { + return new QueryParameter("message"); + } } } diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java index fb5a1ee93..33154ba9f 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java @@ -20,6 +20,8 @@ import static org.springframework.data.elasticsearch.core.query.Query.*; import static org.springframework.data.elasticsearch.utils.IdGenerator.*; +import org.springframework.data.elasticsearch.repositories.custommethod.QueryParameter; +import org.springframework.data.repository.query.Param; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -64,6 +66,7 @@ * @author Christoph Strobl * @author Peter-Josef Meisch * @author Jens Schauder + * @author Haibo Liu */ @SpringIntegrationTest abstract class SimpleReactiveElasticsearchRepositoryIntegrationTests { @@ -234,6 +237,139 @@ void shouldReturnFluxOfSearchHitForStringQuery() { .verifyComplete(); } + @Test + void shouldReturnSearchHitsForStringQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + repository.queryByStringSpEL("message") + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + QueryParameter param = new QueryParameter("message"); + + repository.queryByParameterPropertySpEL(param) + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldReturnSearchHitsForBeanQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + repository.queryByBeanPropertySpEL() + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldReturnSearchHitsForCollectionQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + repository.queryByCollectionSpEL(List.of("message")) + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldNotReturnSearchHitsForEmptyCollectionQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + repository.queryByCollectionSpEL(List.of()) + .as(StepVerifier::create) // + .expectNextCount(0) // + .verifyComplete(); + } + + @Test + void shouldNotReturnSearchHitsForCollectionQueryWithOnlyNullValuesSpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + List params = new ArrayList<>(); + params.add(null); + + repository.queryByCollectionSpEL(params) + .as(StepVerifier::create) // + .expectNextCount(0) // + .verifyComplete(); + } + + @Test + void shouldIgnoreNullValuesInCollectionQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + repository.queryByCollectionSpEL(Arrays.asList("message", null)) + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + QueryParameter param = new QueryParameter("message"); + + repository.queryByParameterPropertyCollectionSpEL(List.of(param)) + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + + @Test + void shouldReturnSearchHitsForParameterPropertyCollectionQuerySpELWithParamAnnotation() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + QueryParameter param = new QueryParameter("message"); + + repository.queryByParameterPropertyCollectionSpELWithParamAnnotation(List.of(param)) + .as(StepVerifier::create) // + .expectNextMatches(searchHit -> SearchHit.class.isAssignableFrom(searchHit.getClass()))// + .expectNextCount(2) // + .verifyComplete(); + } + @Test // DATAES-372 void shouldReturnHighlightsOnAnnotatedMethod() { @@ -787,6 +923,97 @@ interface ReactiveSampleEntityRepository extends ReactiveCrudRepository findAllViaAnnotatedQueryByMessageLikePaged(String message, Pageable pageable); + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "message": "#{#message}" + } + } + ] + } + } + """) + Flux> queryByStringSpEL(String message); + + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "message": "#{#parameter.value}" + } + } + ] + } + } + """) + Flux> queryByParameterPropertySpEL(QueryParameter parameter); + + @Query(""" + { + "bool":{ + "must":[ + { + "term":{ + "message": "#{@queryParameter.value}" + } + } + ] + } + } + """) + Flux> queryByBeanPropertySpEL(); + + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "message": #{#messages} + } + } + ] + } + } + """) + Flux> queryByCollectionSpEL(Collection messages); + + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "message": #{#parameters.![value]} + } + } + ] + } + } + """) + Flux> queryByParameterPropertyCollectionSpEL(Collection parameters); + + @Query(""" + { + "bool":{ + "must":[ + { + "terms":{ + "message": #{#e.![value]} + } + } + ] + } + } + """) + Flux> queryByParameterPropertyCollectionSpELWithParamAnnotation( + @Param("e") Collection parameters); + Mono findFirstByMessageLike(String message); Mono countAllByMessage(String message); From 2ffd2ffc02666065e2713ffad0506274f22b08a5 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Thu, 18 Jan 2024 16:10:13 +0800 Subject: [PATCH 10/11] modify QueryStringSpELEvaluator --- .../repository/query/ElasticsearchStringQuery.java | 5 +++-- .../query/ReactiveElasticsearchStringQuery.java | 5 +++-- .../support/spel/QueryStringSpELEvaluator.java | 13 +++++++------ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java index b50aa012e..8a0ae64d7 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQuery.java @@ -67,8 +67,9 @@ protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor paramet String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService()) .replacePlaceholders(this.queryString, parameterAccessor); - QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, evaluationContextProvider); - var query = new StringQuery(evaluator.evaluate(queryMethod)); + QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, queryMethod, + evaluationContextProvider); + var query = new StringQuery(evaluator.evaluate()); query.addSort(parameterAccessor.getSort()); return query; } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java index c1b49425f..66c36ad84 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ReactiveElasticsearchStringQuery.java @@ -55,8 +55,9 @@ protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor paramet String queryString = new StringQueryUtil(getElasticsearchOperations().getElasticsearchConverter().getConversionService()) .replacePlaceholders(this.query, parameterAccessor); - QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, evaluationContextProvider); - return new StringQuery(evaluator.evaluate(queryMethod)); + QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, queryMethod, + evaluationContextProvider); + return new StringQuery(evaluator.evaluate()); } @Override diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java index 263db8a60..41123d250 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/spel/QueryStringSpELEvaluator.java @@ -48,13 +48,15 @@ public class QueryStringSpELEvaluator { private final String queryString; private final ElasticsearchParametersParameterAccessor parameterAccessor; + private final QueryMethod queryMethod; private final QueryMethodEvaluationContextProvider evaluationContextProvider; private final TypeConverter elasticsearchSpELTypeConverter; public QueryStringSpELEvaluator(String queryString, ElasticsearchParametersParameterAccessor parameterAccessor, - QueryMethodEvaluationContextProvider evaluationContextProvider) { + QueryMethod queryMethod, QueryMethodEvaluationContextProvider evaluationContextProvider) { this.queryString = queryString; this.parameterAccessor = parameterAccessor; + this.queryMethod = queryMethod; this.evaluationContextProvider = evaluationContextProvider; this.elasticsearchSpELTypeConverter = new StandardTypeConverter(ElasticsearchValueSpELConversionService.CONVERSION_SERVICE_LAZY); } @@ -62,10 +64,9 @@ public QueryStringSpELEvaluator(String queryString, ElasticsearchParametersParam /** * Evaluate the SpEL parts of the query string. * - * @param queryMethod the query method * @return a plain string with values evaluated */ - public String evaluate(QueryMethod queryMethod) { + public String evaluate() { Expression expr = getQueryExpression(queryString); if (expr != null) { EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), @@ -74,7 +75,7 @@ public String evaluate(QueryMethod queryMethod) { standardEvaluationContext.setTypeConverter(elasticsearchSpELTypeConverter); } - String parsed = parseExpressions(expr, context, queryMethod); + String parsed = parseExpressions(expr, context); Assert.notNull(parsed, "Query parsed by SpEL should not be null"); return parsed; } @@ -89,7 +90,7 @@ public String evaluate(QueryMethod queryMethod) { * So we just get the string value from {@link LiteralExpression} directly rather than * {@link LiteralExpression#getValue(EvaluationContext, Class)}. */ - private String parseExpressions(Expression rootExpr, EvaluationContext context, QueryMethod queryMethod) { + private String parseExpressions(Expression rootExpr, EvaluationContext context) { StringBuilder parsed = new StringBuilder(); if (rootExpr instanceof LiteralExpression literalExpression) { // get the string literal directly @@ -108,7 +109,7 @@ private String parseExpressions(Expression rootExpr, EvaluationContext context, Expression[] expressions = compositeStringExpression.getExpressions(); for (Expression exp : expressions) { - parsed.append(parseExpressions(exp, context, queryMethod)); + parsed.append(parseExpressions(exp, context)); } } else { // no more From 1157081e61b26ea72de0e10068005c1d95123995 Mon Sep 17 00:00:00 2001 From: puppylpg Date: Thu, 18 Jan 2024 20:31:45 +0800 Subject: [PATCH 11/11] Add exception tests for reactive query --- ...ustomMethodRepositoryIntegrationTests.java | 12 +++++- ...asticsearchRepositoryIntegrationTests.java | 42 ++++++++++++++++++- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java index 6e5aef693..9bb507598 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repositories/custommethod/CustomMethodRepositoryIntegrationTests.java @@ -2072,6 +2072,10 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByString(String type); + /** + * The parameter is annotated with {@link Nullable} deliberately to test that our elasticsearch SpEL converters + * will not accept a null parameter as query value. + */ @Query(""" { "bool":{ @@ -2085,7 +2089,7 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByStringSpEL(String type); + SearchHits queryByStringSpEL(@Nullable String type); @Query(""" { @@ -2117,6 +2121,10 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByBeanPropertySpEL(); + /** + * The parameter is annotated with {@link Nullable} deliberately to test that our elasticsearch SpEL converters + * will not accept a null parameter as query value. + */ @Query(""" { "bool":{ @@ -2130,7 +2138,7 @@ public interface SampleCustomMethodRepository extends ElasticsearchRepository queryByCollectionSpEL(Collection types); + SearchHits queryByCollectionSpEL(@Nullable Collection types); @Query(""" { diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java index 33154ba9f..f7ebbc2fe 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryIntegrationTests.java @@ -16,10 +16,12 @@ package org.springframework.data.elasticsearch.repository.support; import static org.assertj.core.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.*; import static org.springframework.data.elasticsearch.core.IndexOperationsAdapter.*; import static org.springframework.data.elasticsearch.core.query.Query.*; import static org.springframework.data.elasticsearch.utils.IdGenerator.*; +import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.repositories.custommethod.QueryParameter; import org.springframework.data.repository.query.Param; import reactor.core.publisher.Flux; @@ -251,6 +253,20 @@ void shouldReturnSearchHitsForStringQuerySpEL() { .verifyComplete(); } + @Test + void shouldRaiseExceptionForNullStringQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + ConversionException thrown = assertThrows(ConversionException.class, () -> repository.queryByStringSpEL(null)); + + assertThat(thrown.getMessage()) + .isEqualTo("Parameter value can't be null for SpEL expression '#message' in method 'queryByStringSpEL'" + + " when querying elasticsearch"); + } + @Test void shouldReturnSearchHitsForParameterPropertyQuerySpEL() { bulkIndex(new SampleEntity("id-one", "message"), // @@ -295,6 +311,20 @@ void shouldReturnSearchHitsForCollectionQuerySpEL() { .verifyComplete(); } + @Test + void shouldRaiseExceptionForNullCollectionQuerySpEL() { + bulkIndex(new SampleEntity("id-one", "message"), // + new SampleEntity("id-two", "message"), // + new SampleEntity("id-three", "message")) // + .block(); + + ConversionException thrown = assertThrows(ConversionException.class, () -> repository.queryByCollectionSpEL(null)); + + assertThat(thrown.getMessage()) + .isEqualTo("Parameter value can't be null for SpEL expression '#messages' in method 'queryByCollectionSpEL'" + + " when querying elasticsearch"); + } + @Test void shouldNotReturnSearchHitsForEmptyCollectionQuerySpEL() { bulkIndex(new SampleEntity("id-one", "message"), // @@ -923,6 +953,10 @@ interface ReactiveSampleEntityRepository extends ReactiveCrudRepository findAllViaAnnotatedQueryByMessageLikePaged(String message, Pageable pageable); + /** + * The parameter is annotated with {@link Nullable} deliberately to test that our elasticsearch SpEL converters + * will not accept a null parameter as query value. + */ @Query(""" { "bool":{ @@ -936,7 +970,7 @@ interface ReactiveSampleEntityRepository extends ReactiveCrudRepository> queryByStringSpEL(String message); + Flux> queryByStringSpEL(@Nullable String message); @Query(""" { @@ -968,6 +1002,10 @@ interface ReactiveSampleEntityRepository extends ReactiveCrudRepository> queryByBeanPropertySpEL(); + /** + * The parameter is annotated with {@link Nullable} deliberately to test that our elasticsearch SpEL converters + * will not accept a null parameter as query value. + */ @Query(""" { "bool":{ @@ -981,7 +1019,7 @@ interface ReactiveSampleEntityRepository extends ReactiveCrudRepository> queryByCollectionSpEL(Collection messages); + Flux> queryByCollectionSpEL(@Nullable Collection messages); @Query(""" {