From 9e93dd08aaaaac3a124d049023cacbcc54d22b2f Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Tue, 30 Jul 2019 12:40:31 +0200 Subject: [PATCH] DATES-615 - Use annotated field name on repository order by clause. Original PR: #298 --- .../core/ElasticsearchRestTemplate.java | 38 ++++++--- .../core/ElasticsearchTemplate.java | 32 +++++--- .../query/ElasticsearchPartQuery.java | 42 +++++----- .../parser/ElasticsearchQueryCreator.java | 13 ++- .../ElasticsearchTestConfiguration.java | 61 ++++++++++++++ .../RestElasticsearchTestConfiguration.java | 50 ++++++++++++ .../data/elasticsearch/TestNodeResource.java | 53 ++++++++++++ .../data/elasticsearch/Utils.java | 27 ++++--- .../QueryKeywordsRepositoryTests.java | 43 ++++++++++ .../QueryKeywordsRestRepositoryTests.java | 38 +++++++++ .../query/keywords/QueryKeywordsTests.java | 80 +++++++++++++++---- .../resources/repository-query-keywords.xml | 19 ----- 12 files changed, 404 insertions(+), 92 deletions(-) create mode 100644 src/test/java/org/springframework/data/elasticsearch/ElasticsearchTestConfiguration.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/RestElasticsearchTestConfiguration.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/TestNodeResource.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/repository/query/keywords/QueryKeywordsRepositoryTests.java create mode 100644 src/test/java/org/springframework/data/elasticsearch/repository/query/keywords/QueryKeywordsRestRepositoryTests.java delete mode 100644 src/test/resources/repository-query-keywords.xml diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java index aa981eda3..dcc2cee4e 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java @@ -449,7 +449,7 @@ public class ElasticsearchRestTemplate @Override public T query(SearchQuery query, ResultsExtractor resultsExtractor) { - SearchResponse response = doSearch(prepareSearch(query, Optional.ofNullable(query.getQuery())), query); + SearchResponse response = doSearch(prepareSearch(query, Optional.ofNullable(query.getQuery()), null), query); return resultsExtractor.extract(response); } @@ -470,7 +470,7 @@ public class ElasticsearchRestTemplate @Override public List queryForIds(SearchQuery query) { - SearchRequest request = prepareSearch(query, Optional.ofNullable(query.getQuery())); + SearchRequest request = prepareSearch(query, Optional.ofNullable(query.getQuery()), null); request.source().query(query.getQuery()); if (query.getFilter() != null) { request.source().postFilter(query.getFilter()); @@ -627,10 +627,10 @@ public class ElasticsearchRestTemplate } private SearchRequest prepareCount(Query query, Class clazz) { - String indexName[] = !isEmpty(query.getIndices()) + String[] indexName = !isEmpty(query.getIndices()) ? query.getIndices().toArray(new String[query.getIndices().size()]) : retrieveIndexNameFromPersistentEntity(clazz); - String types[] = !isEmpty(query.getTypes()) ? query.getTypes().toArray(new String[query.getTypes().size()]) + String[] types = !isEmpty(query.getTypes()) ? query.getTypes().toArray(new String[query.getTypes().size()]) : retrieveTypeFromPersistentEntity(clazz); Assert.notNull(indexName, "No index defined for Query"); @@ -920,10 +920,12 @@ public class ElasticsearchRestTemplate private SearchRequest prepareScroll(Query query, long scrollTimeInMillis, Class clazz) { setPersistentEntityIndexAndType(query, clazz); - return prepareScroll(query, scrollTimeInMillis); + ElasticsearchPersistentEntity entity = getPersistentEntity(clazz); + return prepareScroll(query, scrollTimeInMillis, entity); } - private SearchRequest prepareScroll(Query query, long scrollTimeInMillis) { + private SearchRequest prepareScroll(Query query, long scrollTimeInMillis, + @Nullable ElasticsearchPersistentEntity entity) { SearchRequest request = new SearchRequest(toArray(query.getIndices())); SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); request.types(toArray(query.getTypes())); @@ -943,7 +945,7 @@ public class ElasticsearchRestTemplate } if (query.getSort() != null) { - prepareSort(query, searchSourceBuilder); + prepareSort(query, searchSourceBuilder, entity); } request.source(searchSourceBuilder); @@ -1272,15 +1274,15 @@ public class ElasticsearchRestTemplate private SearchRequest prepareSearch(Query query, Class clazz) { setPersistentEntityIndexAndType(query, clazz); - return prepareSearch(query, Optional.empty()); + return prepareSearch(query, Optional.empty(), clazz); } private SearchRequest prepareSearch(SearchQuery query, Class clazz) { setPersistentEntityIndexAndType(query, clazz); - return prepareSearch(query, Optional.ofNullable(query.getQuery())); + return prepareSearch(query, Optional.ofNullable(query.getQuery()), clazz); } - private SearchRequest prepareSearch(Query query, Optional builder) { + private SearchRequest prepareSearch(Query query, Optional builder, @Nullable Class clazz) { Assert.notNull(query.getIndices(), "No index defined for Query"); Assert.notNull(query.getTypes(), "No type defined for Query"); @@ -1315,7 +1317,7 @@ public class ElasticsearchRestTemplate } if (query.getSort() != null) { - prepareSort(query, sourceBuilder); + prepareSort(query, sourceBuilder, getPersistentEntity(clazz)); } if (query.getMinScore() > 0) { @@ -1330,9 +1332,14 @@ public class ElasticsearchRestTemplate return request; } - private void prepareSort(Query query, SearchSourceBuilder sourceBuilder) { + private void prepareSort(Query query, SearchSourceBuilder sourceBuilder, + @Nullable ElasticsearchPersistentEntity entity) { for (Sort.Order order : query.getSort()) { - FieldSortBuilder sort = SortBuilders.fieldSort(order.getProperty()) + ElasticsearchPersistentProperty property = entity != null // + ? entity.getPersistentProperty(order.getProperty()) // + : null; + String fieldName = property != null ? property.getFieldName() : order.getProperty(); + FieldSortBuilder sort = SortBuilders.fieldSort(fieldName) .order(order.getDirection().isDescending() ? SortOrder.DESC : SortOrder.ASC); if (order.getNullHandling() == Sort.NullHandling.NULLS_FIRST) { sort.missing("_first"); @@ -1455,6 +1462,11 @@ public class ElasticsearchRestTemplate } } + @Nullable + private ElasticsearchPersistentEntity getPersistentEntity(@Nullable Class clazz) { + return clazz != null ? elasticsearchConverter.getMappingContext().getPersistentEntity(clazz) : null; + } + @Override public ElasticsearchPersistentEntity getPersistentEntityFor(Class clazz) { Assert.isTrue(clazz.isAnnotationPresent(Document.class), "Unable to identify index name. " + clazz.getSimpleName() diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java index ea551b1a4..7ec417717 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java @@ -389,7 +389,7 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< @Override public T query(SearchQuery query, ResultsExtractor resultsExtractor) { - SearchResponse response = doSearch(prepareSearch(query), query); + SearchResponse response = doSearch(prepareSearch(query, (ElasticsearchPersistentEntity) null), query); return resultsExtractor.extract(response); } @@ -410,7 +410,8 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< @Override public List queryForIds(SearchQuery query) { - SearchRequestBuilder request = prepareSearch(query).setQuery(query.getQuery()); + SearchRequestBuilder request = prepareSearch(query, (ElasticsearchPersistentEntity) null) + .setQuery(query.getQuery()); if (query.getFilter() != null) { request.setPostFilter(query.getFilter()); } @@ -781,10 +782,11 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< private SearchRequestBuilder prepareScroll(Query query, long scrollTimeInMillis, Class clazz) { setPersistentEntityIndexAndType(query, clazz); - return prepareScroll(query, scrollTimeInMillis); + return prepareScroll(query, scrollTimeInMillis, getPersistentEntity(clazz)); } - private SearchRequestBuilder prepareScroll(Query query, long scrollTimeInMillis) { + private SearchRequestBuilder prepareScroll(Query query, long scrollTimeInMillis, + @Nullable ElasticsearchPersistentEntity entity) { SearchRequestBuilder requestBuilder = client.prepareSearch(toArray(query.getIndices())) .setTypes(toArray(query.getTypes())).setScroll(TimeValue.timeValueMillis(scrollTimeInMillis)).setFrom(0) .setVersion(true); @@ -803,7 +805,7 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< } if (query.getSort() != null) { - prepareSort(query, requestBuilder); + prepareSort(query, requestBuilder, entity); } return requestBuilder; @@ -1070,10 +1072,10 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< private SearchRequestBuilder prepareSearch(Query query, Class clazz) { setPersistentEntityIndexAndType(query, clazz); - return prepareSearch(query); + return prepareSearch(query, getPersistentEntity(clazz)); } - private SearchRequestBuilder prepareSearch(Query query) { + private SearchRequestBuilder prepareSearch(Query query, @Nullable ElasticsearchPersistentEntity entity) { Assert.notNull(query.getIndices(), "No index defined for Query"); Assert.notNull(query.getTypes(), "No type defined for Query"); @@ -1102,7 +1104,7 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< } if (query.getSort() != null) { - prepareSort(query, searchRequestBuilder); + prepareSort(query, searchRequestBuilder, entity); } if (query.getMinScore() > 0) { @@ -1116,7 +1118,8 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< return searchRequestBuilder; } - private void prepareSort(Query query, SearchRequestBuilder searchRequestBuilder) { + private void prepareSort(Query query, SearchRequestBuilder searchRequestBuilder, + @Nullable ElasticsearchPersistentEntity entity) { for (Sort.Order order : query.getSort()) { SortOrder sortOrder = order.getDirection().isDescending() ? SortOrder.DESC : SortOrder.ASC; @@ -1127,8 +1130,12 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< searchRequestBuilder.addSort(sort); } else { + ElasticsearchPersistentProperty property = entity != null // + ? entity.getPersistentProperty(order.getProperty()) // + : null; + String fieldName = property != null ? property.getFieldName() : order.getProperty(); FieldSortBuilder sort = SortBuilders // - .fieldSort(order.getProperty()) // + .fieldSort(fieldName) // .order(sortOrder); if (order.getNullHandling() == Sort.NullHandling.NULLS_FIRST) { @@ -1203,6 +1210,11 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< .get(indexName); } + @Nullable + private ElasticsearchPersistentEntity getPersistentEntity(@Nullable Class clazz) { + return clazz != null ? elasticsearchConverter.getMappingContext().getPersistentEntity(clazz) : null; + } + @Override public ElasticsearchPersistentEntity getPersistentEntityFor(Class clazz) { Assert.isTrue(clazz.isAnnotationPresent(Document.class), "Unable to identify index name. " + clazz.getSimpleName() diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchPartQuery.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchPartQuery.java index 0e90250da..331748be3 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchPartQuery.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/ElasticsearchPartQuery.java @@ -25,6 +25,7 @@ import org.springframework.data.repository.query.ParametersParameterAccessor; import org.springframework.data.repository.query.parser.PartTree; import org.springframework.data.util.CloseableIterator; import org.springframework.data.util.StreamUtils; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -35,6 +36,7 @@ import org.springframework.util.ClassUtils; * @author Kevin Leturc * @author Mark Paluch * @author Rasmus Faber-Espensen + * @author Peter-Josef Meisch */ public class ElasticsearchPartQuery extends AbstractElasticsearchRepositoryQuery { @@ -54,45 +56,38 @@ public class ElasticsearchPartQuery extends AbstractElasticsearchRepositoryQuery ParametersParameterAccessor accessor = new ParametersParameterAccessor(queryMethod.getParameters(), parameters); CriteriaQuery query = createQuery(accessor); - if (tree.isDelete()) { + Assert.notNull(query, "unsupported query"); + if (tree.isDelete()) { Object result = countOrGetDocumentsForDelete(query, accessor); elasticsearchOperations.delete(query, queryMethod.getEntityInformation().getJavaType()); return result; } else if (queryMethod.isPageQuery()) { - query.setPageable(accessor.getPageable()); return elasticsearchOperations.queryForPage(query, queryMethod.getEntityInformation().getJavaType()); } else if (queryMethod.isStreamQuery()) { - Class entityType = queryMethod.getEntityInformation().getJavaType(); if (accessor.getPageable().isUnpaged()) { - query.setPageable(PageRequest.of(0, DEFAULT_STREAM_BATCH_SIZE)); } else { + query.setPageable(accessor.getPageable()); + } + return StreamUtils + .createStreamFromIterator((CloseableIterator) elasticsearchOperations.stream(query, entityType)); + } else if (queryMethod.isCollectionQuery()) { + if (accessor.getPageable().isUnpaged()) { + + int itemCount = (int) elasticsearchOperations.count(query, queryMethod.getEntityInformation().getJavaType()); + query.setPageable(PageRequest.of(0, Math.max(1, itemCount))); + } else { query.setPageable(accessor.getPageable()); } - return StreamUtils - .createStreamFromIterator((CloseableIterator) elasticsearchOperations.stream(query, entityType)); - - } else if (queryMethod.isCollectionQuery()) { - - if (accessor.getPageable().isUnpaged()) { - - int itemCount = (int) elasticsearchOperations.count(query, queryMethod.getEntityInformation().getJavaType()); - query.setPageable(PageRequest.of(0, Math.max(1, itemCount))); - } else { - - query.setPageable(accessor.getPageable()); - } - - return elasticsearchOperations.queryForList(query, queryMethod.getEntityInformation().getJavaType()); - } else if (tree.isCountProjection()) { - - return elasticsearchOperations.count(query, queryMethod.getEntityInformation().getJavaType()); - } + return elasticsearchOperations.queryForList(query, queryMethod.getEntityInformation().getJavaType()); + } else if (tree.isCountProjection()) { + return elasticsearchOperations.count(query, queryMethod.getEntityInformation().getJavaType()); + } return elasticsearchOperations.queryForObject(query, queryMethod.getEntityInformation().getJavaType()); } @@ -102,6 +97,7 @@ public class ElasticsearchPartQuery extends AbstractElasticsearchRepositoryQuery Object result = null; if (queryMethod.isCollectionQuery()) { + if (accessor.getPageable().isUnpaged()) { int itemCount = (int) elasticsearchOperations.count(query, queryMethod.getEntityInformation().getJavaType()); query.setPageable(PageRequest.of(0, Math.max(1, itemCount))); diff --git a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java index d0ef3f02a..0f823bf4b 100644 --- a/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java +++ b/src/main/java/org/springframework/data/elasticsearch/repository/query/parser/ElasticsearchQueryCreator.java @@ -34,6 +34,7 @@ import org.springframework.data.repository.query.ParameterAccessor; import org.springframework.data.repository.query.parser.AbstractQueryCreator; import org.springframework.data.repository.query.parser.Part; import org.springframework.data.repository.query.parser.PartTree; +import org.springframework.lang.Nullable; /** * ElasticsearchQueryCreator @@ -42,13 +43,14 @@ import org.springframework.data.repository.query.parser.PartTree; * @author Mohsin Husen * @author Franck Marchand * @author Artur Konczak + * @author Peter-Josef Meisch */ public class ElasticsearchQueryCreator extends AbstractQueryCreator { private final MappingContext context; public ElasticsearchQueryCreator(PartTree tree, ParameterAccessor parameters, - MappingContext context) { + MappingContext context) { super(tree, parameters); this.context = context; } @@ -83,9 +85,12 @@ public class ElasticsearchQueryCreator extends AbstractQueryCreator sortedIds = repository.findAllByNameOrderByText("Salt").stream() // + .map(it -> it.id).collect(Collectors.toList()); + + assertThat(sortedIds).containsExactly("4", "5"); + } + + @Test // DATAES-615 + public void shouldSupportSortOnFieldWithCustomFieldNameWithCriteria() { + + List sortedIds = repository.findAllByNameOrderBySortName("Sugar").stream() // + .map(it -> it.id).collect(Collectors.toList()); + + assertThat(sortedIds).containsExactly("3", "2", "1"); + } + + @Test // DATAES-615 + public void shouldSupportSortOnStandardFieldWithoutCriteria() { + List sortedIds = repository.findAllByOrderByText().stream() // + .map(it -> it.text).collect(Collectors.toList()); + + assertThat(sortedIds).containsExactly("Beet sugar", "Cane sugar", "Cane sugar", "Rock salt", "Sea salt"); + } + + @Test // DATAES-615 + public void shouldSupportSortOnFieldWithCustomFieldNameWithoutCriteria() { + + List sortedIds = repository.findAllByOrderBySortName().stream() // + .map(it -> it.id).collect(Collectors.toList()); + + assertThat(sortedIds).containsExactly("5", "4", "3", "2", "1"); + } + /** * @author Mohsin Husen * @author Artur Konczak @@ -176,7 +218,7 @@ public class QueryKeywordsTests { private String description; - private String text; + @Field(type = FieldType.Keyword) private String text; private List categories; @@ -191,12 +233,14 @@ public class QueryKeywordsTests { private String location; private Date lastModified; + + @Field(name = "sort-name", type = FieldType.Keyword) private String sortName; } /** * Created by akonczak on 04/09/15. */ - interface ProductRepository extends PagingAndSortingRepository { + interface ProductRepository extends ElasticsearchRepository { List findByNameAndText(String name, String text); @@ -227,6 +271,14 @@ public class QueryKeywordsTests { List findByPriceGreaterThanEqual(float v); List findByIdNotIn(List strings); + + List findAllByNameOrderByText(String name); + + List findAllByNameOrderBySortName(String name); + + List findAllByOrderByText(); + + List findAllByOrderBySortName(); } } diff --git a/src/test/resources/repository-query-keywords.xml b/src/test/resources/repository-query-keywords.xml deleted file mode 100644 index b4419ad2f..000000000 --- a/src/test/resources/repository-query-keywords.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - - - - -