Skip to content

Commit 4a8e012

Browse files
sothawosothawo
authored
Don't expose ElasticsearchStatusException.
Original Pull Request #1959 Closes #1957
1 parent 2450d57 commit 4a8e012

10 files changed

+133
-51
lines changed

‎src/main/asciidoc/reference/elasticsearch-migration-guide-4.2-4.3.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ Currently this will be a `org
4444
.springframework.data.elasticsearch.core.clients.elasticsearch7.ElasticsearchAggregations` object; later different implementations will be available.
4545
The same change has been done to the `ReactiveSearchOperations.aggregate()` functions, the now return a `Flux<AggregationContainer<?>>`.
4646
Programs using the aggregations need to be changed to cast the returned value to the appropriate class to further proces it.
47+
* methods that might have thrown a `org.elasticsearch.ElasticsearchStatusException` now will throw `org.springframework.data.elasticsearch.RestStatusException` instead.
4748

4849
=== Handling of field and sourceFilter properties of Query
4950

‎src/main/java/org/springframework/data/elasticsearch/RestStatusException.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright 2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.elasticsearch;
17+
18+
import org.springframework.dao.DataAccessException;
19+
20+
/**
21+
* Exception class for REST status exceptions independent from the used client/backend.
22+
*
23+
* @author Peter-Josef Meisch
24+
* @since 4.3
25+
*/
26+
public class RestStatusException extends DataAccessException {
27+
28+
// we do not use a dedicated status class from Elasticsearch, OpenSearch, Spring web or webflux here
29+
private final int status;
30+
31+
public RestStatusException(int status, String msg) {
32+
super(msg);
33+
this.status = status;
34+
}
35+
36+
public RestStatusException(int status, String msg, Throwable cause) {
37+
super(msg, cause);
38+
this.status = status;
39+
}
40+
41+
public int getStatus() {
42+
return status;
43+
}
44+
45+
@Override
46+
public String toString() {
47+
return "RestStatusException{" + "status=" + status + "} " + super.toString();
48+
}
49+
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
import org.elasticsearch.search.aggregations.Aggregation;
102102
import org.elasticsearch.search.suggest.Suggest;
103103
import org.reactivestreams.Publisher;
104+
import org.springframework.data.elasticsearch.RestStatusException;
104105
import org.springframework.data.elasticsearch.UncategorizedElasticsearchException;
105106
import org.springframework.data.elasticsearch.client.ClientConfiguration;
106107
import org.springframework.data.elasticsearch.client.ClientLogger;
@@ -836,8 +837,7 @@ private static <T> Mono<T> doDecode(ClientResponse response, Class<T> responseTy
836837
return Mono.error(BytesRestResponse.errorFromXContent(createParser(mediaType, content)));
837838
} catch (Exception e) {
838839

839-
return Mono
840-
.error(new ElasticsearchStatusException(content, RestStatus.fromCode(response.statusCode().value())));
840+
return Mono.error(new RestStatusException(response.statusCode().value(), content));
841841
}
842842
}
843843
}
@@ -870,14 +870,14 @@ private <T> Publisher<? extends T> handleServerError(Request request, ClientResp
870870
String mediaType = response.headers().contentType().map(MediaType::toString).orElse(XContentType.JSON.mediaType());
871871

872872
return response.body(BodyExtractors.toMono(byte[].class)) //
873-
.switchIfEmpty(Mono.error(
874-
new ElasticsearchStatusException(String.format("%s request to %s returned error code %s and no body.",
875-
request.getMethod(), request.getEndpoint(), statusCode), status)))
873+
.switchIfEmpty(Mono.error(new RestStatusException(status.getStatus(),
874+
String.format("%s request to %s returned error code %s and no body.", request.getMethod(),
875+
request.getEndpoint(), statusCode))))
876876
.map(bytes -> new String(bytes, StandardCharsets.UTF_8)) //
877877
.flatMap(content -> contentOrError(content, mediaType, status))
878878
.flatMap(unused -> Mono
879-
.error(new ElasticsearchStatusException(String.format("%s request to %s returned error code %s.",
880-
request.getMethod(), request.getEndpoint(), statusCode), status)));
879+
.error(new RestStatusException(status.getStatus(), String.format("%s request to %s returned error code %s.",
880+
request.getMethod(), request.getEndpoint(), statusCode))));
881881
}
882882

883883
private <T> Publisher<? extends T> handleClientError(String logId, ClientResponse response, Class<T> responseType) {
@@ -909,7 +909,7 @@ private static Mono<String> contentOrError(String content, String mediaType, Res
909909
if (exception != null) {
910910
StringBuilder sb = new StringBuilder();
911911
buildExceptionMessages(sb, exception);
912-
return Mono.error(new ElasticsearchStatusException(sb.toString(), status, exception));
912+
return Mono.error(new RestStatusException(status.getStatus(), sb.toString(), exception));
913913
}
914914

915915
return Mono.just(content);

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

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@
2222
import org.elasticsearch.ElasticsearchStatusException;
2323
import org.elasticsearch.common.ValidationException;
2424
import org.elasticsearch.index.engine.VersionConflictEngineException;
25-
import org.elasticsearch.rest.RestStatus;
2625
import org.springframework.dao.DataAccessException;
2726
import org.springframework.dao.DataAccessResourceFailureException;
2827
import org.springframework.dao.DataIntegrityViolationException;
2928
import org.springframework.dao.OptimisticLockingFailureException;
3029
import org.springframework.dao.support.PersistenceExceptionTranslator;
3130
import org.springframework.data.elasticsearch.NoSuchIndexException;
31+
import org.springframework.data.elasticsearch.RestStatusException;
3232
import org.springframework.data.elasticsearch.UncategorizedElasticsearchException;
3333
import org.springframework.util.CollectionUtils;
3434
import org.springframework.util.ObjectUtils;
@@ -38,7 +38,7 @@
3838
* Simple {@link PersistenceExceptionTranslator} for Elasticsearch. Convert the given runtime exception to an
3939
* appropriate exception from the {@code org.springframework.dao} hierarchy. Return {@literal null} if no translation is
4040
* appropriate: any other exception may have resulted from user code, and should not be translated.
41-
*
41+
*
4242
* @author Christoph Strobl
4343
* @author Peter-Josef Meisch
4444
* @author Roman Puchkovskiy
@@ -63,9 +63,28 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
6363
ex);
6464
}
6565

66+
if (elasticsearchException instanceof ElasticsearchStatusException) {
67+
ElasticsearchStatusException restStatusException = (ElasticsearchStatusException) elasticsearchException;
68+
return new RestStatusException(restStatusException.status().getStatus(), restStatusException.getMessage(),
69+
restStatusException.getCause());
70+
}
71+
6672
return new UncategorizedElasticsearchException(ex.getMessage(), ex);
6773
}
6874

75+
if (ex instanceof RestStatusException) {
76+
RestStatusException restStatusException = (RestStatusException) ex;
77+
Throwable cause = restStatusException.getCause();
78+
if (cause instanceof ElasticsearchException) {
79+
ElasticsearchException elasticsearchException = (ElasticsearchException) cause;
80+
81+
if (!indexAvailable(elasticsearchException)) {
82+
return new NoSuchIndexException(ObjectUtils.nullSafeToString(elasticsearchException.getMetadata("es.index")),
83+
ex);
84+
}
85+
}
86+
}
87+
6988
if (ex instanceof ValidationException) {
7089
return new DataIntegrityViolationException(ex.getMessage(), ex);
7190
}
@@ -80,13 +99,26 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
8099

81100
private boolean isSeqNoConflict(Exception exception) {
82101

102+
Integer status = null;
103+
String message = null;
104+
83105
if (exception instanceof ElasticsearchStatusException) {
84106

85107
ElasticsearchStatusException statusException = (ElasticsearchStatusException) exception;
108+
status = statusException.status().getStatus();
109+
message = statusException.getMessage();
110+
}
111+
112+
if (exception instanceof RestStatusException) {
113+
114+
RestStatusException statusException = (RestStatusException) exception;
115+
status = statusException.getStatus();
116+
message = statusException.getMessage();
117+
}
86118

87-
return statusException.status() == RestStatus.CONFLICT && statusException.getMessage() != null
88-
&& statusException.getMessage().contains("type=version_conflict_engine_exception")
89-
&& statusException.getMessage().contains("version conflict, required seqNo");
119+
if (status != null && message != null) {
120+
return status == 409 && message.contains("type=version_conflict_engine_exception")
121+
&& message.contains("version conflict, required seqNo");
90122
}
91123

92124
if (exception instanceof VersionConflictEngineException) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.Optional;
2727
import java.util.function.Function;
2828

29-
import org.elasticsearch.ElasticsearchStatusException;
3029
import org.elasticsearch.action.get.GetRequest;
3130
import org.elasticsearch.action.search.SearchRequest;
3231
import org.elasticsearch.client.Request;
@@ -40,6 +39,7 @@
4039
import org.mockito.Mock;
4140
import org.mockito.Spy;
4241
import org.mockito.junit.jupiter.MockitoExtension;
42+
import org.springframework.data.elasticsearch.RestStatusException;
4343
import org.springframework.http.HttpStatus;
4444
import org.springframework.web.reactive.function.client.ClientResponse;
4545
import org.springframework.web.reactive.function.client.WebClient;
@@ -84,8 +84,8 @@ public Mono<ResponseSpec> execute(ReactiveElasticsearchClientCallback callback)
8484
}
8585

8686
@Test // #1712
87-
@DisplayName("should throw ElasticsearchStatusException on server 5xx with empty body")
88-
void shouldThrowElasticsearchStatusExceptionOnServer5xxWithEmptyBody() {
87+
@DisplayName("should throw RestStatusException on server 5xx with empty body")
88+
void shouldThrowRestStatusExceptionOnServer5xxWithEmptyBody() {
8989

9090
when(hostProvider.getActive(any())).thenReturn(Mono.just(webClient));
9191
WebClient.RequestBodyUriSpec requestBodyUriSpec = mock(WebClient.RequestBodyUriSpec.class);
@@ -108,7 +108,7 @@ void shouldThrowElasticsearchStatusExceptionOnServer5xxWithEmptyBody() {
108108

109109
client.get(new GetRequest("42")) //
110110
.as(StepVerifier::create) //
111-
.expectError(ElasticsearchStatusException.class) //
111+
.expectError(RestStatusException.class) //
112112
.verify(); //
113113
}
114114
}

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.UUID;
3131
import java.util.stream.IntStream;
3232

33-
import org.elasticsearch.ElasticsearchStatusException;
3433
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
3534
import org.elasticsearch.action.bulk.BulkRequest;
3635
import org.elasticsearch.action.delete.DeleteRequest;
@@ -63,6 +62,7 @@
6362
import org.springframework.beans.factory.annotation.Autowired;
6463
import org.springframework.context.annotation.Bean;
6564
import org.springframework.context.annotation.Configuration;
65+
import org.springframework.data.elasticsearch.RestStatusException;
6666
import org.springframework.data.elasticsearch.client.ClientConfiguration;
6767
import org.springframework.data.elasticsearch.core.ReactiveElasticsearchOperations;
6868
import org.springframework.data.elasticsearch.core.ReactiveIndexOperations;
@@ -165,7 +165,7 @@ public void getOnNonExistingIndexShouldThrowException() {
165165

166166
client.get(new GetRequest(INDEX_I, "nonono")) //
167167
.as(StepVerifier::create) //
168-
.expectError(ElasticsearchStatusException.class) //
168+
.expectError(RestStatusException.class) //
169169
.verify();
170170
}
171171

@@ -356,7 +356,7 @@ public void indexShouldErrorForExistingDocuments() {
356356

357357
client.index(request) //
358358
.as(StepVerifier::create) //
359-
.verifyError(ElasticsearchStatusException.class);
359+
.verifyError(RestStatusException.class);
360360
}
361361

362362
@Test // DATAES-488
@@ -405,7 +405,7 @@ public void updateShouldErrorNonExistingDocumentWhenNotUpserted() {
405405

406406
client.update(request) //
407407
.as(StepVerifier::create) //
408-
.verifyError(ElasticsearchStatusException.class);
408+
.verifyError(RestStatusException.class);
409409
}
410410

411411
@Test // DATAES-488
@@ -715,7 +715,7 @@ public void createExistingIndexErrors() {
715715

716716
client.indices().createIndex(request -> request.index(INDEX_I)) //
717717
.as(StepVerifier::create) //
718-
.verifyError(ElasticsearchStatusException.class);
718+
.verifyError(RestStatusException.class);
719719
}
720720

721721
@Test // #1658
@@ -737,7 +737,7 @@ public void createExistingIndexErrors_() {
737737

738738
client.indices().createIndex(new CreateIndexRequest(INDEX_I)) //
739739
.as(StepVerifier::create) //
740-
.verifyError(ElasticsearchStatusException.class);
740+
.verifyError(RestStatusException.class);
741741
}
742742

743743
@Test // #1658
@@ -757,7 +757,7 @@ public void getIndexError() {
757757
operations.indexOps(IndexCoordinates.of(INDEX_I)).create().block();
758758

759759
client.indices().getIndex(new GetIndexRequest(INDEX_II)).as(StepVerifier::create)
760-
.verifyError(ElasticsearchStatusException.class);
760+
.verifyError(RestStatusException.class);
761761
}
762762

763763
@Test // DATAES-569, DATAES-678
@@ -782,7 +782,7 @@ public void deleteNonExistingIndexErrors() {
782782

783783
client.indices().deleteIndex(request -> request.indices(INDEX_I)) //
784784
.as(StepVerifier::create) //
785-
.verifyError(ElasticsearchStatusException.class);
785+
.verifyError(RestStatusException.class);
786786
}
787787

788788
@Test // DATAES-569
@@ -800,7 +800,7 @@ public void openNonExistingIndex() {
800800

801801
client.indices().openIndex(request -> request.indices(INDEX_I)) //
802802
.as(StepVerifier::create) //
803-
.verifyError(ElasticsearchStatusException.class);
803+
.verifyError(RestStatusException.class);
804804
}
805805

806806
@Test // DATAES-569
@@ -818,7 +818,7 @@ public void closeNonExistingIndex() {
818818

819819
client.indices().closeIndex(request -> request.indices(INDEX_I)) //
820820
.as(StepVerifier::create) //
821-
.verifyError(ElasticsearchStatusException.class);
821+
.verifyError(RestStatusException.class);
822822
}
823823

824824
@Test // DATAES-569
@@ -836,7 +836,7 @@ public void refreshNonExistingIndex() {
836836

837837
client.indices().refreshIndex(request -> request.indices(INDEX_I)) //
838838
.as(StepVerifier::create) //
839-
.verifyError(ElasticsearchStatusException.class);
839+
.verifyError(RestStatusException.class);
840840
}
841841

842842
@Test // #1640
@@ -869,7 +869,7 @@ void putMappingError() {
869869

870870
client.indices().putMapping(putMappingRequest) //
871871
.as(StepVerifier::create) //
872-
.verifyError(ElasticsearchStatusException.class);
872+
.verifyError(RestStatusException.class);
873873
}
874874

875875
@Test // #1640
@@ -911,7 +911,7 @@ void getMappingError() {
911911

912912
client.indices().getMapping(getMappingsRequest) //
913913
.as(StepVerifier::create) //
914-
.verifyError(ElasticsearchStatusException.class);
914+
.verifyError(RestStatusException.class);
915915
}
916916

917917
@Test // DATAES-569
@@ -936,7 +936,7 @@ public void updateMappingNonExistingIndex() {
936936

937937
client.indices().putMapping(request -> request.indices(INDEX_I).source(jsonMap)) //
938938
.as(StepVerifier::create) //
939-
.verifyError(ElasticsearchStatusException.class);
939+
.verifyError(RestStatusException.class);
940940
}
941941

942942
@Test // DATAES-569
@@ -954,7 +954,7 @@ public void flushNonExistingIndex() {
954954

955955
client.indices().flushIndex(request -> request.indices(INDEX_I)) //
956956
.as(StepVerifier::create) //
957-
.verifyError(ElasticsearchStatusException.class);
957+
.verifyError(RestStatusException.class);
958958
}
959959

960960
@Test // DATAES-684
@@ -1076,7 +1076,7 @@ void getFieldMappingNonExistingField() {
10761076
void getFieldMappingNonExistingIndex() {
10771077

10781078
client.indices().getFieldMapping(request -> request.indices(INDEX_I).fields("message1")).as(StepVerifier::create)
1079-
.verifyError(ElasticsearchStatusException.class);
1079+
.verifyError(RestStatusException.class);
10801080
}
10811081

10821082
@Test // DATAES-796

0 commit comments

Comments
 (0)