Skip to content

Commit 626b274

Browse files
sothawosothawo
committed
Use registered converters for parameters of @query annotated methods.
Original Pull Request #1867 Closes #1866 (cherry picked from commit 2709472) (cherry picked from commit 303438a)
1 parent 979c164 commit 626b274

File tree

6 files changed

+173
-108
lines changed

6 files changed

+173
-108
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public Object execute(Object[] parameters) {
9494
}
9595

9696
protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) {
97-
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
97+
String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService())
98+
.replacePlaceholders(this.query, parameterAccessor);
9899
return new StringQuery(queryString);
99100
}
100101

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ public ReactiveElasticsearchStringQuery(String query, ReactiveElasticsearchQuery
4747

4848
@Override
4949
protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) {
50-
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
50+
String queryString = new StringQueryUtil(
51+
getElasticsearchOperations().getElasticsearchConverter().getConversionService()).replacePlaceholders(this.query,
52+
parameterAccessor);
5153
return new StringQuery(queryString);
5254
}
5355

‎src/main/java/org/springframework/data/elasticsearch/repository/support/StringQueryUtil.java

+43-21
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
import java.util.regex.Pattern;
2121
import java.util.stream.Collectors;
2222

23+
import org.springframework.core.convert.ConversionService;
2324
import org.springframework.core.convert.support.GenericConversionService;
2425
import org.springframework.data.elasticsearch.core.convert.DateTimeConverters;
2526
import org.springframework.data.elasticsearch.repository.query.ElasticsearchStringQuery;
2627
import org.springframework.data.repository.query.ParameterAccessor;
28+
import org.springframework.util.Assert;
2729
import org.springframework.util.ClassUtils;
2830
import org.springframework.util.NumberUtils;
2931

