Skip to content

Commit cea8c93

Browse files
mp911demp911de
committed
DATAES-822 - Convert Reactive Client exceptions to ElasticsearchStatusException.
We now use ElasticsearchStatusException instead of HttpClientErrorException to simplify exception translation so that ElasticsearchExceptionTranslator does no longer depend on spring-web.
1 parent 5100fe0 commit cea8c93

File tree

7 files changed

+50
-70
lines changed

7 files changed

+50
-70
lines changed

‎src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
import org.elasticsearch.search.SearchHits;
9595
import org.elasticsearch.search.aggregations.Aggregation;
9696
import org.reactivestreams.Publisher;
97+
9798
import org.springframework.data.elasticsearch.client.ClientConfiguration;
9899
import org.springframework.data.elasticsearch.client.ClientLogger;
99100
import org.springframework.data.elasticsearch.client.ElasticsearchHost;
@@ -112,7 +113,6 @@
112113
import org.springframework.util.Assert;
113114
import org.springframework.util.ObjectUtils;
114115
import org.springframework.util.ReflectionUtils;
115-
import org.springframework.web.client.HttpClientErrorException;
116116
import org.springframework.web.client.HttpServerErrorException;
117117
import org.springframework.web.reactive.function.BodyExtractors;
118118
import org.springframework.web.reactive.function.client.ClientRequest;
@@ -819,16 +819,17 @@ private <T> Publisher<? extends T> handleClientError(String logId, Request reque
819819
.flatMap(content -> {
820820
String mediaType = response.headers().contentType().map(MediaType::toString)
821821
.orElse(XContentType.JSON.mediaType());
822+
RestStatus status = RestStatus.fromCode(response.statusCode().value());
822823
try {
823824
ElasticsearchException exception = getElasticsearchException(response, content, mediaType);
824825
if (exception != null) {
825826
StringBuilder sb = new StringBuilder();
826827
buildExceptionMessages(sb, exception);
827-
return Mono.error(new HttpClientErrorException(response.statusCode(), sb.toString()));
828+
return Mono.error(new ElasticsearchStatusException(sb.toString(), status, exception));
828829
}
829830
} catch (Exception e) {
830831
return Mono
831-
.error(new ElasticsearchStatusException(content, RestStatus.fromCode(response.statusCode().value())));
832+
.error(new ElasticsearchStatusException(content, status));
832833
}
833834
return Mono.just(content);
834835
}).doOnNext(it -> ClientLogger.logResponse(logId, response.statusCode(), it)) //

‎src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslator.java

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
1716
package org.springframework.data.elasticsearch.core;
1817

1918
import java.io.IOException;
@@ -24,6 +23,7 @@
2423
import org.elasticsearch.common.ValidationException;
2524
import org.elasticsearch.index.engine.VersionConflictEngineException;
2625
import org.elasticsearch.rest.RestStatus;
26+
2727
import org.springframework.dao.DataAccessException;
2828
import org.springframework.dao.DataAccessResourceFailureException;
2929
import org.springframework.dao.DataIntegrityViolationException;
@@ -34,19 +34,27 @@
3434
import org.springframework.util.CollectionUtils;
3535
import org.springframework.util.ObjectUtils;
3636
import org.springframework.util.StringUtils;
37-
import org.springframework.web.client.HttpClientErrorException;
3837

3938
/**
39+
* Simple {@link PersistenceExceptionTranslator} for Elasticsearch. Convert the given runtime exception to an
40+
* appropriate exception from the {@code org.springframework.dao} hierarchy. Return {@literal null} if no translation is
41+
* appropriate: any other exception may have resulted from user code, and should not be translated.
42+
*
4043
* @author Christoph Strobl
4144
* @author Peter-Josef Meisch
4245
* @author Roman Puchkovskiy
46+
* @author Mark Paluch
4347
* @since 3.2
4448
*/
4549
public class ElasticsearchExceptionTranslator implements PersistenceExceptionTranslator {
4650

4751
@Override
4852
public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
4953

54+
if (isSeqNoConflict(ex)) {
55+
return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict", ex);
56+
}
57+
5058
if (ex instanceof ElasticsearchException) {
5159

5260
ElasticsearchException elasticsearchException = (ElasticsearchException) ex;
@@ -56,23 +64,9 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
5664
ex);
5765
}
5866

59-
if (isSeqNoConflict(elasticsearchException)) {
60-
return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict",
61-
elasticsearchException);
62-
}
63-
6467
return new UncategorizedElasticsearchException(ex.getMessage(), ex);
6568
}
6669

