Skip to content

Commit 2709472

Browse files
sothawosothawo
authored
Use registered converters for parameters of @query annotated methods.
Original Pull Request #1867 Closes #1866
1 parent 567bdf2 commit 2709472

File tree

6 files changed

+138
-71
lines changed

6 files changed

+138
-71
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ public Object execute(Object[] parameters) {
102102
}
103103

104104
protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) {
105-
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor);
105+
String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService())
106+
.replacePlaceholders(this.query, parameterAccessor);
106107
return new StringQuery(queryString);
107108
}
108109

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

Lines changed: 3 additions & 1 deletion
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‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.util.regex.Pattern;
2121
import java.util.stream.Collectors;
2222

23-
import org.springframework.core.convert.support.GenericConversionService;
23+
import org.springframework.core.convert.ConversionService;
2424
import org.springframework.data.repository.query.ParameterAccessor;
2525
import org.springframework.util.NumberUtils;
2626

@@ -31,11 +31,14 @@
3131
final public class StringQueryUtil {
3232

3333
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
34-
private static final GenericConversionService conversionService = new GenericConversionService();
3534

36-
private StringQueryUtil() {}
35+
private final ConversionService conversionService;
3736

38-
public static String replacePlaceholders(String input, ParameterAccessor accessor) {
37+
public StringQueryUtil(ConversionService conversionService) {
38+
this.conversionService = conversionService;
39+
}
40+
41+
public String replacePlaceholders(String input, ParameterAccessor accessor) {
3942

4043
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
4144
String result = input;
@@ -48,7 +51,7 @@ public static String replacePlaceholders(String input, ParameterAccessor accesso
4851
return result;
4952
}
5053

51-
private static String getParameterWithIndex(ParameterAccessor accessor, int index) {
54+
private String getParameterWithIndex(ParameterAccessor accessor, int index) {
5255

5356
Object parameter = accessor.getBindableValue(index);
5457
String parameterValue = "null";
@@ -63,7 +66,7 @@ private static String getParameterWithIndex(ParameterAccessor accessor, int inde
6366

6467
}
6568

66-
private static String convert(Object parameter) {
69+
private String convert(Object parameter) {
6770
if (Collection.class.isAssignableFrom(parameter.getClass())) {
6871
Collection<?> collectionParam = (Collection<?>) parameter;
6972
StringBuilder sb = new StringBuilder("[");

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

Lines changed: 78 additions & 0 deletions
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‎

Lines changed: 23 additions & 32 deletions
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
}

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

Lines changed: 23 additions & 31 deletions
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 reactor.core.publisher.Flux;
2122
import reactor.core.publisher.Mono;
@@ -43,9 +44,6 @@
4344
import org.springframework.data.elasticsearch.annotations.Query;
4445
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
4546
import org.springframework.data.elasticsearch.core.SearchHit;
46-
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
47-
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
48-
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
4947
import org.springframework.data.elasticsearch.core.query.StringQuery;
5048
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
5149
import org.springframework.data.repository.Repository;
@@ -59,16 +57,15 @@
5957
* @author Peter-Josef Meisch
6058
*/
6159
@ExtendWith(MockitoExtension.class)
62-
public class ReactiveElasticsearchStringQueryUnitTests {
60+
public class ReactiveElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase {
6361

6462
SpelExpressionParser PARSER = new SpelExpressionParser();
65-
ElasticsearchConverter converter;
6663

6764
@Mock ReactiveElasticsearchOperations operations;
6865

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

7471
@Test // DATAES-519
@@ -132,7 +129,22 @@ void shouldEscapeStringsInQueryParameters() throws Exception {
132129
.isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}");
133130
}
134131

135-
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args)
132+
@Test // #1866
133+
@DisplayName("should use converter on parameters")
134+
void shouldUseConverterOnParameters() throws Exception {
135+
136+
Car car = new Car();
137+
car.setName("Toyota");
138+
car.setModel("Prius");
139+
140+
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByCar", car);
141+
142+
assertThat(query).isInstanceOf(StringQuery.class);
143+
assertThat(((StringQuery) query).getSource())
144+
.isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'car' : 'Toyota-Prius' } } } }");
145+
}
146+
147+
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, Object... args)
136148
throws NoSuchMethodException {
137149

138150
Class<?>[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new);
@@ -152,7 +164,7 @@ private ReactiveElasticsearchQueryMethod getQueryMethod(String name, Class<?>...
152164

153165
Method method = SampleRepository.class.getMethod(name, parameters);
154166
return new ReactiveElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
155-
new SpelAwareProxyProjectionFactory(), converter.getMappingContext());
167+
new SpelAwareProxyProjectionFactory(), operations.getElasticsearchConverter().getMappingContext());
156168
}
157169

158170
private ReactiveElasticsearchStringQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
@@ -180,6 +192,9 @@ Person findWithRepeatedPlaceholder(String arg0, String arg1, String arg2, String
180192
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
181193
Flux<SearchHit<Book>> findByPrefix(String prefix);
182194

195+
@Query("{ 'bool' : { 'must' : { 'term' : { 'car' : '?0' } } } }")
196+
Mono<Person> findByCar(Car car);
197+
183198
}
184199

185200
/**
@@ -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;

0 commit comments

Comments
 (0)