Skip to content

Commit 303438a

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

File tree

6 files changed

+172
-86
lines changed

6 files changed

+172
-86
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ public Object execute(Object[] parameters) {
105105
}
106106

107107
protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) {
108-
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
108+
String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService())
109+
.replacePlaceholders(this.query, parameterAccessor);
109110
return new StringQuery(queryString);
110111
}
111112

‎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-32
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 java.lang.reflect.Method;
2122
import java.util.ArrayList;
@@ -40,9 +41,6 @@
4041
import org.springframework.data.elasticsearch.annotations.Query;
4142
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
4243
import org.springframework.data.elasticsearch.core.SearchHits;
43-
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
44-
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
45-
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
4644
import org.springframework.data.elasticsearch.core.query.StringQuery;
4745
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
4846
import org.springframework.data.repository.Repository;
@@ -55,14 +53,13 @@
5553
* @author Niklas Herder
5654
*/
5755
@ExtendWith(MockitoExtension.class)
58-
public class ElasticsearchStringQueryUnitTests {
56+
public class ElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase {
5957

6058
@Mock ElasticsearchOperations operations;
61-
ElasticsearchConverter converter;
6259

6360
@BeforeEach
6461
public void setUp() {
65-
converter = new MappingElasticsearchConverter(new SimpleElasticsearchMappingContext());
62+
when(operations.getElasticsearchConverter()).thenReturn(setupConverter());
6663
}
6764

6865
@Test // DATAES-552
@@ -141,6 +138,21 @@ private org.springframework.data.elasticsearch.core.query.Query createQuery(Stri
141138
return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
142139
}
143140

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

150162
Method method = SampleRepository.class.getMethod(name, parameters);
151163
return new ElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
152-
new SpelAwareProxyProjectionFactory(), converter.getMappingContext());
164+
new SpelAwareProxyProjectionFactory(), operations.getElasticsearchConverter().getMappingContext());
153165
}
154166

155167
private interface SampleRepository extends Repository<Person, String> {
@@ -172,13 +184,16 @@ Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String
172184

173185
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
174186
SearchHits<Book> findByPrefix(String prefix);
187+
188+
@Query("{ 'bool' : { 'must' : { 'term' : { 'car' : '?0' } } } }")
189+
Person findByCar(Car car);
175190
}
176191

177192
/**
178193
* @author Rizwan Idrees
179194
* @author Mohsin Husen
180195
* @author Artur Konczak
181-
* @author Niklas Herder
196+
* @author Niklas Herder
182197
*/
183198

184199
@Document(indexName = "test-index-person-query-unittest")
@@ -292,29 +307,6 @@ public void setDescription(@Nullable String description) {
292307
}
293308
}
294309

295-
static class Car {
296-
@Nullable private String name;
297-
@Nullable private String model;
298-
299-
@Nullable
300-
public String getName() {
301-
return name;
302-
}
303-
304-
public void setName(@Nullable String name) {
305-
this.name = name;
306-
}
307-
308-
@Nullable
309-
public String getModel() {
310-
return model;
311-
}
312-
313-
public void setModel(@Nullable String model) {
314-
this.model = model;
315-
}
316-
}
317-
318310
static class Author {
319311

320312
@Nullable private String id;
@@ -338,5 +330,4 @@ public void setName(String name) {
338330
this.name = name;
339331
}
340332
}
341-
342333
}

0 commit comments

Comments
 (0)