Use registered converters for parameters of @Query annotated methods.

Original Pull Request #1867 
Closes #1866
This commit is contained in:
Peter-Josef Meisch 2021-07-14 19:31:30 +02:00 committed by GitHub
parent 567bdf21fa
commit 27094724dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 138 additions and 71 deletions

View File

@ -102,7 +102,8 @@ public class ElasticsearchStringQuery extends AbstractElasticsearchRepositoryQue
} }
protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) { protected StringQuery createQuery(ParametersParameterAccessor parameterAccessor) {
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor); String queryString = new StringQueryUtil(elasticsearchOperations.getElasticsearchConverter().getConversionService())
.replacePlaceholders(this.query, parameterAccessor);
return new StringQuery(queryString); return new StringQuery(queryString);
} }

View File

@ -47,7 +47,9 @@ public class ReactiveElasticsearchStringQuery extends AbstractReactiveElasticsea
@Override @Override
protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) { protected StringQuery createQuery(ElasticsearchParameterAccessor parameterAccessor) {
String queryString = StringQueryUtil.replacePlaceholders(this.query, parameterAccessor); String queryString = new StringQueryUtil(
getElasticsearchOperations().getElasticsearchConverter().getConversionService()).replacePlaceholders(this.query,
parameterAccessor);
return new StringQuery(queryString); return new StringQuery(queryString);
} }

View File

