From 3782c8e738ef46088c16116a0f73aaa71fb511da Mon Sep 17 00:00:00 2001 From: Peter-Josef Meisch Date: Mon, 29 Jun 2020 22:00:47 +0200 Subject: [PATCH] DATAES-875 - MappingElasticsearchConverter.updateQuery not called at all places. Original PR: #489 --- .../core/ElasticsearchRestTemplate.java | 2 +- .../core/ElasticsearchTemplate.java | 2 +- .../core/ReactiveElasticsearchTemplate.java | 7 +--- .../elasticsearch/core/RequestFactory.java | 42 ++++++------------- .../core/convert/ElasticsearchConverter.java | 26 ++++++++++-- .../MappingElasticsearchConverter.java | 2 +- 6 files changed, 40 insertions(+), 41 deletions(-) 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 cc82a7eb..094d0363 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java @@ -171,7 +171,7 @@ public class ElasticsearchRestTemplate extends AbstractElasticsearchTemplate { Assert.notNull(index, "index must not be null"); Assert.notEmpty(query.getIds(), "No Id define for Query"); - MultiGetRequest request = requestFactory.multiGetRequest(query, index); + MultiGetRequest request = requestFactory.multiGetRequest(query, clazz, index); MultiGetResponse result = execute(client -> client.mget(request, RequestOptions.DEFAULT)); DocumentCallback callback = new ReadDocumentCallback<>(elasticsearchConverter, clazz, index); 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 0892a1fa..07b79481 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java @@ -186,7 +186,7 @@ public class ElasticsearchTemplate extends AbstractElasticsearchTemplate { Assert.notNull(index, "index must not be null"); Assert.notEmpty(query.getIds(), "No Ids defined for Query"); - MultiGetRequestBuilder builder = requestFactory.multiGetRequestBuilder(client, query, index); + MultiGetRequestBuilder builder = requestFactory.multiGetRequestBuilder(client, query, clazz, index); DocumentCallback callback = new ReadDocumentCallback<>(elasticsearchConverter, clazz, index); List documents = DocumentAdapters.from(builder.execute().actionGet()); diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplate.java b/src/main/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplate.java index 0b4de71c..b1b06ff0 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplate.java @@ -69,7 +69,6 @@ import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersiste import org.springframework.data.elasticsearch.core.mapping.IndexCoordinates; import org.springframework.data.elasticsearch.core.mapping.SimpleElasticsearchMappingContext; import org.springframework.data.elasticsearch.core.query.BulkOptions; -import org.springframework.data.elasticsearch.core.query.CriteriaQuery; import org.springframework.data.elasticsearch.core.query.IndexQuery; import org.springframework.data.elasticsearch.core.query.Query; import org.springframework.data.elasticsearch.core.query.SeqNoPrimaryTerm; @@ -267,7 +266,7 @@ public class ReactiveElasticsearchTemplate implements ReactiveElasticsearchOpera DocumentCallback callback = new ReadDocumentCallback<>(converter, clazz, index); - MultiGetRequest request = requestFactory.multiGetRequest(query, index); + MultiGetRequest request = requestFactory.multiGetRequest(query, clazz, index); return Flux.from(execute(client -> client.multiGet(request))) // .concatMap(result -> callback.doWith(DocumentAdapters.from(result))); } @@ -617,10 +616,6 @@ public class ReactiveElasticsearchTemplate implements ReactiveElasticsearchOpera private Flux doFind(Query query, Class clazz, IndexCoordinates index) { - if (query instanceof CriteriaQuery) { - converter.updateQuery((CriteriaQuery) query, clazz); - } - return Flux.defer(() -> { SearchRequest request = requestFactory.searchRequest(query, clazz, index); request = prepareSearchRequest(request); diff --git a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java index 944a6071..a8761b74 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/RequestFactory.java @@ -21,7 +21,6 @@ import static org.springframework.util.CollectionUtils.*; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -60,7 +59,6 @@ import org.elasticsearch.client.indices.GetIndexRequest; import org.elasticsearch.client.indices.GetMappingsRequest; import org.elasticsearch.client.indices.PutMappingRequest; import org.elasticsearch.cluster.metadata.AliasMetadata; -import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.geo.GeoDistance; import org.elasticsearch.common.settings.Settings; @@ -192,12 +190,7 @@ class RequestFactory { Query filterQuery = parameters.getFilterQuery(); if (filterQuery != null) { - - if (filterQuery instanceof CriteriaQuery && parameters.getFilterQueryClass() != null) { - CriteriaQuery query = (CriteriaQuery) filterQuery; - elasticsearchConverter.updateQuery(query, parameters.getFilterQueryClass()); - } - + elasticsearchConverter.updateQuery(filterQuery, parameters.getFilterQueryClass()); QueryBuilder queryBuilder = getFilter(filterQuery); if (queryBuilder == null) { @@ -343,7 +336,6 @@ class RequestFactory { * @param settings optional settings * @return request */ - @SuppressWarnings("unchecked") public CreateIndexRequest createIndexRequest(IndexCoordinates index, @Nullable Document settings) { CreateIndexRequest request = new CreateIndexRequest(index.getIndexName()); @@ -481,21 +473,6 @@ class RequestFactory { return new org.elasticsearch.action.admin.indices.mapping.get.GetMappingsRequest().indices(indexNames); } - public Map> convertAliasesResponse( - ImmutableOpenMap> aliasesResponse) { - - Map> mapped = new LinkedHashMap<>(); - Iterator keysIt = aliasesResponse.keysIt(); - while (keysIt.hasNext()) { - String key = keysIt.next(); - - List aliasMetaData = aliasesResponse.get(key); - mapped.put(key, new LinkedHashSet<>(aliasMetaData)); - } - - return convertAliasesResponse(mapped); - } - public Map> convertAliasesResponse(Map> aliasesResponse) { Map> converted = new LinkedHashMap<>(); aliasesResponse.forEach((index, aliasMetaDataSet) -> { @@ -622,19 +599,24 @@ class RequestFactory { return client.prepareGet(index.getIndexName(), null, id); } - public MultiGetRequest multiGetRequest(Query query, IndexCoordinates index) { + public MultiGetRequest multiGetRequest(Query query, Class clazz, IndexCoordinates index) { + MultiGetRequest multiGetRequest = new MultiGetRequest(); - getMultiRequestItems(query, index).forEach(multiGetRequest::add); + getMultiRequestItems(query, clazz, index).forEach(multiGetRequest::add); return multiGetRequest; } - public MultiGetRequestBuilder multiGetRequestBuilder(Client client, Query searchQuery, IndexCoordinates index) { + public MultiGetRequestBuilder multiGetRequestBuilder(Client client, Query searchQuery, Class clazz, + IndexCoordinates index) { + MultiGetRequestBuilder multiGetRequestBuilder = client.prepareMultiGet(); - getMultiRequestItems(searchQuery, index).forEach(multiGetRequestBuilder::add); + getMultiRequestItems(searchQuery, clazz, index).forEach(multiGetRequestBuilder::add); return multiGetRequestBuilder; } - private List getMultiRequestItems(Query searchQuery, IndexCoordinates index) { + private List getMultiRequestItems(Query searchQuery, Class clazz, IndexCoordinates index) { + + elasticsearchConverter.updateQuery(searchQuery, clazz); List items = new ArrayList<>(); if (!isEmpty(searchQuery.getFields())) { @@ -830,6 +812,7 @@ class RequestFactory { public SearchRequest searchRequest(Query query, @Nullable Class clazz, IndexCoordinates index) { + elasticsearchConverter.updateQuery(query, clazz); SearchRequest searchRequest = prepareSearchRequest(query, clazz, index); QueryBuilder elasticsearchQuery = getQuery(query); QueryBuilder elasticsearchFilter = getFilter(query); @@ -851,6 +834,7 @@ class RequestFactory { public SearchRequestBuilder searchRequestBuilder(Client client, Query query, @Nullable Class clazz, IndexCoordinates index) { + elasticsearchConverter.updateQuery(query, clazz); SearchRequestBuilder searchRequestBuilder = prepareSearchRequestBuilder(query, client, clazz, index); QueryBuilder elasticsearchQuery = getQuery(query); QueryBuilder elasticsearchFilter = getFilter(query); diff --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java index 8b34b191..1b1e6d10 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/ElasticsearchConverter.java @@ -20,6 +20,7 @@ import org.springframework.data.elasticsearch.core.document.Document; import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentEntity; import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentProperty; import org.springframework.data.elasticsearch.core.query.CriteriaQuery; +import org.springframework.data.elasticsearch.core.query.Query; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.lang.Nullable; @@ -83,15 +84,34 @@ public interface ElasticsearchConverter } // endregion + // region query /** * Updates a query by renaming the property names in the query to the correct mapped field names and the values to the * converted values if the {@link ElasticsearchPersistentProperty} for a property has a - * {@link org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter}. + * {@link org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter}. If + * domainClass is null, it's a noop; handling null here eliminates null checks in the caller. * + * @param query the query that is internally updated + * @param domainClass the class of the object that is searched with the query + */ + default void updateQuery(Query query, @Nullable Class domainClass) { + + if (domainClass != null) { + + if (query instanceof CriteriaQuery) { + updateCriteriaQuery((CriteriaQuery) query, domainClass); + } + } + } + + /** + * Updates a {@link CriteriaQuery} by renaming the property names in the query to the correct mapped field names and + * the values to the converted values if the {@link ElasticsearchPersistentProperty} for a property has a + * {@link org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentPropertyConverter}. + * * @param criteriaQuery the query that is internally updated * @param domainClass the class of the object that is searched with the query */ - // region query - void updateQuery(CriteriaQuery criteriaQuery, Class domainClass); + void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class domainClass); // endregion } diff --git a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java index 21092c01..15c53ee8 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/convert/MappingElasticsearchConverter.java @@ -748,7 +748,7 @@ public class MappingElasticsearchConverter // region queries @Override - public void updateQuery(CriteriaQuery criteriaQuery, Class domainClass) { + public void updateCriteriaQuery(CriteriaQuery criteriaQuery, Class domainClass) { ElasticsearchPersistentEntity persistentEntity = mappingContext.getPersistentEntity(domainClass); if (persistentEntity != null) {