67-
if (ex instanceof HttpClientErrorException) {
68-
HttpClientErrorException httpClientErrorException = (HttpClientErrorException) ex;
69-
70-
if (isSeqNoConflict(httpClientErrorException)) {
71-
return new OptimisticLockingFailureException("Cannot index a document due to seq_no+primary_term conflict",
72-
httpClientErrorException);
73-
}
74-
}
75-
7670
if (ex instanceof ValidationException) {
7771
return new DataIntegrityViolationException(ex.getMessage(), ex);
7872
}
@@ -88,25 +82,22 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
8882
private boolean isSeqNoConflict(Exception exception) {
8983

9084
if (exception instanceof ElasticsearchStatusException) {
85+
9186
ElasticsearchStatusException statusException = (ElasticsearchStatusException) exception;
87+
9288
return statusException.status() == RestStatus.CONFLICT && statusException.getMessage() != null
9389
&& statusException.getMessage().contains("type=version_conflict_engine_exception")
9490
&& statusException.getMessage().contains("version conflict, required seqNo");
9591
}
9692

9793
if (exception instanceof VersionConflictEngineException) {
94+
9895
VersionConflictEngineException versionConflictEngineException = (VersionConflictEngineException) exception;
96+
9997
return versionConflictEngineException.getMessage() != null
10098
&& versionConflictEngineException.getMessage().contains("version conflict, required seqNo");
10199
}
102100

103-
if (exception instanceof HttpClientErrorException) {
104-
HttpClientErrorException httpClientErrorException = (HttpClientErrorException) exception;
105-
106-
return httpClientErrorException.getMessage() != null
107-
&& httpClientErrorException.getMessage().contains("version conflict, required seqNo");
108-
109-
}
110101
return false;
111102
}
112103

‎src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientTests.java

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static org.assertj.core.api.Assertions.*;
1919

2020
import lombok.SneakyThrows;
21-
import org.springframework.web.client.HttpClientErrorException;
2221
import reactor.test.StepVerifier;
2322

2423
import java.io.IOException;
@@ -143,12 +142,12 @@ public void infoShouldReturnClusterInformation() {
143142
.verifyComplete();
144143
}
145144

146-
@Test // DATAES-519
145+
@Test // DATAES-519, DATAES-822
147146
public void getOnNonExistingIndexShouldThrowException() {
148147

149148
client.get(new GetRequest(INDEX_I, "nonono")) //
150149
.as(StepVerifier::create) //
151-
.expectError(HttpClientErrorException.class) //
150+
.expectError(ElasticsearchStatusException.class) //
152151
.verify();
153152
}
154153

@@ -305,7 +304,7 @@ public void indexShouldErrorForExistingDocuments() {
305304

306305
client.index(request) //
307306
.as(StepVerifier::create) //
308-
.verifyError(HttpClientErrorException.class);
307+
.verifyError(ElasticsearchStatusException.class);
309308
}
310309

311310
@Test // DATAES-488
@@ -354,7 +353,7 @@ public void updateShouldErrorNonExistingDocumentWhenNotUpserted() {
354353

355354
client.update(request) //
356355
.as(StepVerifier::create) //
357-
.verifyError(HttpClientErrorException.class);
356+
.verifyError(ElasticsearchStatusException.class);
358357
}
359358

360359
@Test // DATAES-488
@@ -515,7 +514,7 @@ public void createExistingIndexErrors() throws IOException {
515514

516515
client.indices().createIndex(request -> request.index(INDEX_I)) //
517516
.as(StepVerifier::create) //
518-
.verifyError(HttpClientErrorException.class);
517+
.verifyError(ElasticsearchStatusException.class);
519518
}
520519

521520
@Test // DATAES-569
@@ -535,7 +534,7 @@ public void deleteNonExistingIndexErrors() {
535534

536535
client.indices().deleteIndex(request -> request.indices(INDEX_I)) //
537536
.as(StepVerifier::create) //
538-
.verifyError(HttpClientErrorException.class);
537+
.verifyError(ElasticsearchStatusException.class);
539538
}
540539