@ -20,7 +20,7 @@ import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.springframework.core.convert.support.GenericConversionService; import org.springframework.core.convert.ConversionService;
import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.ParameterAccessor;
import org.springframework.util.NumberUtils; import org.springframework.util.NumberUtils;
@ -31,11 +31,14 @@ import org.springframework.util.NumberUtils;
final public class StringQueryUtil { final public class StringQueryUtil {
private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)"); private static final Pattern PARAMETER_PLACEHOLDER = Pattern.compile("\\?(\\d+)");
private static final GenericConversionService conversionService = new GenericConversionService();
private StringQueryUtil() {} private final ConversionService conversionService;
public static String replacePlaceholders(String input, ParameterAccessor accessor) { public StringQueryUtil(ConversionService conversionService) {
this.conversionService = conversionService;
}
public String replacePlaceholders(String input, ParameterAccessor accessor) {
Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input); Matcher matcher = PARAMETER_PLACEHOLDER.matcher(input);
String result = input; String result = input;
@ -48,7 +51,7 @@ final public class StringQueryUtil {
return result; return result;
} }
private static String getParameterWithIndex(ParameterAccessor accessor, int index) { private String getParameterWithIndex(ParameterAccessor accessor, int index) {
Object parameter = accessor.getBindableValue(index); Object parameter = accessor.getBindableValue(index);
String parameterValue = "null"; String parameterValue = "null";
@ -63,7 +66,7 @@ final public class StringQueryUtil {
} }
private static String convert(Object parameter) { private String convert(Object parameter) {
if (Collection.class.isAssignableFrom(parameter.getClass())) { if (Collection.class.isAssignableFrom(parameter.getClass())) {
Collection<?> collectionParam = (Collection<?>) parameter; Collection<?> collectionParam = (Collection<?>) parameter;
StringBuilder sb = new StringBuilder("["); StringBuilder sb = new StringBuilder("[");

View File

@ -0,0 +1,78 @@
/*
* Copyright 2021 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.ArrayList;
import java.util.Collection;
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
import org.springframework.data.elasticsearch.core.convert.ElasticsearchCustomConversions;
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
import org.springframework.lang.Nullable;
/**
* @author Peter-Josef Meisch
*/
public class ElasticsearchStringQueryUnitTestBase {
protected ElasticsearchConverter setupConverter() {
MappingElasticsearchConverter converter = new MappingElasticsearchConverter(
new SimpleElasticsearchMappingContext());
Collection<Converter<?, ?>> converters = new ArrayList<>();
converters.add(ElasticsearchStringQueryUnitTests.CarConverter.INSTANCE);
CustomConversions customConversions = new ElasticsearchCustomConversions(converters);
converter.setConversions(customConversions);
converter.afterPropertiesSet();
return converter;
}
static class Car {
@Nullable private String name;
@Nullable private String model;
@Nullable
public String getName() {
return name;
}
public void setName(@Nullable String name) {
this.name = name;
}
@Nullable
public String getModel() {
return model;
}
public void setModel(@Nullable String model) {
this.model = model;
}
}
enum CarConverter implements Converter<Car, String> {
INSTANCE;
@Override
public String convert(ElasticsearchStringQueryUnitTests.Car car) {
return (car.getName() != null ? car.getName() : "null") + '-'
+ (car.getModel() != null ? car.getModel() : "null");
}
}
}

View File

@ -16,6 +16,7 @@
package org.springframework.data.elasticsearch.repository.query; package org.springframework.data.elasticsearch.repository.query;
import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.ArrayList; import java.util.ArrayList;
@ -40,9 +41,6 @@ import org.springframework.data.elasticsearch.annotations.MultiField;
import org.springframework.data.elasticsearch.annotations.Query; import org.springframework.data.elasticsearch.annotations.Query;
import org.springframework.data.elasticsearch.core.ElasticsearchOperations; import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
import org.springframework.data.elasticsearch.core.SearchHits; import org.springframework.data.elasticsearch.core.SearchHits;
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.core.query.StringQuery;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.Repository; import org.springframework.data.repository.Repository;
@ -55,14 +53,13 @@ import org.springframework.lang.Nullable;
* @author Niklas Herder * @author Niklas Herder
*/ */
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
public class ElasticsearchStringQueryUnitTests { public class ElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase {
@Mock ElasticsearchOperations operations; @Mock ElasticsearchOperations operations;
ElasticsearchConverter converter;
@BeforeEach @BeforeEach
public void setUp() { public void setUp() {
converter = new MappingElasticsearchConverter(new SimpleElasticsearchMappingContext()); when(operations.getElasticsearchConverter()).thenReturn(setupConverter());
} }
@Test // DATAES-552 @Test // DATAES-552
@ -141,6 +138,21 @@ public class ElasticsearchStringQueryUnitTests {
return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args)); return elasticsearchStringQuery.createQuery(new ElasticsearchParametersParameterAccessor(queryMethod, args));
} }
@Test // #1866
@DisplayName("should use converter on parameters")
void shouldUseConverterOnParameters() throws NoSuchMethodException {
Car car = new Car();
car.setName("Toyota");
car.setModel("Prius");
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByCar", car);
assertThat(query).isInstanceOf(StringQuery.class);
assertThat(((StringQuery) query).getSource())
.isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'car' : 'Toyota-Prius' } } } }");
}
private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) { private ElasticsearchStringQuery queryForMethod(ElasticsearchQueryMethod queryMethod) {
return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery()); return new ElasticsearchStringQuery(queryMethod, operations, queryMethod.getAnnotatedQuery());
} }
@ -149,7 +161,7 @@ public class ElasticsearchStringQueryUnitTests {
Method method = SampleRepository.class.getMethod(name, parameters); Method method = SampleRepository.class.getMethod(name, parameters);
return new ElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class), return new ElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
new SpelAwareProxyProjectionFactory(), converter.getMappingContext()); new SpelAwareProxyProjectionFactory(), operations.getElasticsearchConverter().getMappingContext());
} }
private interface SampleRepository extends Repository<Person, String> { private interface SampleRepository extends Repository<Person, String> {
@ -172,13 +184,16 @@ public class ElasticsearchStringQueryUnitTests {
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
SearchHits<Book> findByPrefix(String prefix); SearchHits<Book> findByPrefix(String prefix);
@Query("{ 'bool' : { 'must' : { 'term' : { 'car' : '?0' } } } }")
Person findByCar(Car car);
} }
/** /**
* @author Rizwan Idrees * @author Rizwan Idrees
* @author Mohsin Husen * @author Mohsin Husen
* @author Artur Konczak * @author Artur Konczak
* @author Niklas Herder * @author Niklas Herder
*/ */
@Document(indexName = "test-index-person-query-unittest") @Document(indexName = "test-index-person-query-unittest")
@ -292,29 +307,6 @@ public class ElasticsearchStringQueryUnitTests {
} }
} }
static class Car {
@Nullable private String name;
@Nullable private String model;
@Nullable
public String getName() {
return name;
}
public void setName(@Nullable String name) {
this.name = name;
}
@Nullable
public String getModel() {
return model;
}
public void setModel(@Nullable String model) {
this.model = model;
}
}
static class Author { static class Author {
@Nullable private String id; @Nullable private String id;
@ -338,5 +330,4 @@ public class ElasticsearchStringQueryUnitTests {
this.name = name; this.name = name;
} }
} }
} }

View File

