From 16bf8450f07ef73ac4b346bca299a0834881a412 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 9 Apr 2019 13:58:52 +0200 Subject: [PATCH] DATAES-547 - Polishing. Add test and directly use SearchHit to pass on the index name. Fix minor flaw in Exception translation for non existing indices along the way. Original Pull Request: #257 --- .../data/elasticsearch/core/DeleteEntry.java | 31 ------- .../ElasticsearchExceptionTranslator.java | 16 +++- .../core/ElasticsearchRestTemplate.java | 26 ++---- .../core/ElasticsearchTemplate.java | 25 ++---- .../core/ElasticsearchTemplateTests.java | 88 +++++++++++++++++++ .../ReactiveElasticsearchTemplateTests.java | 58 ++++++++++++ 6 files changed, 177 insertions(+), 67 deletions(-) delete mode 100644 src/main/java/org/springframework/data/elasticsearch/core/DeleteEntry.java diff --git a/src/main/java/org/springframework/data/elasticsearch/core/DeleteEntry.java b/src/main/java/org/springframework/data/elasticsearch/core/DeleteEntry.java deleted file mode 100644 index 3cf7a568d..000000000 --- a/src/main/java/org/springframework/data/elasticsearch/core/DeleteEntry.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2019 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.core; - -import lombok.Value; - -/** - * DeleteEntry - * - * @author Lorenzo Spinelli - */ -@Value -public class DeleteEntry { - - private final String id; - private final String indexName; - -} diff --git a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java index 4ef1b2154..a453dda4b 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java @@ -17,13 +17,17 @@ package org.springframework.data.elasticsearch.core; import java.net.ConnectException; +import java.util.List; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.ElasticsearchStatusException; import org.springframework.dao.DataAccessException; import org.springframework.dao.DataAccessResourceFailureException; import org.springframework.dao.support.PersistenceExceptionTranslator; import org.springframework.data.elasticsearch.NoSuchIndexException; import org.springframework.util.CollectionUtils; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; /** * @author Christoph Strobl @@ -39,7 +43,8 @@ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTra ElasticsearchException elasticsearchException = (ElasticsearchException) ex; if (!indexAvailable(elasticsearchException)) { - return new NoSuchIndexException(elasticsearchException.getMetadata("es.index").toString(), ex); + return new NoSuchIndexException(ObjectUtils.nullSafeToString(elasticsearchException.getMetadata("es.index")), + ex); } } @@ -51,6 +56,13 @@ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTra } private boolean indexAvailable(ElasticsearchException ex) { - return !CollectionUtils.contains(ex.getMetadata("es.index_uuid").iterator(), "_na_"); + + List metadata = ex.getMetadata("es.index_uuid"); + if (metadata == null) { + if (ex instanceof ElasticsearchStatusException) { + return StringUtils.hasText(ObjectUtils.nullSafeToString(((ElasticsearchStatusException) ex).getIndex())); + } + } + return !CollectionUtils.contains(metadata.iterator(), "_na_"); } } 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 09a9661a4..97701d682 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java @@ -869,35 +869,27 @@ public class ElasticsearchRestTemplate SearchQuery searchQuery = new NativeSearchQueryBuilder().withQuery(deleteQuery.getQuery()).withIndices(indexName) .withTypes(typeName).withPageable(PageRequest.of(0, pageSize)).build(); - SearchResultMapper deleteentryResultMapper = new SearchResultMapperAdapter() { + SearchResultMapper deleteEntryResultMapper = new SearchResultMapperAdapter() { + @Override public AggregatedPage mapResults(SearchResponse response, Class clazz, Pageable pageable) { - List result = new ArrayList<>(); - for (SearchHit searchHit : response.getHits().getHits()) { - String id = searchHit.getId(); - String indexName = searchHit.getIndex(); - result.add(new DeleteEntry(id, indexName)); - } - if (result.size() > 0) { - return new AggregatedPageImpl<>((List) result, response.getScrollId()); - } - return new AggregatedPageImpl<>(Collections.emptyList(), response.getScrollId()); + return new AggregatedPageImpl<>((List) Arrays.asList(response.getHits().getHits()), response.getScrollId()); } }; - Page scrolledResult = startScroll(scrollTimeInMillis, searchQuery, DeleteEntry.class, - deleteentryResultMapper); + Page scrolledResult = startScroll(scrollTimeInMillis, searchQuery, SearchHit.class, + deleteEntryResultMapper); BulkRequest request = new BulkRequest(); - List documentsToDelete = new ArrayList<>(); + List documentsToDelete = new ArrayList<>(); do { documentsToDelete.addAll(scrolledResult.getContent()); scrolledResult = continueScroll(((ScrolledPage) scrolledResult).getScrollId(), scrollTimeInMillis, - DeleteEntry.class, deleteentryResultMapper); + SearchHit.class, deleteEntryResultMapper); } while (scrolledResult.getContent().size() != 0); - for (DeleteEntry entry : documentsToDelete) { - request.add(new DeleteRequest(entry.getIndexName(), typeName, entry.getId())); + for (SearchHit entry : documentsToDelete) { + request.add(new DeleteRequest(entry.getIndex(), typeName, entry.getId())); } if (request.numberOfActions() > 0) { 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 c309028b0..9dda4d255 100755 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java @@ -24,7 +24,7 @@ import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; @@ -757,35 +757,26 @@ public class ElasticsearchTemplate implements ElasticsearchOperations, EsClient< .withTypes(typeName).withPageable(PageRequest.of(0, pageSize)).build(); SearchResultMapper deleteEntryResultMapper = new SearchResultMapperAdapter() { + @Override public AggregatedPage mapResults(SearchResponse response, Class clazz, Pageable pageable) { - List result = new ArrayList<>(); - for (SearchHit searchHit : response.getHits().getHits()) { - - String id = searchHit.getId(); - String indexName = searchHit.getIndex(); - result.add(new DeleteEntry(id, indexName)); - } - if (result.size() > 0) { - return new AggregatedPageImpl((List) result, response.getScrollId()); - } - return new AggregatedPageImpl(Collections.emptyList(), response.getScrollId()); + return new AggregatedPageImpl<>((List) Arrays.asList(response.getHits().getHits()), response.getScrollId()); } }; - Page scrolledResult = startScroll(scrollTimeInMillis, searchQuery, DeleteEntry.class, + Page scrolledResult = startScroll(scrollTimeInMillis, searchQuery, SearchHit.class, deleteEntryResultMapper); BulkRequestBuilder bulkRequestBuilder = client.prepareBulk(); - List documentsToDelete = new ArrayList<>(); + List documentsToDelete = new ArrayList<>(); do { documentsToDelete.addAll(scrolledResult.getContent()); scrolledResult = continueScroll(((ScrolledPage) scrolledResult).getScrollId(), scrollTimeInMillis, - DeleteEntry.class, deleteEntryResultMapper); + SearchHit.class, deleteEntryResultMapper); } while (scrolledResult.getContent().size() != 0); - for (DeleteEntry entry : documentsToDelete) { - bulkRequestBuilder.add(client.prepareDelete(entry.getIndexName(), typeName, entry.getId())); + for (SearchHit entry : documentsToDelete) { + bulkRequestBuilder.add(client.prepareDelete(entry.getIndex(), typeName, entry.getId())); } if (bulkRequestBuilder.numberOfActions() > 0) { diff --git a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java index 10a655272..44cf67e5e 100755 --- a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplateTests.java @@ -66,12 +66,14 @@ import org.springframework.data.elasticsearch.entities.GTEVersionEntity; import org.springframework.data.elasticsearch.entities.HetroEntity1; import org.springframework.data.elasticsearch.entities.HetroEntity2; import org.springframework.data.elasticsearch.entities.SampleEntity; +import org.springframework.data.elasticsearch.entities.SampleEntityUUIDKeyed; import org.springframework.data.elasticsearch.entities.SampleMappingEntity; import org.springframework.data.elasticsearch.entities.UseServerConfigurationEntity; import org.springframework.data.util.CloseableIterator; /** * Base for testing rest/transport templates + * * @author Rizwan Idrees * @author Mohsin Husen * @author Franck Marchand @@ -101,9 +103,15 @@ public class ElasticsearchTemplateTests { @Before public void before() { + elasticsearchTemplate.deleteIndex(SampleEntity.class); elasticsearchTemplate.createIndex(SampleEntity.class); elasticsearchTemplate.putMapping(SampleEntity.class); + + elasticsearchTemplate.deleteIndex(SampleEntityUUIDKeyed.class); + elasticsearchTemplate.createIndex(SampleEntityUUIDKeyed.class); + elasticsearchTemplate.putMapping(SampleEntityUUIDKeyed.class); + elasticsearchTemplate.deleteIndex(INDEX_1_NAME); elasticsearchTemplate.deleteIndex(INDEX_2_NAME); elasticsearchTemplate.deleteIndex(UseServerConfigurationEntity.class); @@ -405,6 +413,86 @@ public class ElasticsearchTemplateTests { assertThat(sampleEntities.getTotalElements(), equalTo(0L)); } + @Test // DATAES-547 + public void shouldDeleteAcrossIndex() { + + // given + SampleEntity sampleEntity = SampleEntity.builder() // + .message("foo") // + .version(System.currentTimeMillis()) // + .build(); + + IndexQuery idxQuery1 = new IndexQueryBuilder().withIndexName(INDEX_1_NAME).withId(randomNumeric(5)) + .withObject(sampleEntity).build(); + + elasticsearchTemplate.index(idxQuery1); + elasticsearchTemplate.refresh(INDEX_1_NAME); + + IndexQuery idxQuery2 = new IndexQueryBuilder().withIndexName(INDEX_2_NAME).withId(randomNumeric(5)) + .withObject(sampleEntity).build(); + + elasticsearchTemplate.index(idxQuery2); + elasticsearchTemplate.refresh(INDEX_2_NAME); + + // when + DeleteQuery deleteQuery = new DeleteQuery(); + deleteQuery.setQuery(termQuery("message", "foo")); + deleteQuery.setType("test-type"); + deleteQuery.setIndex("test-index-*"); + + elasticsearchTemplate.delete(deleteQuery); + + elasticsearchTemplate.refresh(INDEX_1_NAME); + elasticsearchTemplate.refresh(INDEX_2_NAME); + + // then + SearchQuery searchQuery = new NativeSearchQueryBuilder().withQuery(termQuery("message", "foo")) + .withIndices(INDEX_1_NAME, INDEX_2_NAME) // + .build(); + + assertThat(elasticsearchTemplate.count(searchQuery), equalTo(0L)); + } + + @Test // DATAES-547 + public void shouldDeleteAcrossIndexWhenNoMatchingDataPresent() { + + // given + SampleEntity sampleEntity = SampleEntity.builder() // + .message("positive") // + .version(System.currentTimeMillis()) // + .build(); + + IndexQuery idxQuery1 = new IndexQueryBuilder().withIndexName(INDEX_1_NAME).withId(randomNumeric(5)) + .withObject(sampleEntity).build(); + + elasticsearchTemplate.index(idxQuery1); + elasticsearchTemplate.refresh(INDEX_1_NAME); + + IndexQuery idxQuery2 = new IndexQueryBuilder().withIndexName(INDEX_2_NAME).withId(randomNumeric(5)) + .withObject(sampleEntity).build(); + + elasticsearchTemplate.index(idxQuery2); + elasticsearchTemplate.refresh(INDEX_2_NAME); + + // when + DeleteQuery deleteQuery = new DeleteQuery(); + deleteQuery.setQuery(termQuery("message", "negative")); + deleteQuery.setType("test-type"); + deleteQuery.setIndex("test-index-*"); + + elasticsearchTemplate.delete(deleteQuery); + + elasticsearchTemplate.refresh(INDEX_1_NAME); + elasticsearchTemplate.refresh(INDEX_2_NAME); + + // then + SearchQuery searchQuery = new NativeSearchQueryBuilder().withQuery(termQuery("message", "positive")) + .withIndices(INDEX_1_NAME, INDEX_2_NAME) // + .build(); + + assertThat(elasticsearchTemplate.count(searchQuery), equalTo(2L)); + } + @Test public void shouldFilterSearchResultsForGivenFilter() { // given diff --git a/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java b/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java index 3843bc938..8d2a19cb1 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java @@ -52,6 +52,8 @@ import org.springframework.data.elasticsearch.core.query.Criteria; import org.springframework.data.elasticsearch.core.query.CriteriaQuery; import org.springframework.data.elasticsearch.core.query.IndexQuery; import org.springframework.data.elasticsearch.core.query.IndexQueryBuilder; +import org.springframework.data.elasticsearch.core.query.NativeSearchQueryBuilder; +import org.springframework.data.elasticsearch.core.query.SearchQuery; import org.springframework.data.elasticsearch.core.query.StringQuery; import org.springframework.data.elasticsearch.entities.SampleEntity; import org.springframework.test.context.ContextConfiguration; @@ -519,6 +521,62 @@ public class ReactiveElasticsearchTemplateTests { .verifyComplete(); } + @Test // DATAES-547 + @ElasticsearchVersion(asOf = "6.5.0") + public void shouldDeleteAcrossIndex() { + + String indexPrefix = "rx-template-test-index"; + String thisIndex = indexPrefix + "-this"; + String thatIndex = indexPrefix + "-that"; + + template.save(randomEntity("test"), thisIndex) // + .then(template.save(randomEntity("test"), thatIndex)) // + .then() // + .as(StepVerifier::create)// + .verifyComplete(); + + restTemplate.refresh(thisIndex); + restTemplate.refresh(thatIndex); + + SearchQuery searchQuery = new NativeSearchQueryBuilder() // + .withQuery(termQuery("message", "test")) // + .withIndices(indexPrefix + "*") // + .build(); + + template.deleteBy(searchQuery, SampleEntity.class) // + .as(StepVerifier::create) // + .expectNext(2L) // + .verifyComplete(); + } + + @Test // DATAES-547 + @ElasticsearchVersion(asOf = "6.5.0") + public void shouldDeleteAcrossIndexWhenNoMatchingDataPresent() { + + String indexPrefix = "rx-template-test-index"; + String thisIndex = indexPrefix + "-this"; + String thatIndex = indexPrefix + "-that"; + + template.save(randomEntity("positive"), thisIndex) // + .then(template.save(randomEntity("positive"), thatIndex)) // + .then() // + .as(StepVerifier::create)// + .verifyComplete(); + + restTemplate.refresh(thisIndex); + restTemplate.refresh(thatIndex); + + SearchQuery searchQuery = new NativeSearchQueryBuilder() // + .withQuery(termQuery("message", "negative")) // + .withIndices(indexPrefix + "*") // + .build(); + + template.deleteBy(searchQuery, SampleEntity.class) // + .as(StepVerifier::create) // + .expectNext(0L) // + .verifyComplete(); + } + @Test // DATAES-504 @ElasticsearchVersion(asOf = "6.5.0") public void deleteByQueryShouldReturnNumberOfDeletedDocuments() {