541540
@Test // DATAES-569
@@ -553,7 +552,7 @@ public void openNonExistingIndex() {
553552

554553
client.indices().openIndex(request -> request.indices(INDEX_I)) //
555554
.as(StepVerifier::create) //
556-
.verifyError(HttpClientErrorException.class);
555+
.verifyError(ElasticsearchStatusException.class);
557556
}
558557

559558
@Test // DATAES-569
@@ -571,7 +570,7 @@ public void closeNonExistingIndex() {
571570

572571
client.indices().closeIndex(request -> request.indices(INDEX_I)) //
573572
.as(StepVerifier::create) //
574-
.verifyError(HttpClientErrorException.class);
573+
.verifyError(ElasticsearchStatusException.class);
575574
}
576575

577576
@Test // DATAES-569
@@ -589,7 +588,7 @@ public void refreshNonExistingIndex() {
589588

590589
client.indices().refreshIndex(request -> request.indices(INDEX_I)) //
591590
.as(StepVerifier::create) //
592-
.verifyError(HttpClientErrorException.class);
591+
.verifyError(ElasticsearchStatusException.class);
593592
}
594593

595594
@Test // DATAES-569
@@ -613,7 +612,7 @@ public void updateMappingNonExistingIndex() {
613612

614613
client.indices().updateMapping(request -> request.indices(INDEX_I).type(TYPE_I).source(jsonMap)) //
615614
.as(StepVerifier::create) //
616-
.verifyError(HttpClientErrorException.class);
615+
.verifyError(ElasticsearchStatusException.class);
617616
}
618617

619618
@Test // DATAES-569
@@ -631,7 +630,7 @@ public void flushNonExistingIndex() {
631630

632631
client.indices().flushIndex(request -> request.indices(INDEX_I)) //
633632
.as(StepVerifier::create) //
634-
.verifyError(HttpClientErrorException.class);
633+
.verifyError(ElasticsearchStatusException.class);
635634
}
636635

637636
@Test // DATAES-684

‎src/test/java/org/springframework/data/elasticsearch/client/reactive/ReactiveElasticsearchClientUnitTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static org.mockito.Mockito.*;
2020
import static org.springframework.data.elasticsearch.client.reactive.ReactiveMockClientTestsUtils.MockWebClientProvider.Receive.*;
2121

22-
import org.springframework.web.client.HttpClientErrorException;
2322
import reactor.core.publisher.Mono;
2423
import reactor.test.StepVerifier;
2524

@@ -460,15 +459,15 @@ public void updateShouldEmitResponseCorrectly() {
460459
.verifyComplete();
461460
}
462461

463-
@Test // DATAES-488, DATAES-767
462+
@Test // DATAES-488, DATAES-767, DATAES-822
464463
public void updateShouldEmitErrorWhenNotFound() {
465464

466465
hostProvider.when(HOST) //
467466
.updateFail();
468467

469468
client.update(new UpdateRequest("twitter", "doc", "1").doc(Collections.singletonMap("user", "cstrobl")))
470469
.as(StepVerifier::create) //
471-
.expectError(HttpClientErrorException.class) //
470+
.expectError(ElasticsearchStatusException.class) //
472471
.verify();
473472
}
474473

‎src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchExceptionTranslatorTests.java

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,9 @@
2222
import org.elasticsearch.index.shard.ShardId;
2323
import org.elasticsearch.rest.RestStatus;
2424
import org.junit.jupiter.api.Test;
25+
2526
import org.springframework.dao.DataAccessException;
2627
import org.springframework.dao.OptimisticLockingFailureException;
27-
import org.springframework.http.HttpStatus;
28-
import org.springframework.web.client.HttpClientErrorException;
2928