@ -16,6 +16,7 @@
package org.springframework.data.elasticsearch.repository.query; package org.springframework.data.elasticsearch.repository.query;
import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import reactor.core.publisher.Flux; import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
@ -43,9 +44,6 @@ import org.springframework.data.elasticsearch.annotations.MultiField;
import org.springframework.data.elasticsearch.annotations.Query; import org.springframework.data.elasticsearch.annotations.Query;
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations; import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
import org.springframework.data.elasticsearch.core.SearchHit; import org.springframework.data.elasticsearch.core.SearchHit;
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter;
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter;
import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext;
import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.core.query.StringQuery;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.Repository; import org.springframework.data.repository.Repository;
@ -59,16 +57,15 @@ import org.springframework.lang.Nullable;
* @author Peter-Josef Meisch * @author Peter-Josef Meisch
*/ */
@ExtendWith(MockitoExtension.class) @ExtendWith(MockitoExtension.class)
public class ReactiveElasticsearchStringQueryUnitTests { public class ReactiveElasticsearchStringQueryUnitTests extends ElasticsearchStringQueryUnitTestBase {
SpelExpressionParser PARSER = new SpelExpressionParser(); SpelExpressionParser PARSER = new SpelExpressionParser();
ElasticsearchConverter converter;
@Mock ReactiveElasticsearchOperations operations; @Mock ReactiveElasticsearchOperations operations;
@BeforeEach @BeforeEach
public void setUp() { public void setUp() {
converter = new MappingElasticsearchConverter(new SimpleElasticsearchMappingContext()); when(operations.getElasticsearchConverter()).thenReturn(setupConverter());
} }
@Test // DATAES-519 @Test // DATAES-519
@ -132,7 +129,22 @@ public class ReactiveElasticsearchStringQueryUnitTests {
.isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}"); .isEqualTo("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"hello \\\"Stranger\\\"\"}}]}}");
} }
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, String... args) @Test // #1866
@DisplayName("should use converter on parameters")
void shouldUseConverterOnParameters() throws Exception {
Car car = new Car();
car.setName("Toyota");
car.setModel("Prius");
org.springframework.data.elasticsearch.core.query.Query query = createQuery("findByCar", car);
assertThat(query).isInstanceOf(StringQuery.class);
assertThat(((StringQuery) query).getSource())
.isEqualTo("{ 'bool' : { 'must' : { 'term' : { 'car' : 'Toyota-Prius' } } } }");
}
private org.springframework.data.elasticsearch.core.query.Query createQuery(String methodName, Object... args)
throws NoSuchMethodException { throws NoSuchMethodException {
Class<?>[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new); Class<?>[] argTypes = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new);
@ -152,7 +164,7 @@ public class ReactiveElasticsearchStringQueryUnitTests {
Method method = SampleRepository.class.getMethod(name, parameters); Method method = SampleRepository.class.getMethod(name, parameters);
return new ReactiveElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class), return new ReactiveElasticsearchQueryMethod(method, new DefaultRepositoryMetadata(SampleRepository.class),
new SpelAwareProxyProjectionFactory(), converter.getMappingContext()); new SpelAwareProxyProjectionFactory(), operations.getElasticsearchConverter().getMappingContext());
} }
private ReactiveElasticsearchStringQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception { private ReactiveElasticsearchStringQuery createQueryForMethod(String name, Class<?>... parameters) throws Exception {
@ -180,6 +192,9 @@ public class ReactiveElasticsearchStringQueryUnitTests {
@Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}") @Query("{\"bool\":{\"must\": [{\"match\": {\"prefix\": {\"name\" : \"?0\"}}]}}")
Flux<SearchHit<Book>> findByPrefix(String prefix); Flux<SearchHit<Book>> findByPrefix(String prefix);
@Query("{ 'bool' : { 'must' : { 'term' : { 'car' : '?0' } } } }")
Mono<Person> findByCar(Car car);
} }
/** /**
@ -292,29 +307,6 @@ public class ReactiveElasticsearchStringQueryUnitTests {
} }
} }
static class Car {
@Nullable private String name;
@Nullable private String model;
@Nullable
public String getName() {
return name;
}
public void setName(@Nullable String name) {
this.name = name;
}
@Nullable
public String getModel() {
return model;
}
public void setModel(@Nullable String model) {
this.model = model;
}
}
static class Author { static class Author {
@Nullable private String id; @Nullable private String id;