@@ -34,25 +36,38 @@
3436
final public class StringQueryUtil {
3537

3638
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
37-
private static final GenericConversionService conversionService = new GenericConversionService();
38-
39-
{
40-
if (!conversionService.canConvert(java.util.Date.class, String.class)) {
41-
conversionService.addConverter(DateTimeConverters.JavaDateConverter.INSTANCE);
42-
}
43-
if (ClassUtils.isPresent("org.joda.time.DateTimeZone", ElasticsearchStringQuery.class.getClassLoader())) {
44-
if (!conversionService.canConvert(org.joda.time.ReadableInstant.class, String.class)) {
45-
conversionService.addConverter(DateTimeConverters.JodaDateTimeConverter.INSTANCE);
46-
}
47-
if (!conversionService.canConvert(org.joda.time.LocalDateTime.class, String.class)) {
48-
conversionService.addConverter(DateTimeConverters.JodaLocalDateTimeConverter.INSTANCE);
49-
}
50-
}
51-
}
52-
53-
private StringQueryUtil() {}
54-
55-
public static String replacePlaceholders(String input, ParameterAccessor accessor) {
39+
40+
private final ConversionService conversionService;
41+
private final GenericConversionService genericConversionService;
42+
43+
public StringQueryUtil(ConversionService conversionService) {
44+
45+
Assert.notNull(conversionService, "conversionService must not be null");
46+
47+
this.conversionService = conversionService;
48+
genericConversionService = setupGenericConversionService();
49+
}
50+
51+
private GenericConversionService setupGenericConversionService() {
52+
53+
GenericConversionService genericConversionService = new GenericConversionService();
54+
55+
if (!genericConversionService.canConvert(java.util.Date.class, String.class)) {
56+
genericConversionService.addConverter(DateTimeConverters.JavaDateConverter.INSTANCE);
57+
}
58+
59+
if (ClassUtils.isPresent("org.joda.time.DateTimeZone", ElasticsearchStringQuery.class.getClassLoader())) {
60+
if (!genericConversionService.canConvert(org.joda.time.ReadableInstant.class, String.class)) {
61+
genericConversionService.addConverter(DateTimeConverters.JodaDateTimeConverter.INSTANCE);
62+
}
63+
if (!genericConversionService.canConvert(org.joda.time.LocalDateTime.class, String.class)) {
64+
genericConversionService.addConverter(DateTimeConverters.JodaLocalDateTimeConverter.INSTANCE);
65+
}
66+
}
67+
return genericConversionService;
68+
}
69+
70+
public String replacePlaceholders(String input, ParameterAccessor accessor) {
5671

5772
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
5873
String result = input;
@@ -65,7 +80,7 @@ public static String replacePlaceholders(String input, ParameterAccessor accesso
6580
return result;
6681
}
6782

68-
private static String getParameterWithIndex(ParameterAccessor accessor, int index) {
83+
private String getParameterWithIndex(ParameterAccessor accessor, int index) {
6984

7085
Object parameter = accessor.getBindableValue(index);
7186
String parameterValue = "null";
@@ -80,7 +95,7 @@ private static String getParameterWithIndex(ParameterAccessor accessor, int inde
8095

8196
}
8297

83-
private static String convert(Object parameter) {
98+
private String convert(Object parameter) {
8499
if (Collection.class.isAssignableFrom(parameter.getClass())) {
85100
Collection<?> collectionParam = (Collection<?>) parameter;
86101
StringBuilder sb = new StringBuilder("[");
@@ -101,6 +116,13 @@ private static String convert(Object parameter) {
101116
if (converted != null) {
102117
parameterValue = converted;
103118
}
119+
} else if (genericConversionService.canConvert(parameter.getClass(), String.class)) {
120+
String converted = genericConversionService.convert(parameter, String.class);
121+
122+
if (converted != null) {
123+
parameterValue = converted;
124+
125+
}
104126
} else {
105127
parameterValue = parameter.toString();
106128
}

‎src/test/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchStringQueryUnitTestBase.java

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Copyright 2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.elasticsearch.repository.query;
17+
18+
import java.util.ArrayList;
19+
import java.util.Collection;
20+
21+
import org.springframework.core.convert.converter.Converter;
22+
import org.springframework.data.convert.CustomConversions;
23+
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
24+
import org.springframework.data.elasticsearch.core.convert.ElasticsearchCustomConversions;
25+
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
26+
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
27+
import org.springframework.lang.Nullable;
28+
29+
/**
30+
* @author Peter-Josef Meisch
31+
*/
32+
public class ElasticsearchStringQueryUnitTestBase {
33+
34+
protected ElasticsearchConverter setupConverter() {
35+
MappingElasticsearchConverter converter = new MappingElasticsearchConverter(
36+
new SimpleElasticsearchMappingContext());
37+
Collection<Converter<?, ?>> converters = new ArrayList<>();
38+
converters.add(ElasticsearchStringQueryUnitTests.CarConverter.INSTANCE);
39+
CustomConversions customConversions = new ElasticsearchCustomConversions(converters);
40+
converter.setConversions(customConversions);
41+
converter.afterPropertiesSet();
42+
return converter;
43+
}
44+
45+
static class Car {
46+
@Nullable private String name;
47+
@Nullable private String model;
48+
49+
@Nullable
50+
public String getName() {
51+
return name;
52+
}
53+
54+
public void setName(@Nullable String name) {
55+
this.name = name;
56+
}
57+
58+
@Nullable
59+
public String getModel() {
60+
return model;
61+
}
62+
63+
public void setModel(@Nullable String model) {
64+
this.model = model;
65+
}
66+
}
67+
68+
enum CarConverter implements Converter<Car, String> {
69+
INSTANCE;
70+
71+
@Override
72+
public String convert(ElasticsearchStringQueryUnitTests.Car car) {
73+
return (car.getName() != null ? car.getName() : "null") + '-'
74+
+ (car.getModel() != null ? car.getModel() : "null");
75+
}
76+
}
77+
78+
}

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

+23-40
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.elasticsearch.repository.query;
1717

1818
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.Mockito.*;
1920

2021
import lombok.AllArgsConstructor;
2122
import lombok.Builder;
@@ -46,9 +47,6 @@
4647
import org.springframework.data.elasticsearch.annotations.Query;
4748
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
4849
import org.springframework.data.elasticsearch.core.SearchHits;
49-
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
50-
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
51-
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
5250
import org.springframework.data.elasticsearch.core.query.StringQuery;
5351
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
5452
import org.springframework.data.repository.Repository;
@@ -61,14 +59,13 @@
6159
* @author Niklas Herder
6260
*/
6361
@ExtendWith(MockitoExtension.class)
64-
public class ElasticsearchStringQueryUnitTests {
62+
public class ElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase {
6563

6664
@Mock ElasticsearchOperations operations;
67-
ElasticsearchConverter converter;
6865

6966
@BeforeEach
7067
public void setUp() {
71-
converter = new MappingElasticsearchConverter(new SimpleElasticsearchMappingContext());
68+
when(operations.getElasticsearchConverter()).thenReturn(setupConverter());
7269
}
7370

7471
@Test // DATAES-552
@@ -147,6 +144,21 @@ private org.springframework.data.elasticsearch.core.query.Query createQuery(Stri
147144
return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
148145
}
149146

147+
@Test // #1866
148+
@DisplayName("should use converter on parameters")
149+
void shouldUseConverterOnParameters() throws NoSuchMethodException {
150+
151+
Car car = new Car();
152+
car.setName("Toyota");
153+
car.setModel("Prius");
154+
155+
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByCar", car);
156+
157+
assertThat(query).isInstanceOf(StringQuery.class);
158+
assertThat(((StringQuery) query).getSource())
159+
.isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'car' : 'Toyota-Prius' } } } }");
160+
}
161+
150162
private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) {
151163
return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery());
152164
}
@@ -155,7 +167,7 @@ private ElasticsearchQueryMethod getQueryMethod(String name, Class<?>... paramet
155167

156168
Method method = SampleRepository.class.getMethod(name, parameters);
157169
return new ElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
158-
new SpelAwareProxyProjectionFactory(), converter.getMappingContext());
170+
new SpelAwareProxyProjectionFactory(), operations.getElasticsearchConverter().getMappingContext());
159171
}
160172

161173
private interface SampleRepository extends Repository<Person, String> {
@@ -178,13 +190,16 @@ Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String
178190

179191
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
180192
SearchHits<Book> findByPrefix(String prefix);
193+
194+
@Query("{ 'bool' : { 'must' : { 'term' : { 'car' : '?0' } } } }")
195+
Person findByCar(Car car);
181196
}
182197

183198
/**
184199
* @author Rizwan Idrees
185200
* @author Mohsin Husen
186201
* @author Artur Konczak
187-
* @author Niklas Herder
202+
* @author Niklas Herder
188203
*/
189204

190205
@Document(indexName = "test-index-person-query-unittest", replicas = 0, refreshInterval = "-1")
@@ -264,38 +279,6 @@ static class Book {
264279
searchAnalyzer = "standard") }) private String description;
265280
}
266281

267-
/**
268-
* @author Rizwan Idrees
269-
* @author Mohsin Husen
270-
* @author Artur Konczak
271-
*/
272-
@Setter
273-
@Getter
274-
@NoArgsConstructor
275-
@AllArgsConstructor
276-
@Builder
277-
static class Car {
278-
279-
private String name;
280-
private String model;
281-
282-
public String getName() {
283-
return name;
284-
}
285-
286-
public void setName(String name) {
287-
this.name = name;
288-
}
289-
290-
public String getModel() {
291-
return model;
292-
}
293-
294-
public void setModel(String model) {
295-
this.model = model;
296-
}
297-
}
298-
299282
/**
300283
* @author Rizwan Idrees
301284
* @author Mohsin Husen

0 commit comments

Comments
 (0)