3029
/**
3130
* @author Roman Puchkovskiy
@@ -60,15 +59,4 @@ void shouldConvertVersionConflictEngineExceptionWithSeqNoConflictToOptimisticLoc
6059
assertThat(translated.getCause()).isSameAs(ex);
6160
}
6261

63-
@Test // DATAES-767
64-
void shouldConvertHttpClientErrorExceptionWithSeqNoConflictToOptimisticLockingFailureException() {
65-
HttpClientErrorException ex = new HttpClientErrorException(HttpStatus.BAD_REQUEST,
66-
"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]]");
67-
68-
DataAccessException translated = translator.translateExceptionIfPossible(ex);
69-
70-
assertThat(translated).isInstanceOf(OptimisticLockingFailureException.class);
71-
assertThat(translated.getMessage()).startsWith("Cannot index a document due to seq_no+primary_term conflict");
72-
assertThat(translated.getCause()).isSameAs(ex);
73-
}
7462
}

‎src/test/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplateTests.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import lombok.Data;
2626
import lombok.EqualsAndHashCode;
2727
import lombok.NoArgsConstructor;
28-
import org.springframework.web.client.HttpClientErrorException;
2928
import reactor.core.publisher.Mono;
3029
import reactor.test.StepVerifier;
3130

@@ -41,6 +40,7 @@
4140
import java.util.stream.Collectors;
4241
import java.util.stream.IntStream;
4342

43+
import org.elasticsearch.ElasticsearchStatusException;
4444
import org.elasticsearch.index.query.IdsQueryBuilder;
4545
import org.elasticsearch.search.aggregations.AggregationBuilders;
4646
import org.elasticsearch.search.aggregations.bucket.terms.ParsedStringTerms;
@@ -49,6 +49,7 @@
4949
import org.junit.jupiter.api.AfterEach;
5050
import org.junit.jupiter.api.BeforeEach;
5151
import org.junit.jupiter.api.Test;
52+
5253
import org.springframework.dao.DataAccessResourceFailureException;
5354
import org.springframework.dao.OptimisticLockingFailureException;
5455
import org.springframework.data.annotation.Id;
@@ -214,12 +215,12 @@ public void insertShouldErrorOnNullEntity() {
214215
}).isInstanceOf(IllegalArgumentException.class);
215216
}
216217

217-
@Test // DATAES-519, DATAES-767
218+
@Test // DATAES-519, DATAES-767, DATAES-822
218219
public void getByIdShouldErrorWhenIndexDoesNotExist() {
219220

220221
template.get("foo", SampleEntity.class, IndexCoordinates.of("no-such-index").withTypes("test-type")) //
221222
.as(StepVerifier::create) //
222-
.expectError(HttpClientErrorException.class);
223+
.expectError(ElasticsearchStatusException.class);
223224
}
224225

225226
@Test // DATAES-504
@@ -334,7 +335,7 @@ public void searchShouldCompleteWhenIndexDoesNotExist() {
334335
.search(new CriteriaQuery(Criteria.where("message").is("some message")), SampleEntity.class,
335336
IndexCoordinates.of("no-such-index")) //
336337
.as(StepVerifier::create) //
337-
.expectError(HttpClientErrorException.class);
338+
.expectError(ElasticsearchStatusException.class);
338339
}
339340

340341
@Test // DATAES-504
@@ -446,7 +447,7 @@ public void shouldThrowElasticsearchStatusExceptionWhenInvalidPreferenceForGiven
446447

447448
template.search(queryWithInvalidPreference, SampleEntity.class) //
448449
.as(StepVerifier::create) //
449-
.expectError(HttpClientErrorException.class).verify();
450+
.expectError(UncategorizedElasticsearchException.class).verify();
450451
}
451452

452453
@Test // DATAES-504
@@ -526,15 +527,15 @@ public void aggregateShouldErrorWhenIndexDoesNotExist() {
526527
template.aggregate(new CriteriaQuery(Criteria.where("message").is("some message")), SampleEntity.class,
527528
IndexCoordinates.of("no-such-index")) //
528529
.as(StepVerifier::create) //
529-
.expectError(HttpClientErrorException.class);
530+
.expectError(ElasticsearchStatusException.class);
530531
}
531532

532533
@Test // DATAES-519, DATAES-767
533534
public void countShouldReturnZeroWhenIndexDoesNotExist() {
534535

535536
template.count(SampleEntity.class) //
536537
.as(StepVerifier::create) //
537-
.expectError(HttpClientErrorException.class);
538+
.expectError(ElasticsearchStatusException.class);
538539
}
539540

540541
@Test // DATAES-504
@@ -566,7 +567,7 @@ public void deleteShouldErrorWhenIndexDoesNotExist() {
566567

567568
template.delete("does-not-exists", IndexCoordinates.of("no-such-index")) //
568569
.as(StepVerifier::create)//
569-
.expectError(HttpClientErrorException.class);
570+
.expectError(ElasticsearchStatusException.class);
570571
}
571572

572573
@Test // DATAES-504

0 commit comments

Comments
 (0)