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 16d76dea1..b558dacb4 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 @@ -42,6 +42,7 @@ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQue public ElasticsearchStringQuery(ElasticsearchQueryMethod queryMethod, ElasticsearchOperations elasticsearchOperations, String queryString, QueryMethodEvaluationContextProvider evaluationContextProvider) { super(queryMethod, elasticsearchOperations); + Assert.notNull(queryString, "Query cannot be empty"); Assert.notNull(evaluationContextProvider, "ExpressionEvaluationContextProvider must not be null"); @@ -65,11 +66,11 @@ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQue } protected BaseQuery createQuery(ElasticsearchParametersParameterAccessor parameterAccessor) { - ConversionService conversionService = elasticsearchOperations.getElasticsearchConverter().getConversionService(); - String queryString = new StringQueryUtil(conversionService) - .replacePlaceholders(this.queryString, parameterAccessor); - QueryStringSpELEvaluator evaluator = new QueryStringSpELEvaluator(queryString, parameterAccessor, queryMethod, + ConversionService conversionService = elasticsearchOperations.getElasticsearchConverter().getConversionService(); + var replacedString = new StringQueryUtil(conversionService).replacePlaceholders(this.queryString, + parameterAccessor); + var evaluator = new QueryStringSpELEvaluator(replacedString, parameterAccessor, queryMethod, evaluationContextProvider, conversionService); var query = new StringQuery(evaluator.evaluate()); query.addSort(parameterAccessor.getSort()); 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 266e1978c..368577c47 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 @@ -23,6 +23,7 @@ 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; +import org.springframework.util.Assert; /** * @author Christoph Strobl @@ -45,8 +46,11 @@ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsea public ReactiveElasticsearchStringQuery(String query, ReactiveElasticsearchQueryMethod queryMethod, ReactiveElasticsearchOperations operations, SpelExpressionParser expressionParser, QueryMethodEvaluationContextProvider evaluationContextProvider) { - super(queryMethod, operations); + + Assert.notNull(query, "query must not be null"); + Assert.notNull(evaluationContextProvider, "evaluationContextProvider must not be null"); + this.query = query; this.evaluationContextProvider = evaluationContextProvider; } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java index 62c6a5bb1..ff429a26d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java @@ -22,6 +22,7 @@ import org.springframework.core.convert.ConversionService; import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.repository.support.value.ElasticsearchQueryValueConversionService; import org.springframework.data.repository.query.ParameterAccessor; +import org.springframework.util.Assert; import org.springframework.util.NumberUtils; /** @@ -36,6 +37,9 @@ final public class StringQueryUtil { private final ConversionService conversionService; public StringQueryUtil(ConversionService conversionService) { + + Assert.notNull(conversionService, "conversionService must not be null"); + this.conversionService = ElasticsearchQueryValueConversionService.getInstance(conversionService); } @@ -43,8 +47,8 @@ final public class StringQueryUtil { Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input); String result = input; - while (matcher.find()) { + while (matcher.find()) { String placeholder = Pattern.quote(matcher.group()) + "(?!\\d+)"; int index = NumberUtils.parseNumber(matcher.group(1), Integer.class); String replacement = Matcher.quoteReplacement(getParameterWithIndex(accessor, index, input)); @@ -59,7 +63,8 @@ final public class StringQueryUtil { private String getParameterWithIndex(ParameterAccessor accessor, int index, String input) { Object parameter = accessor.getBindableValue(index); - String value = conversionService.convert(parameter, String.class); + String value = conversionService.convert(parameter, String.class); + if (value == null) { throw new ConversionException(String.format( "Parameter value can't be null for placeholder at index '%s' in query '%s' when querying elasticsearch", 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 e7f5876ee..66fe369b4 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 @@ -21,8 +21,8 @@ import java.util.concurrent.ConcurrentHashMap; import org.springframework.core.convert.ConversionService; import org.springframework.data.elasticsearch.core.convert.ConversionException; import org.springframework.data.elasticsearch.repository.query.ElasticsearchParametersParameterAccessor; -import org.springframework.data.elasticsearch.repository.support.value.ElasticsearchQueryValueConversionService; import org.springframework.data.elasticsearch.repository.support.value.ElasticsearchCollectionValueToStringConverter; +import org.springframework.data.elasticsearch.repository.support.value.ElasticsearchQueryValueConversionService; import org.springframework.data.elasticsearch.repository.support.value.ElasticsearchStringValueToStringConverter; import org.springframework.data.repository.query.QueryMethod; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; @@ -59,6 +59,13 @@ public class QueryStringSpELEvaluator { public QueryStringSpELEvaluator(String queryString, ElasticsearchParametersParameterAccessor parameterAccessor, QueryMethod queryMethod, QueryMethodEvaluationContextProvider evaluationContextProvider, ConversionService conversionService) { + + Assert.notNull(queryString, "queryString must not be null"); + Assert.notNull(parameterAccessor, "parameterAccessor must not be null"); + Assert.notNull(queryMethod, "queryMethod must not be null"); + Assert.notNull(evaluationContextProvider, "evaluationContextProvider must not be null"); + Assert.notNull(conversionService, "conversionService must not be null"); + this.queryString = queryString; this.parameterAccessor = parameterAccessor; this.queryMethod = queryMethod; @@ -74,9 +81,11 @@ public class QueryStringSpELEvaluator { */ public String evaluate() { Expression expr = getQueryExpression(queryString); + if (expr != null) { EvaluationContext context = evaluationContextProvider.getEvaluationContext(parameterAccessor.getParameters(), parameterAccessor.getValues()); + if (context instanceof StandardEvaluationContext standardEvaluationContext) { standardEvaluationContext.setTypeConverter(elasticsearchSpELTypeConverter); } @@ -97,12 +106,14 @@ public class QueryStringSpELEvaluator { */ 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", diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchCollectionValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchCollectionValueToStringConverter.java index 835a54d8d..4179fa1a1 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchCollectionValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchCollectionValueToStringConverter.java @@ -70,14 +70,18 @@ public class ElasticsearchCollectionValueToStringConverter implements GenericCon @Override @Nullable public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + if (source == null) { return "[]"; } Collection sourceCollection = (Collection) source; + if (sourceCollection.isEmpty()) { return "[]"; } + StringJoiner sb = new StringJoiner(DELIMITER, "[", "]"); + for (Object sourceElement : sourceCollection) { // ignore the null value in collection if (Objects.isNull(sourceElement)) { @@ -86,8 +90,9 @@ public class ElasticsearchCollectionValueToStringConverter implements GenericCon Object targetElement = this.conversionService.convert( sourceElement, sourceType.elementTypeDescriptor(sourceElement), targetType); + if (sourceElement instanceof String) { - sb.add("\"" + targetElement + "\""); + sb.add("\"" + targetElement + '"'); } else { sb.add(String.valueOf(targetElement)); } diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchQueryValueConversionService.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchQueryValueConversionService.java index c0f8b8abd..5f276bc80 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchQueryValueConversionService.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchQueryValueConversionService.java @@ -27,7 +27,8 @@ import org.springframework.util.Assert; /** * A {@link ConversionService} using custom converters to handle query values in elasticsearch query. If the value to be - * converted beyond the scope of custom converters, it'll delegate to the {@link #delegate delegated conversion service}. + * converted beyond the scope of custom converters, it'll delegate to the {@link #delegate delegated conversion + * service}. *

* This is a better solution for converting query values in elasticsearch query, because it has all the capability the * {@link #delegate delegated conversion service} has, especially for user-registered {@link Converter}s. @@ -44,7 +45,9 @@ public class ElasticsearchQueryValueConversionService implements ConversionServi private final ConversionService delegate; private ElasticsearchQueryValueConversionService(ConversionService delegate) { + Assert.notNull(delegate, "delegated ConversionService must not be null"); + this.delegate = delegate; // register elasticsearch custom type converters for conversion service @@ -85,6 +88,7 @@ public class ElasticsearchQueryValueConversionService implements ConversionServi @Override @Nullable public Object convert(@Nullable Object source, @Nullable TypeDescriptor sourceType, TypeDescriptor targetType) { + if (valueConversionService.canConvert(sourceType, targetType)) { return valueConversionService.convert(source, sourceType, targetType); } else { diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchStringValueToStringConverter.java b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchStringValueToStringConverter.java index 610445ae6..b336e075d 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchStringValueToStringConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/support/value/ElasticsearchStringValueToStringConverter.java @@ -40,10 +40,7 @@ public class ElasticsearchStringValueToStringConverter implements GenericConvert @Override @Nullable public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - if (source == null) { - return null; - } - return escape(source); + return source != null ? escape(source) : null; } private String escape(@Nullable Object 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 10cc3b068..078cdb684 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 @@ -2109,8 +2109,8 @@ public abstract class CustomMethodRepositoryIntegrationTests { SearchHits queryByType(String type); /** - * The parameter is annotated with {@link Nullable} deliberately to test that our placeholder parameter will - * not accept a null parameter as query value. + * The parameter is annotated with {@link Nullable} deliberately to test that our placeholder parameter will not + * accept a null parameter as query value. */ @Query("{\"bool\": {\"must\": [{\"term\": {\"type\": \"?0\"}}]}}") @Highlight(fields = { @HighlightField(name = "type") }) @@ -2444,8 +2444,7 @@ public abstract class CustomMethodRepositoryIntegrationTests { } static class SampleProperty { - @Nullable - private String first; + @Nullable private String first; @Nullable private String last; SampleProperty(@Nullable String first, @Nullable String last) {