diff --git a/src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java b/src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java index be5fd5c82..b3863985a 100644 --- a/src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java +++ b/src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java @@ -94,6 +94,7 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; import org.elasticsearch.search.aggregations.Aggregation; import org.reactivestreams.Publisher; + import org.springframework.data.elasticsearch.client.ClientConfiguration; import org.springframework.data.elasticsearch.client.ClientLogger; import org.springframework.data.elasticsearch.client.ElasticsearchHost; @@ -112,7 +113,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; -import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.reactive.function.BodyExtractors; import org.springframework.web.reactive.function.client.ClientRequest; @@ -819,16 +819,17 @@ public class DefaultReactiveElasticsearchClient implements ReactiveElasticsearch .flatMap(content -> { String mediaType = response.headers().contentType().map(MediaType::toString) .orElse(XContentType.JSON.mediaType()); + RestStatus status = RestStatus.fromCode(response.statusCode().value()); try { ElasticsearchException exception = getElasticsearchException(response, content, mediaType); if (exception != null) { StringBuilder sb = new StringBuilder(); buildExceptionMessages(sb, exception); - return Mono.error(new HttpClientErrorException(response.statusCode(), sb.toString())); + return Mono.error(new ElasticsearchStatusException(sb.toString(), status, exception)); } } catch (Exception e) { return Mono - .error(new ElasticsearchStatusException(content, RestStatus.fromCode(response.statusCode().value()))); + .error(new ElasticsearchStatusException(content, status)); } return Mono.just(content); }).doOnNext(it -> ClientLogger.logResponse(logId, response.statusCode(), it)) // 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 f038294ce..c8c532ad6 100644 --- a/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java +++ b/src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.elasticsearch.core; import java.io.IOException; @@ -24,6 +23,7 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.common.ValidationException; import org.elasticsearch.index.engine.VersionConflictEngineException; import org.elasticsearch.rest.RestStatus; + import org.springframework.dao.DataAccessException; import org.springframework.dao.DataAccessResourceFailureException; import org.springframework.dao.DataIntegrityViolationException; @@ -34,12 +34,16 @@ import org.springframework.data.elasticsearch.UncategorizedElasticsearchExceptio import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; -import org.springframework.web.client.HttpClientErrorException; /** + * Simple {@link PersistenceExceptionTranslator} for Elasticsearch. Convert the given runtime exception to an + * appropriate exception from the {@code org.springframework.dao} hierarchy. Return {@literal null} if no translation is + * appropriate: any other exception may have resulted from user code, and should not be translated. + * * @author Christoph Strobl * @author Peter-Josef Meisch * @author Roman Puchkovskiy + * @author Mark Paluch * @since 3.2 */ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTranslator { @@ -47,6 +51,10 @@ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTra @Override public DataAccessException translateExceptionIfPossible(RuntimeException ex) { + if (isSeqNoConflict(ex)) { + return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", ex); + } + if (ex instanceof ElasticsearchException) { ElasticsearchException elasticsearchException = (ElasticsearchException) ex; @@ -56,23 +64,9 @@ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTra ex); } - if (isSeqNoConflict(elasticsearchException)) { - return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", - elasticsearchException); - } - return new UncategorizedElasticsearchException(ex.getMessage(), ex); } - if (ex instanceof HttpClientErrorException) { - HttpClientErrorException httpClientErrorException = (HttpClientErrorException) ex; - - if (isSeqNoConflict(httpClientErrorException)) { - return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", - httpClientErrorException); - } - } - if (ex instanceof ValidationException) { return new DataIntegrityViolationException(ex.getMessage(), ex); } @@ -88,25 +82,22 @@ public class ElasticsearchExceptionTranslator implements PersistenceExceptionTra private boolean isSeqNoConflict(Exception exception) { if (exception instanceof ElasticsearchStatusException) { + ElasticsearchStatusException statusException = (ElasticsearchStatusException) exception; + return statusException.status() == RestStatus.CONFLICT && statusException.getMessage() != null && statusException.getMessage().contains("type=version_conflict_engine_exception") && statusException.getMessage().contains("version conflict, required seqNo"); } if (exception instanceof VersionConflictEngineException) { + VersionConflictEngineException versionConflictEngineException = (VersionConflictEngineException) exception; + return versionConflictEngineException.getMessage() != null && versionConflictEngineException.getMessage().contains("version conflict, required seqNo"); } - if (exception instanceof HttpClientErrorException) { - HttpClientErrorException httpClientErrorException = (HttpClientErrorException) exception; - - return httpClientErrorException.getMessage() != null - && httpClientErrorException.getMessage().contains("version conflict, required seqNo"); - - } return false; } diff --git a/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientTests.java b/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientTests.java index db8c89083..8146ae47d 100644 --- a/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientTests.java @@ -18,7 +18,6 @@ package org.springframework.data.elasticsearch.client.reactive; import static org.assertj.core.api.Assertions.*; import lombok.SneakyThrows; -import org.springframework.web.client.HttpClientErrorException; import reactor.test.StepVerifier; import java.io.IOException; @@ -143,12 +142,12 @@ public class ReactiveElasticsearchClientTests { .verifyComplete(); } - @Test // DATAES-519 + @Test // DATAES-519, DATAES-822 public void getOnNonExistingIndexShouldThrowException() { client.get(new GetRequest(INDEX_I, "nonono")) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class) // + .expectError(ElasticsearchStatusException.class) // .verify(); } @@ -305,7 +304,7 @@ public class ReactiveElasticsearchClientTests { client.index(request) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-488 @@ -354,7 +353,7 @@ public class ReactiveElasticsearchClientTests { client.update(request) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-488 @@ -515,7 +514,7 @@ public class ReactiveElasticsearchClientTests { client.indices().createIndex(request -> request.index(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -535,7 +534,7 @@ public class ReactiveElasticsearchClientTests { client.indices().deleteIndex(request -> request.indices(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -553,7 +552,7 @@ public class ReactiveElasticsearchClientTests { client.indices().openIndex(request -> request.indices(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -571,7 +570,7 @@ public class ReactiveElasticsearchClientTests { client.indices().closeIndex(request -> request.indices(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -589,7 +588,7 @@ public class ReactiveElasticsearchClientTests { client.indices().refreshIndex(request -> request.indices(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -613,7 +612,7 @@ public class ReactiveElasticsearchClientTests { client.indices().updateMapping(request -> request.indices(INDEX_I).type(TYPE_I).source(jsonMap)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-569 @@ -631,7 +630,7 @@ public class ReactiveElasticsearchClientTests { client.indices().flushIndex(request -> request.indices(INDEX_I)) // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); + .verifyError(ElasticsearchStatusException.class); } @Test // DATAES-684 diff --git a/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientUnitTests.java b/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientUnitTests.java index 6a1b0fef5..4b0b4e79d 100644 --- a/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientUnitTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientUnitTests.java @@ -19,7 +19,6 @@ import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.*; import static org.springframework.data.elasticsearch.client.reactive.ReactiveMockClientTestsUtils.MockWebClientProvider.Receive.*; -import org.springframework.web.client.HttpClientErrorException; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -460,7 +459,7 @@ public class ReactiveElasticsearchClientUnitTests { .verifyComplete(); } - @Test // DATAES-488, DATAES-767 + @Test // DATAES-488, DATAES-767, DATAES-822 public void updateShouldEmitErrorWhenNotFound() { hostProvider.when(HOST) // @@ -468,7 +467,7 @@ public class ReactiveElasticsearchClientUnitTests { client.update(new UpdateRequest("twitter", "doc", "1").doc(Collections.singletonMap("user", "cstrobl"))) .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class) // + .expectError(ElasticsearchStatusException.class) // .verify(); } diff --git a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslatorTests.java b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslatorTests.java index 88a3367f3..a805502e2 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslatorTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslatorTests.java @@ -22,10 +22,9 @@ import org.elasticsearch.index.engine.VersionConflictEngineException; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.rest.RestStatus; import org.junit.jupiter.api.Test; + import org.springframework.dao.DataAccessException; import org.springframework.dao.OptimisticLockingFailureException; -import org.springframework.http.HttpStatus; -import org.springframework.web.client.HttpClientErrorException; /** * @author Roman Puchkovskiy @@ -60,15 +59,4 @@ class ElasticsearchExceptionTranslatorTests { assertThat(translated.getCause()).isSameAs(ex); } - @Test // DATAES-767 - void shouldConvertHttpClientErrorExceptionWithSeqNoConflictToOptimisticLockingFailureException() { - HttpClientErrorException ex = new HttpClientErrorException(HttpStatus.BAD_REQUEST, - "Elasticsearch exception [type=version_conflict_engine_exception, reason=[WPUUsXEB6uuA6j8_A7AB]: version conflict, required seqNo [34], primary term [16]. current document has seqNo [35] and primary term [16]]"); - - DataAccessException translated = translator.translateExceptionIfPossible(ex); - - assertThat(translated).isInstanceOf(OptimisticLockingFailureException.class); - assertThat(translated.getMessage()).startsWith("Cannot index a document due to seq_no+primary_term conflict"); - assertThat(translated.getCause()).isSameAs(ex); - } } 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 d765206e9..ecbad86c9 100644 --- a/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java @@ -25,7 +25,6 @@ import lombok.Builder; import lombok.Data; import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; -import org.springframework.web.client.HttpClientErrorException; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -41,6 +40,7 @@ import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.IntStream; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.index.query.IdsQueryBuilder; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringTerms; @@ -49,6 +49,7 @@ import org.elasticsearch.search.sort.SortOrder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import org.springframework.dao.DataAccessResourceFailureException; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.annotation.Id; @@ -214,12 +215,12 @@ public class ReactiveElasticsearchTemplateTests { }).isInstanceOf(IllegalArgumentException.class); } - @Test // DATAES-519, DATAES-767 + @Test // DATAES-519, DATAES-767, DATAES-822 public void getByIdShouldErrorWhenIndexDoesNotExist() { template.get("foo", SampleEntity.class, IndexCoordinates.of("no-such-index").withTypes("test-type")) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-504 @@ -334,7 +335,7 @@ public class ReactiveElasticsearchTemplateTests { .search(new CriteriaQuery(Criteria.where("message").is("some message")), SampleEntity.class, IndexCoordinates.of("no-such-index")) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-504 @@ -446,7 +447,7 @@ public class ReactiveElasticsearchTemplateTests { template.search(queryWithInvalidPreference, SampleEntity.class) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class).verify(); + .expectError(UncategorizedElasticsearchException.class).verify(); } @Test // DATAES-504 @@ -526,7 +527,7 @@ public class ReactiveElasticsearchTemplateTests { template.aggregate(new CriteriaQuery(Criteria.where("message").is("some message")), SampleEntity.class, IndexCoordinates.of("no-such-index")) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-519, DATAES-767 @@ -534,7 +535,7 @@ public class ReactiveElasticsearchTemplateTests { template.count(SampleEntity.class) // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-504 @@ -566,7 +567,7 @@ public class ReactiveElasticsearchTemplateTests { template.delete("does-not-exists", IndexCoordinates.of("no-such-index")) // .as(StepVerifier::create)// - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-504 diff --git a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java index dddf330cd..d0c517305 100644 --- a/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java +++ b/src/test/java/org/springframework/data/elasticsearch/repository/support/SimpleReactiveElasticsearchRepositoryTests.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.UUID; import java.util.stream.IntStream; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.action.bulk.BulkRequest; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.WriteRequest.RefreshPolicy; @@ -47,6 +48,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.reactivestreams.Publisher; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -57,6 +59,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; import org.springframework.data.elasticsearch.TestUtils; +import org.springframework.data.elasticsearch.UncategorizedElasticsearchException; import org.springframework.data.elasticsearch.annotations.Document; import org.springframework.data.elasticsearch.annotations.Field; import org.springframework.data.elasticsearch.annotations.Highlight; @@ -72,7 +75,6 @@ import org.springframework.data.elasticsearch.repository.config.EnableReactiveEl import org.springframework.data.repository.reactive.ReactiveCrudRepository; import org.springframework.test.context.ContextConfiguration; import org.springframework.util.StringUtils; -import org.springframework.web.client.HttpClientErrorException; /** * @author Christoph Strobl @@ -130,11 +132,11 @@ public class SimpleReactiveElasticsearchRepositoryTests { .verifyComplete(); } - @Test // DATAES-519, DATAES-767 + @Test // DATAES-519, DATAES-767, DATAES-822 public void findByIdShouldErrorIfIndexDoesNotExist() { repository.findById("id-two") // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-519 @@ -267,11 +269,11 @@ public class SimpleReactiveElasticsearchRepositoryTests { .verifyComplete(); } - @Test // DATAES-519, DATAES-767 + @Test // DATAES-519, DATAES-767, DATAES-822 public void countShouldErrorWhenIndexDoesNotExist() { repository.count() // .as(StepVerifier::create) // - .expectError(HttpClientErrorException.class); + .expectError(ElasticsearchStatusException.class); } @Test // DATAES-519 @@ -357,12 +359,11 @@ public class SimpleReactiveElasticsearchRepositoryTests { repository.deleteById("does-not-exist").as(StepVerifier::create).verifyComplete(); } - @Test // DATAES-519, DATAES-767 + @Test // DATAES-519, DATAES-767, DATAES-822 public void deleteByIdShouldErrorWhenIndexDoesNotExist() { repository.deleteById("does-not-exist") // .as(StepVerifier::create) // - .verifyError(HttpClientErrorException.class); - ; + .verifyError(UncategorizedElasticsearchException.class); } @Test // DATAES-519