From 51976d1e502fb30b71b77345883146a44692155f Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 1 Oct 2024 15:54:18 +0200 Subject: [PATCH 01/13] base working impl --- .../transport/DefaultTransportOptions.java | 13 +++++ .../transport/ElasticsearchTransportBase.java | 35 +++++++++---- .../TransportBodyResponseException.java | 51 +++++++++++++++++++ .../clients/transport/TransportOptions.java | 4 ++ .../rest_client/RestClientOptions.java | 16 ++++++ .../documentation/DocTestsTransport.java | 5 ++ 6 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java diff --git a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java index f199705c1..c7c2caa02 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java @@ -88,6 +88,11 @@ public Function, Boolean> onWarnings() { return onWarnings; } + @Override + public boolean retrieveOriginalJsonResponseOnException() { + return false; + } + @Override public Builder toBuilder() { return new Builder(this); @@ -111,6 +116,7 @@ public abstract static class AbstractBuilder parameters; private Function, Boolean> onWarnings; + private boolean retrieveOriginalJsonResponseOnException; public AbstractBuilder() { } @@ -119,10 +125,17 @@ public AbstractBuilder(DefaultTransportOptions options) { this.headers = new HeaderMap(options.headers); this.parameters = copyOrNull(options.parameters); this.onWarnings = options.onWarnings; + this.retrieveOriginalJsonResponseOnException = options.retrieveOriginalJsonResponseOnException(); } protected abstract BuilderT self(); + @Override + public BuilderT retrieveOriginalJsonResponseOnException(boolean value){ + this.retrieveOriginalJsonResponseOnException = value; + return self(); + } + @Override public BuilderT addHeader(String name, String value) { if (name.equalsIgnoreCase(HeaderMap.CLIENT_META)) { diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 7d1e98608..f9614e5bd 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -45,6 +45,8 @@ import jakarta.json.stream.JsonParser; import javax.annotation.Nullable; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; @@ -377,28 +379,41 @@ private ResponseT decodeTransportResponse( ) throws IOException { if (endpoint instanceof JsonEndpoint) { + + // Expecting a body + if (entity == null) { + throw new TransportException( + clientResp, + "Expecting a response body, but none was sent", + endpoint.id() + ); + } + InputStream content = entity.asInputStream(); + InputStream contentForException = null; + + // if the option to print the original body has been set, the body has to be + // copied first to another stream to be read again by the exception class + if(options().retrieveOriginalJsonResponseOnException()){ + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + entity.writeTo(baos); + content = new ByteArrayInputStream(baos.toByteArray()); + contentForException = new ByteArrayInputStream(baos.toByteArray()); + } + @SuppressWarnings("unchecked") JsonEndpoint jsonEndpoint = (JsonEndpoint) endpoint; // Successful response ResponseT response = null; JsonpDeserializer responseParser = jsonEndpoint.responseDeserializer(); if (responseParser != null) { - // Expecting a body - if (entity == null) { - throw new TransportException( - clientResp, - "Expecting a response body, but none was sent", - endpoint.id() - ); - } checkJsonContentType(entity.contentType(), clientResp, endpoint); try ( - InputStream content = entity.asInputStream(); JsonParser parser = mapper.jsonProvider().createParser(content) ) { response = responseParser.deserialize(parser, mapper); } catch (Exception e) { - throw new TransportException( + throw new TransportBodyResponseException( + contentForException, clientResp, "Failed to decode response", endpoint.id(), diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java new file mode 100644 index 000000000..b0749757b --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java @@ -0,0 +1,51 @@ +package co.elastic.clients.transport; + +import co.elastic.clients.transport.http.TransportHttpClient; + +import javax.annotation.Nullable; +import java.io.BufferedReader; +import java.io.InputStream; +import java.io.InputStreamReader; + +public class TransportBodyResponseException extends TransportException { + + private String originalBody; + + public TransportBodyResponseException(InputStream originalBody, TransportHttpClient.Response response, String message, String endpointId) { + this(originalBody, response, message, endpointId, null); + } + + public TransportBodyResponseException(InputStream originalBody,TransportHttpClient.Response response, String message, String endpointId, + Throwable cause) { + super(response, message, endpointId, cause); + try { + if (originalBody != null) { + StringBuilder sb = new StringBuilder(); + BufferedReader br = new BufferedReader(new InputStreamReader(originalBody)); + String read; + + while ((read=br.readLine()) != null) { + sb.append(read); + } + + br.close(); + this.originalBody = sb.toString(); + // Closing original body input stream + originalBody.close(); + } + + // Make sure the response is closed to free up resources. + response.close(); + } catch (Exception e) { + this.addSuppressed(e); + } + } + + /** + * The original response body, before json deserialization. + */ + @Nullable + public String originalBody() { + return originalBody; + } +} diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java index d6c41f490..1e23b93d3 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java @@ -38,6 +38,8 @@ public interface TransportOptions { Function, Boolean> onWarnings(); + boolean retrieveOriginalJsonResponseOnException(); + Builder toBuilder(); default TransportOptions with(Consumer fn) { @@ -59,5 +61,7 @@ interface Builder extends ObjectBuilder { Builder removeParameter(String name); Builder onWarnings(Function, Boolean> listener); + + Builder retrieveOriginalJsonResponseOnException(boolean value); } } diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java index 9de6da07e..fd92ee307 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java @@ -42,6 +42,8 @@ public class RestClientOptions implements TransportOptions { private final RequestOptions options; + boolean retrieveOriginalJsonResponseOnException; + @VisibleForTesting static final String CLIENT_META_VALUE = getClientMeta(); @VisibleForTesting @@ -99,6 +101,15 @@ public Function, Boolean> onWarnings() { return warnings -> options.getWarningsHandler().warningsShouldFailRequest(warnings); } + @Override + public boolean retrieveOriginalJsonResponseOnException() { + return this.retrieveOriginalJsonResponseOnException; + } + + public void setRetrieveOriginalJsonResponseOnException(boolean retrieveOriginalJsonResponseOnException) { + this.retrieveOriginalJsonResponseOnException = retrieveOriginalJsonResponseOnException; + } + @Override public Builder toBuilder() { return new Builder(options.toBuilder()); @@ -181,6 +192,11 @@ public TransportOptions.Builder onWarnings(Function, Boolean> liste return this; } + @Override + public TransportOptions.Builder retrieveOriginalJsonResponseOnException(boolean value) { + return this; + } + @Override public RestClientOptions build() { return new RestClientOptions(addBuiltinHeaders(builder).build()); diff --git a/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java b/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java index 29ea6a629..953dae431 100644 --- a/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java +++ b/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java @@ -64,6 +64,11 @@ public Function, Boolean> onWarnings() { return null; } + @Override + public boolean retrieveOriginalJsonResponseOnException() { + return false; + } + @Override public Builder toBuilder() { return null; From 35c07e9845424ff074ac37d62ca223906e17b019 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 1 Oct 2024 16:00:10 +0200 Subject: [PATCH 02/13] twr, remove unused constructor --- .../clients/transport/ElasticsearchTransportBase.java | 11 ++++++----- .../transport/TransportBodyResponseException.java | 4 ---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index f9614e5bd..959e5d31e 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -393,11 +393,12 @@ private ResponseT decodeTransportResponse( // if the option to print the original body has been set, the body has to be // copied first to another stream to be read again by the exception class - if(options().retrieveOriginalJsonResponseOnException()){ - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - entity.writeTo(baos); - content = new ByteArrayInputStream(baos.toByteArray()); - contentForException = new ByteArrayInputStream(baos.toByteArray()); + if(options().retrieveOriginalJsonResponseOnException()) { + try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + entity.writeTo(baos); + content = new ByteArrayInputStream(baos.toByteArray()); + contentForException = new ByteArrayInputStream(baos.toByteArray()); + } } @SuppressWarnings("unchecked") diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java index b0749757b..f157ee2c1 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java @@ -11,10 +11,6 @@ public class TransportBodyResponseException extends TransportException { private String originalBody; - public TransportBodyResponseException(InputStream originalBody, TransportHttpClient.Response response, String message, String endpointId) { - this(originalBody, response, message, endpointId, null); - } - public TransportBodyResponseException(InputStream originalBody,TransportHttpClient.Response response, String message, String endpointId, Throwable cause) { super(response, message, endpointId, cause); From 9835bc4c165829942739c8f47c1d217c05893bdf Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 1 Oct 2024 17:09:12 +0200 Subject: [PATCH 03/13] unit test --- .../clients/transport/TransportTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java index b79b03893..723f738c3 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java @@ -21,9 +21,11 @@ import co.elastic.clients.elasticsearch.ElasticsearchClient; import co.elastic.clients.json.jackson.JacksonJsonpMapper; +import co.elastic.clients.transport.rest_client.RestClientOptions; import co.elastic.clients.transport.rest_client.RestClientTransport; import com.sun.net.httpserver.HttpServer; import org.apache.http.HttpHost; +import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; import org.junit.jupiter.api.Assertions; @@ -33,6 +35,9 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import java.util.Collections; + +import static co.elastic.clients.util.ContentType.APPLICATION_JSON; public class TransportTest extends Assertions { @@ -72,4 +77,58 @@ public void testXMLResponse() throws Exception { Response restClientResponse = (Response)ex.response().originalResponse(); assertEquals(401, restClientResponse.getStatusLine().getStatusCode()); } + + + @Test + public void testOriginalJsonBodyRetrievalException() throws Exception { + HttpServer httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + + httpServer.createContext("/_cat/indices", exchange -> { + exchange.getResponseHeaders().put("Content-Type", Collections.singletonList(APPLICATION_JSON)); + exchange.getResponseHeaders().put("X-Elastic-Product", Collections.singletonList("Elasticsearch")); + exchange.sendResponseHeaders(200, 0); + OutputStream out = exchange.getResponseBody(); + out.write( + "definitely not json".getBytes(StandardCharsets.UTF_8) + ); + out.close(); + }); + + httpServer.start(); + InetSocketAddress address = httpServer.getAddress(); + + RestClient restClient = RestClient + .builder(new HttpHost(address.getHostString(), address.getPort(), "http")) + .build(); + + // no transport options, should throw TransportBodyResponseException, but with an empty originalBody + ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, new JacksonJsonpMapper())); + + TransportBodyResponseException ex = Assertions.assertThrows( + TransportBodyResponseException.class, + () -> esClient.cat().indices() + ); + + assertEquals(200, ex.statusCode()); + assertEquals(null, ex.originalBody()); + + // setting transport option + RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT); + options.setRetrieveOriginalJsonResponseOnException(true); + + ElasticsearchTransport transport = new RestClientTransport( + restClient, new JacksonJsonpMapper(), options); + + ElasticsearchClient esClientOptions = new ElasticsearchClient(transport); + + ex = Assertions.assertThrows( + TransportBodyResponseException.class, + () -> esClientOptions.cat().indices() + ); + + httpServer.stop(0); + + assertEquals(200, ex.statusCode()); + assertEquals( "definitely not json", ex.originalBody()); + } } From 0e93afd5479a48201583f3e736857c2c8e344813 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 1 Oct 2024 17:22:47 +0200 Subject: [PATCH 04/13] license --- .../TransportBodyResponseException.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java index f157ee2c1..ac5e4f424 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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 + * + * http://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 co.elastic.clients.transport; import co.elastic.clients.transport.http.TransportHttpClient; From d003cc6158d2e14d9ca4c1ce95dd0dd24c54c04e Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 8 Oct 2024 12:52:47 +0200 Subject: [PATCH 05/13] complete refactor --- .../transport/DefaultTransportOptions.java | 10 +-- .../transport/ElasticsearchTransportBase.java | 22 +------ .../TransportBodyResponseException.java | 66 ------------------- .../clients/transport/TransportOptions.java | 4 +- .../rest_client/RestClientHttpClient.java | 61 ++++++++++++++++- .../rest_client/RestClientOptions.java | 22 +++---- .../documentation/DocTestsTransport.java | 2 +- .../clients/transport/TransportTest.java | 37 +++++++---- .../rest_client/RestClientOptionsTest.java | 2 +- 9 files changed, 103 insertions(+), 123 deletions(-) delete mode 100644 java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java diff --git a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java index c7c2caa02..66aa555cd 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java @@ -89,7 +89,7 @@ public Function, Boolean> onWarnings() { } @Override - public boolean retrieveOriginalJsonResponseOnException() { + public boolean keepResponseBodyOnException() { return false; } @@ -116,7 +116,7 @@ public abstract static class AbstractBuilder parameters; private Function, Boolean> onWarnings; - private boolean retrieveOriginalJsonResponseOnException; + private boolean keepResponseBodyOnException; public AbstractBuilder() { } @@ -125,14 +125,14 @@ public AbstractBuilder(DefaultTransportOptions options) { this.headers = new HeaderMap(options.headers); this.parameters = copyOrNull(options.parameters); this.onWarnings = options.onWarnings; - this.retrieveOriginalJsonResponseOnException = options.retrieveOriginalJsonResponseOnException(); + this.keepResponseBodyOnException = options.keepResponseBodyOnException(); } protected abstract BuilderT self(); @Override - public BuilderT retrieveOriginalJsonResponseOnException(boolean value){ - this.retrieveOriginalJsonResponseOnException = value; + public BuilderT keepResponseBodyOnException(boolean value){ + this.keepResponseBodyOnException = value; return self(); } diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 959e5d31e..8ee5567a7 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -335,11 +335,6 @@ private ResponseT getApiResponse( checkJsonContentType(entity.contentType(), clientResp, endpoint); - // We may have to replay it. - if (!entity.isRepeatable()) { - entity = new ByteArrayBinaryData(entity); - } - try (InputStream content = entity.asInputStream()) { try (JsonParser parser = mapper.jsonProvider().createParser(content)) { ErrorT error = errorDeserializer.deserialize(parser, mapper); @@ -388,18 +383,6 @@ private ResponseT decodeTransportResponse( endpoint.id() ); } - InputStream content = entity.asInputStream(); - InputStream contentForException = null; - - // if the option to print the original body has been set, the body has to be - // copied first to another stream to be read again by the exception class - if(options().retrieveOriginalJsonResponseOnException()) { - try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { - entity.writeTo(baos); - content = new ByteArrayInputStream(baos.toByteArray()); - contentForException = new ByteArrayInputStream(baos.toByteArray()); - } - } @SuppressWarnings("unchecked") JsonEndpoint jsonEndpoint = (JsonEndpoint) endpoint; @@ -409,12 +392,11 @@ private ResponseT decodeTransportResponse( if (responseParser != null) { checkJsonContentType(entity.contentType(), clientResp, endpoint); try ( - JsonParser parser = mapper.jsonProvider().createParser(content) + JsonParser parser = mapper.jsonProvider().createParser(entity.asInputStream()) ) { response = responseParser.deserialize(parser, mapper); } catch (Exception e) { - throw new TransportBodyResponseException( - contentForException, + throw new TransportException( clientResp, "Failed to decode response", endpoint.id(), diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java b/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java deleted file mode 100644 index ac5e4f424..000000000 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportBodyResponseException.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you 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 - * - * http://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 co.elastic.clients.transport; - -import co.elastic.clients.transport.http.TransportHttpClient; - -import javax.annotation.Nullable; -import java.io.BufferedReader; -import java.io.InputStream; -import java.io.InputStreamReader; - -public class TransportBodyResponseException extends TransportException { - - private String originalBody; - - public TransportBodyResponseException(InputStream originalBody,TransportHttpClient.Response response, String message, String endpointId, - Throwable cause) { - super(response, message, endpointId, cause); - try { - if (originalBody != null) { - StringBuilder sb = new StringBuilder(); - BufferedReader br = new BufferedReader(new InputStreamReader(originalBody)); - String read; - - while ((read=br.readLine()) != null) { - sb.append(read); - } - - br.close(); - this.originalBody = sb.toString(); - // Closing original body input stream - originalBody.close(); - } - - // Make sure the response is closed to free up resources. - response.close(); - } catch (Exception e) { - this.addSuppressed(e); - } - } - - /** - * The original response body, before json deserialization. - */ - @Nullable - public String originalBody() { - return originalBody; - } -} diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java index 1e23b93d3..e0182ef6f 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java @@ -38,7 +38,7 @@ public interface TransportOptions { Function, Boolean> onWarnings(); - boolean retrieveOriginalJsonResponseOnException(); + boolean keepResponseBodyOnException(); Builder toBuilder(); @@ -62,6 +62,6 @@ interface Builder extends ObjectBuilder { Builder onWarnings(Function, Boolean> listener); - Builder retrieveOriginalJsonResponseOnException(boolean value); + Builder keepResponseBodyOnException(boolean value); } } diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java index 64b5aa08a..f7b25d994 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java @@ -23,6 +23,7 @@ import co.elastic.clients.transport.http.HeaderMap; import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.util.BinaryData; +import co.elastic.clients.util.ByteArrayBinaryData; import co.elastic.clients.util.NoCopyByteArrayOutputStream; import org.apache.http.Header; import org.apache.http.HeaderElement; @@ -34,8 +35,10 @@ import org.elasticsearch.client.RestClient; import javax.annotation.Nullable; +import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; import java.io.OutputStream; import java.nio.ByteBuffer; import java.util.AbstractList; @@ -85,10 +88,14 @@ public RestClientOptions createOptions(@Nullable TransportOptions options) { } @Override - public Response performRequest(String endpointId, @Nullable Node node, Request request, TransportOptions options) throws IOException { + public Response performRequest(String endpointId, @Nullable Node node, Request request, + TransportOptions options) throws IOException { RestClientOptions rcOptions = RestClientOptions.of(options); org.elasticsearch.client.Request restRequest = createRestRequest(request, rcOptions); org.elasticsearch.client.Response restResponse = restClient.performRequest(restRequest); + if (options.keepResponseBodyOnException()) { + return new RepeatableBodyResponse(restResponse); + } return new RestResponse(restResponse); } @@ -103,7 +110,7 @@ public CompletableFuture performRequestAsync( try { RestClientOptions rcOptions = RestClientOptions.of(options); restRequest = createRestRequest(request, rcOptions); - } catch(Throwable thr) { + } catch (Throwable thr) { // Terminate early future.completeExceptionally(thr); return future; @@ -112,6 +119,9 @@ public CompletableFuture performRequestAsync( future.cancellable = restClient.performRequestAsync(restRequest, new ResponseListener() { @Override public void onSuccess(org.elasticsearch.client.Response response) { + if (options.keepResponseBodyOnException()) { + future.complete(new RepeatableBodyResponse(response)); + } future.complete(new RestResponse(response)); } @@ -166,7 +176,7 @@ private org.elasticsearch.client.Request createRestRequest(Request request, Rest if (body != null) { ContentType ct = null; String ctStr; - if (( ctStr = requestHeaders.get(HeaderMap.CONTENT_TYPE)) != null) { + if ((ctStr = requestHeaders.get(HeaderMap.CONTENT_TYPE)) != null) { ct = ContentTypeCache.computeIfAbsent(ctStr, ContentType::parse); } clientReq.setEntity(new MultiBufferEntity(body, ct)); @@ -241,6 +251,51 @@ public void close() throws IOException { } } + public class RepeatableBodyResponse extends RestResponse { + + BinaryData repeatableBody; + + RepeatableBodyResponse(org.elasticsearch.client.Response restResponse) { + super(restResponse); + } + + @Nullable + @Override + public BinaryData body() throws IOException { + if(repeatableBody != null) { + return repeatableBody; + } + BinaryData body = super.body(); + if (body != null) { + if(body.isRepeatable()){ + repeatableBody = body; + } + else{ + repeatableBody = new ByteArrayBinaryData(body); + } + } + return repeatableBody; + } + + public String getOriginalBodyAsString() throws IOException { + BinaryData body = body(); + + if (body != null) { + StringBuilder sb = new StringBuilder(); + BufferedReader br = new BufferedReader(new InputStreamReader(body.asInputStream())); + String read; + + while ((read = br.readLine()) != null) { + sb.append(read); + } + br.close(); + return sb.toString(); + } + return null; + } + + } + private static class HttpEntityBinaryData implements BinaryData { private final HttpEntity entity; diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java index fd92ee307..842a45c62 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientOptions.java @@ -42,7 +42,7 @@ public class RestClientOptions implements TransportOptions { private final RequestOptions options; - boolean retrieveOriginalJsonResponseOnException; + boolean keepResponseBodyOnException; @VisibleForTesting static final String CLIENT_META_VALUE = getClientMeta(); @@ -65,7 +65,8 @@ static RestClientOptions of(@Nullable TransportOptions options) { return builder.build(); } - public RestClientOptions(RequestOptions options) { + public RestClientOptions(RequestOptions options, boolean keepResponseBodyOnException) { + this.keepResponseBodyOnException = keepResponseBodyOnException; this.options = addBuiltinHeaders(options.toBuilder()).build(); } @@ -102,12 +103,8 @@ public Function, Boolean> onWarnings() { } @Override - public boolean retrieveOriginalJsonResponseOnException() { - return this.retrieveOriginalJsonResponseOnException; - } - - public void setRetrieveOriginalJsonResponseOnException(boolean retrieveOriginalJsonResponseOnException) { - this.retrieveOriginalJsonResponseOnException = retrieveOriginalJsonResponseOnException; + public boolean keepResponseBodyOnException() { + return this.keepResponseBodyOnException; } @Override @@ -119,6 +116,8 @@ public static class Builder implements TransportOptions.Builder { private RequestOptions.Builder builder; + private boolean keepResponseBodyOnException; + public Builder(RequestOptions.Builder builder) { this.builder = builder; } @@ -193,18 +192,19 @@ public TransportOptions.Builder onWarnings(Function, Boolean> liste } @Override - public TransportOptions.Builder retrieveOriginalJsonResponseOnException(boolean value) { + public TransportOptions.Builder keepResponseBodyOnException(boolean value) { + this.keepResponseBodyOnException = value; return this; } @Override public RestClientOptions build() { - return new RestClientOptions(addBuiltinHeaders(builder).build()); + return new RestClientOptions(addBuiltinHeaders(builder).build(), keepResponseBodyOnException); } } static RestClientOptions initialOptions() { - return new RestClientOptions(SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS); + return new RestClientOptions(SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS, false); } private static RequestOptions.Builder addBuiltinHeaders(RequestOptions.Builder builder) { diff --git a/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java b/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java index 953dae431..6d1c41eb5 100644 --- a/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java +++ b/java-client/src/test/java/co/elastic/clients/documentation/DocTestsTransport.java @@ -65,7 +65,7 @@ public Function, Boolean> onWarnings() { } @Override - public boolean retrieveOriginalJsonResponseOnException() { + public boolean keepResponseBodyOnException() { return false; } diff --git a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java index 723f738c3..6a8be7435 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java @@ -21,6 +21,7 @@ import co.elastic.clients.elasticsearch.ElasticsearchClient; import co.elastic.clients.json.jackson.JacksonJsonpMapper; +import co.elastic.clients.transport.rest_client.RestClientHttpClient; import co.elastic.clients.transport.rest_client.RestClientOptions; import co.elastic.clients.transport.rest_client.RestClientTransport; import com.sun.net.httpserver.HttpServer; @@ -43,7 +44,8 @@ public class TransportTest extends Assertions { @Test public void testXMLResponse() throws Exception { - HttpServer httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + HttpServer httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), + 0), 0); httpServer.createContext("/_cat/indices", exchange -> { exchange.sendResponseHeaders(401, 0); @@ -61,7 +63,8 @@ public void testXMLResponse() throws Exception { .builder(new HttpHost(address.getHostString(), address.getPort(), "http")) .build(); - ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, new JacksonJsonpMapper())); + ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, + new JacksonJsonpMapper())); TransportException ex = Assertions.assertThrows( TransportException.class, @@ -74,18 +77,20 @@ public void testXMLResponse() throws Exception { assertEquals("es/cat.indices", ex.endpointId()); // Original response is transport-dependent - Response restClientResponse = (Response)ex.response().originalResponse(); + Response restClientResponse = (Response) ex.response().originalResponse(); assertEquals(401, restClientResponse.getStatusLine().getStatusCode()); } @Test public void testOriginalJsonBodyRetrievalException() throws Exception { - HttpServer httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); + HttpServer httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), + 0), 0); httpServer.createContext("/_cat/indices", exchange -> { exchange.getResponseHeaders().put("Content-Type", Collections.singletonList(APPLICATION_JSON)); - exchange.getResponseHeaders().put("X-Elastic-Product", Collections.singletonList("Elasticsearch")); + exchange.getResponseHeaders().put("X-Elastic-Product", Collections.singletonList("Elasticsearch" + )); exchange.sendResponseHeaders(200, 0); OutputStream out = exchange.getResponseBody(); out.write( @@ -101,20 +106,20 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { .builder(new HttpHost(address.getHostString(), address.getPort(), "http")) .build(); - // no transport options, should throw TransportBodyResponseException, but with an empty originalBody - ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, new JacksonJsonpMapper())); + // no transport options, should throw TransportException, but original body cannot be retrieved + ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, + new JacksonJsonpMapper())); - TransportBodyResponseException ex = Assertions.assertThrows( - TransportBodyResponseException.class, + TransportException ex = Assertions.assertThrows( + TransportException.class, () -> esClient.cat().indices() ); assertEquals(200, ex.statusCode()); - assertEquals(null, ex.originalBody()); + assertNotEquals(RestClientHttpClient.RepeatableBodyResponse.class, ex.response().getClass()); // setting transport option - RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT); - options.setRetrieveOriginalJsonResponseOnException(true); + RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT, true); ElasticsearchTransport transport = new RestClientTransport( restClient, new JacksonJsonpMapper(), options); @@ -122,13 +127,17 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { ElasticsearchClient esClientOptions = new ElasticsearchClient(transport); ex = Assertions.assertThrows( - TransportBodyResponseException.class, + TransportException.class, () -> esClientOptions.cat().indices() ); httpServer.stop(0); assertEquals(200, ex.statusCode()); - assertEquals( "definitely not json", ex.originalBody()); + assertEquals(RestClientHttpClient.RepeatableBodyResponse.class, ex.response().getClass()); + + try (RestClientHttpClient.RepeatableBodyResponse repeatableResponse = (RestClientHttpClient.RepeatableBodyResponse) ex.response()){ + assertEquals("definitely not json",repeatableResponse.getOriginalBodyAsString()); + } } } diff --git a/java-client/src/test/java/co/elastic/clients/transport/rest_client/RestClientOptionsTest.java b/java-client/src/test/java/co/elastic/clients/transport/rest_client/RestClientOptionsTest.java index cd6558a4f..cf8995944 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/rest_client/RestClientOptionsTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/rest_client/RestClientOptionsTest.java @@ -192,7 +192,7 @@ void testRequestOptionsOverridingBuiltin() throws Exception { new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort(), "http") ).build(); - ElasticsearchTransport transport = newRestClientTransport(llrc, new SimpleJsonpMapper(), new RestClientOptions(options)); + ElasticsearchTransport transport = newRestClientTransport(llrc, new SimpleJsonpMapper(), new RestClientOptions(options,false)); ElasticsearchClient esClient = new ElasticsearchClient(transport); // Should not override client meta String id = checkHeaders(esClient); From 5bcf666979ac293567dbabfac8e6d2b69579a789 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 8 Oct 2024 13:00:41 +0200 Subject: [PATCH 06/13] reverting old changes --- .../transport/ElasticsearchTransportBase.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 8ee5567a7..6b0800e6e 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -375,21 +375,20 @@ private ResponseT decodeTransportResponse( if (endpoint instanceof JsonEndpoint) { - // Expecting a body - if (entity == null) { - throw new TransportException( - clientResp, - "Expecting a response body, but none was sent", - endpoint.id() - ); - } - @SuppressWarnings("unchecked") JsonEndpoint jsonEndpoint = (JsonEndpoint) endpoint; // Successful response ResponseT response = null; JsonpDeserializer responseParser = jsonEndpoint.responseDeserializer(); if (responseParser != null) { + // Expecting a body + if (entity == null) { + throw new TransportException( + clientResp, + "Expecting a response body, but none was sent", + endpoint.id() + ); + } checkJsonContentType(entity.contentType(), clientResp, endpoint); try ( JsonParser parser = mapper.jsonProvider().createParser(entity.asInputStream()) From 9832695f51952a1f0c5f8182b6574bc533c0e115 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 8 Oct 2024 13:04:49 +0200 Subject: [PATCH 07/13] codestyle --- .../elastic/clients/transport/ElasticsearchTransportBase.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 6b0800e6e..0ea9e6d2e 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -36,7 +36,6 @@ import co.elastic.clients.util.LanguageRuntimeVersions; import co.elastic.clients.util.ApiTypeHelper; import co.elastic.clients.util.BinaryData; -import co.elastic.clients.util.ByteArrayBinaryData; import co.elastic.clients.util.ContentType; import co.elastic.clients.util.MissingRequiredPropertyException; import co.elastic.clients.util.NoCopyByteArrayOutputStream; @@ -45,8 +44,6 @@ import jakarta.json.stream.JsonParser; import javax.annotation.Nullable; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; From cdeb1920bf9f6f3dd9188fabe52084d9271275ac Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Tue, 8 Oct 2024 13:13:11 +0200 Subject: [PATCH 08/13] leave ElasticsearchTransportBase untouched --- .../clients/transport/ElasticsearchTransportBase.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 0ea9e6d2e..790debeb7 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -33,6 +33,7 @@ import co.elastic.clients.transport.instrumentation.Instrumentation; import co.elastic.clients.transport.instrumentation.NoopInstrumentation; import co.elastic.clients.transport.instrumentation.OpenTelemetryForElasticsearch; +import co.elastic.clients.util.ByteArrayBinaryData; import co.elastic.clients.util.LanguageRuntimeVersions; import co.elastic.clients.util.ApiTypeHelper; import co.elastic.clients.util.BinaryData; @@ -332,6 +333,11 @@ private ResponseT getApiResponse( checkJsonContentType(entity.contentType(), clientResp, endpoint); + // We may have to replay it. + if (!entity.isRepeatable()) { + entity = new ByteArrayBinaryData(entity); + } + try (InputStream content = entity.asInputStream()) { try (JsonParser parser = mapper.jsonProvider().createParser(content)) { ErrorT error = errorDeserializer.deserialize(parser, mapper); @@ -388,7 +394,8 @@ private ResponseT decodeTransportResponse( } checkJsonContentType(entity.contentType(), clientResp, endpoint); try ( - JsonParser parser = mapper.jsonProvider().createParser(entity.asInputStream()) + InputStream content = entity.asInputStream(); + JsonParser parser = mapper.jsonProvider().createParser(content) ) { response = responseParser.deserialize(parser, mapper); } catch (Exception e) { From 30ee62651243a528169be3279396a5a773307e75 Mon Sep 17 00:00:00 2001 From: Laura Trotta <153528055+l-trotta@users.noreply.github.com> Date: Wed, 16 Oct 2024 12:42:51 +0200 Subject: [PATCH 09/13] Update java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java Co-authored-by: Sylvain Wallez --- .../java/co/elastic/clients/transport/TransportOptions.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java index e0182ef6f..6b9fe4c66 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java @@ -38,6 +38,11 @@ public interface TransportOptions { Function, Boolean> onWarnings(); + /** + * If {@code true}, the response body in {@code TransportException.response().body()} is guaranteed to be + * replayable (i.e. buffered), even if the original response was streamed. This allows inspecting the + * response body in case of error. + */ boolean keepResponseBodyOnException(); Builder toBuilder(); From 435d2ef375267cdc741d88ee57397207cdf254a8 Mon Sep 17 00:00:00 2001 From: Laura Trotta <153528055+l-trotta@users.noreply.github.com> Date: Wed, 16 Oct 2024 12:43:58 +0200 Subject: [PATCH 10/13] Update java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java Co-authored-by: Sylvain Wallez --- .../java/co/elastic/clients/transport/TransportOptions.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java index 6b9fe4c66..9cbbdd40d 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/TransportOptions.java @@ -67,6 +67,11 @@ interface Builder extends ObjectBuilder { Builder onWarnings(Function, Boolean> listener); + /** + * Should the response body be buffered and made available in {@code TransportException.response().body()}? + * This setting guarantees that the response body is buffered for inspection if parsing fails, even if originally + * streamed by the http library. + */ Builder keepResponseBodyOnException(boolean value); } } From 0beeebb5e4dcfdd4ccfb2c762492a4d41538d2e4 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Wed, 16 Oct 2024 18:35:05 +0200 Subject: [PATCH 11/13] custom generic response, checking in transport base --- .../transport/DefaultTransportOptions.java | 20 ++++-- .../transport/ElasticsearchTransportBase.java | 4 ++ .../http/RepeatableBodyResponse.java | 65 +++++++++++++++++++ .../rest_client/RestClientHttpClient.java | 54 --------------- .../clients/transport/TransportTest.java | 26 ++++++-- 5 files changed, 104 insertions(+), 65 deletions(-) create mode 100644 java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java diff --git a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java index 66aa555cd..cdc35639e 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java +++ b/java-client/src/main/java/co/elastic/clients/transport/DefaultTransportOptions.java @@ -37,6 +37,7 @@ public class DefaultTransportOptions implements TransportOptions { private final HeaderMap headers; private final Map parameters; private final Function, Boolean> onWarnings; + private boolean keepResponseBodyOnException; public static final DefaultTransportOptions EMPTY = new DefaultTransportOptions(); @@ -44,6 +45,16 @@ public DefaultTransportOptions() { this(new HeaderMap(), Collections.emptyMap(), null); } + public DefaultTransportOptions( + @Nullable HeaderMap headers, + @Nullable Map parameters, + @Nullable Function, Boolean> onWarnings, + boolean keepResponseBodyOnException + ) { + this(headers,parameters,onWarnings); + this.keepResponseBodyOnException = keepResponseBodyOnException; + } + public DefaultTransportOptions( @Nullable HeaderMap headers, @Nullable Map parameters, @@ -53,10 +64,11 @@ public DefaultTransportOptions( this.parameters = (parameters == null || parameters.isEmpty()) ? Collections.emptyMap() : Collections.unmodifiableMap(parameters); this.onWarnings = onWarnings; + this.keepResponseBodyOnException = false; } protected DefaultTransportOptions(AbstractBuilder builder) { - this(builder.headers, builder.parameters, builder.onWarnings); + this(builder.headers, builder.parameters, builder.onWarnings, builder.keepResponseBodyOnException); } public static DefaultTransportOptions of(@Nullable TransportOptions options) { @@ -90,7 +102,7 @@ public Function, Boolean> onWarnings() { @Override public boolean keepResponseBodyOnException() { - return false; + return keepResponseBodyOnException; } @Override @@ -125,13 +137,13 @@ public AbstractBuilder(DefaultTransportOptions options) { this.headers = new HeaderMap(options.headers); this.parameters = copyOrNull(options.parameters); this.onWarnings = options.onWarnings; - this.keepResponseBodyOnException = options.keepResponseBodyOnException(); + this.keepResponseBodyOnException = options.keepResponseBodyOnException; } protected abstract BuilderT self(); @Override - public BuilderT keepResponseBodyOnException(boolean value){ + public BuilderT keepResponseBodyOnException(boolean value) { this.keepResponseBodyOnException = value; return self(); } diff --git a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java index 790debeb7..1d422eb66 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java +++ b/java-client/src/main/java/co/elastic/clients/transport/ElasticsearchTransportBase.java @@ -29,6 +29,7 @@ import co.elastic.clients.transport.endpoints.BooleanEndpoint; import co.elastic.clients.transport.endpoints.BooleanResponse; import co.elastic.clients.transport.http.HeaderMap; +import co.elastic.clients.transport.http.RepeatableBodyResponse; import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.transport.instrumentation.Instrumentation; import co.elastic.clients.transport.instrumentation.NoopInstrumentation; @@ -306,6 +307,9 @@ private ResponseT getApiResponse( int statusCode = clientResp.statusCode(); + if(options().keepResponseBodyOnException()){ + clientResp = RepeatableBodyResponse.of(clientResp); + } try { if (statusCode == 200) { checkProductHeader(clientResp, endpoint); diff --git a/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java b/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java new file mode 100644 index 000000000..1d965468d --- /dev/null +++ b/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java @@ -0,0 +1,65 @@ +package co.elastic.clients.transport.http; + +import co.elastic.clients.util.BinaryData; +import co.elastic.clients.util.ByteArrayBinaryData; + +import javax.annotation.Nullable; +import java.io.IOException; +import java.util.List; + +public class RepeatableBodyResponse implements TransportHttpClient.Response { + + private final TransportHttpClient.Response response; + private final BinaryData body; + + public static TransportHttpClient.Response of(TransportHttpClient.Response response) throws IOException { + BinaryData body = response.body(); + if (body == null || body.isRepeatable()) { + return response; + } + return new RepeatableBodyResponse(response); + } + + public RepeatableBodyResponse(TransportHttpClient.Response response) throws IOException { + this.response = response; + this.body = new ByteArrayBinaryData(response.body()); + } + + @Override + public TransportHttpClient.Node node() { + return response.node(); + } + + @Override + public int statusCode() { + return response.statusCode(); + } + + @Nullable + @Override + public String header(String name) { + return response.header(name); + } + + @Override + public List headers(String name) { + return response.headers(name); + } + + @Nullable + @Override + public BinaryData body() throws IOException { + return this.body; + } + + @Nullable + @Override + public Object originalResponse() { + return response.originalResponse(); + } + + @Override + public void close() throws IOException { + response.close(); + } +} diff --git a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java index f7b25d994..1bcc06b05 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java +++ b/java-client/src/main/java/co/elastic/clients/transport/rest_client/RestClientHttpClient.java @@ -23,7 +23,6 @@ import co.elastic.clients.transport.http.HeaderMap; import co.elastic.clients.transport.http.TransportHttpClient; import co.elastic.clients.util.BinaryData; -import co.elastic.clients.util.ByteArrayBinaryData; import co.elastic.clients.util.NoCopyByteArrayOutputStream; import org.apache.http.Header; import org.apache.http.HeaderElement; @@ -35,10 +34,8 @@ import org.elasticsearch.client.RestClient; import javax.annotation.Nullable; -import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; -import java.io.InputStreamReader; import java.io.OutputStream; import java.nio.ByteBuffer; import java.util.AbstractList; @@ -93,9 +90,6 @@ public Response performRequest(String endpointId, @Nullable Node node, Request r RestClientOptions rcOptions = RestClientOptions.of(options); org.elasticsearch.client.Request restRequest = createRestRequest(request, rcOptions); org.elasticsearch.client.Response restResponse = restClient.performRequest(restRequest); - if (options.keepResponseBodyOnException()) { - return new RepeatableBodyResponse(restResponse); - } return new RestResponse(restResponse); } @@ -119,9 +113,6 @@ public CompletableFuture performRequestAsync( future.cancellable = restClient.performRequestAsync(restRequest, new ResponseListener() { @Override public void onSuccess(org.elasticsearch.client.Response response) { - if (options.keepResponseBodyOnException()) { - future.complete(new RepeatableBodyResponse(response)); - } future.complete(new RestResponse(response)); } @@ -251,51 +242,6 @@ public void close() throws IOException { } } - public class RepeatableBodyResponse extends RestResponse { - - BinaryData repeatableBody; - - RepeatableBodyResponse(org.elasticsearch.client.Response restResponse) { - super(restResponse); - } - - @Nullable - @Override - public BinaryData body() throws IOException { - if(repeatableBody != null) { - return repeatableBody; - } - BinaryData body = super.body(); - if (body != null) { - if(body.isRepeatable()){ - repeatableBody = body; - } - else{ - repeatableBody = new ByteArrayBinaryData(body); - } - } - return repeatableBody; - } - - public String getOriginalBodyAsString() throws IOException { - BinaryData body = body(); - - if (body != null) { - StringBuilder sb = new StringBuilder(); - BufferedReader br = new BufferedReader(new InputStreamReader(body.asInputStream())); - String read; - - while ((read = br.readLine()) != null) { - sb.append(read); - } - br.close(); - return sb.toString(); - } - return null; - } - - } - private static class HttpEntityBinaryData implements BinaryData { private final HttpEntity entity; diff --git a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java index 6a8be7435..d25466bbd 100644 --- a/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java +++ b/java-client/src/test/java/co/elastic/clients/transport/TransportTest.java @@ -21,9 +21,10 @@ import co.elastic.clients.elasticsearch.ElasticsearchClient; import co.elastic.clients.json.jackson.JacksonJsonpMapper; -import co.elastic.clients.transport.rest_client.RestClientHttpClient; +import co.elastic.clients.transport.http.RepeatableBodyResponse; import co.elastic.clients.transport.rest_client.RestClientOptions; import co.elastic.clients.transport.rest_client.RestClientTransport; +import co.elastic.clients.util.BinaryData; import com.sun.net.httpserver.HttpServer; import org.apache.http.HttpHost; import org.elasticsearch.client.RequestOptions; @@ -32,6 +33,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import java.io.BufferedReader; +import java.io.InputStreamReader; import java.io.OutputStream; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -106,7 +109,7 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { .builder(new HttpHost(address.getHostString(), address.getPort(), "http")) .build(); - // no transport options, should throw TransportException, but original body cannot be retrieved + // no transport options, response is not RepeatableBodyResponse, original body cannot be retrieved ElasticsearchClient esClient = new ElasticsearchClient(new RestClientTransport(restClient, new JacksonJsonpMapper())); @@ -116,7 +119,7 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { ); assertEquals(200, ex.statusCode()); - assertNotEquals(RestClientHttpClient.RepeatableBodyResponse.class, ex.response().getClass()); + assertNotEquals(RepeatableBodyResponse.class, ex.response().getClass()); // setting transport option RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT, true); @@ -134,10 +137,19 @@ public void testOriginalJsonBodyRetrievalException() throws Exception { httpServer.stop(0); assertEquals(200, ex.statusCode()); - assertEquals(RestClientHttpClient.RepeatableBodyResponse.class, ex.response().getClass()); - - try (RestClientHttpClient.RepeatableBodyResponse repeatableResponse = (RestClientHttpClient.RepeatableBodyResponse) ex.response()){ - assertEquals("definitely not json",repeatableResponse.getOriginalBodyAsString()); + assertEquals(RepeatableBodyResponse.class, ex.response().getClass()); + + try (RepeatableBodyResponse repeatableResponse = (RepeatableBodyResponse) ex.response()){ + BinaryData body = repeatableResponse.body(); + StringBuilder sb = new StringBuilder(); + BufferedReader br = new BufferedReader(new InputStreamReader(body.asInputStream())); + String read; + + while ((read = br.readLine()) != null) { + sb.append(read); + } + br.close(); + assertEquals("definitely not json",sb.toString()); } } } From d93cd29c132db93032cdb5df66ad4dfcf9223335 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Wed, 16 Oct 2024 18:41:27 +0200 Subject: [PATCH 12/13] license header --- .../http/RepeatableBodyResponse.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java b/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java index 1d965468d..d578cdcec 100644 --- a/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java +++ b/java-client/src/main/java/co/elastic/clients/transport/http/RepeatableBodyResponse.java @@ -1,3 +1,22 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you 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 + * + * http://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 co.elastic.clients.transport.http; import co.elastic.clients.util.BinaryData; From d4164909a3f83690febde466316df8381f51c479 Mon Sep 17 00:00:00 2001 From: Laura Trotta Date: Thu, 17 Oct 2024 17:02:31 +0200 Subject: [PATCH 13/13] asciidocs --- .../release-notes/release-highlights.asciidoc | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/release-notes/release-highlights.asciidoc b/docs/release-notes/release-highlights.asciidoc index f76a6c1c8..49ee99ed8 100644 --- a/docs/release-notes/release-highlights.asciidoc +++ b/docs/release-notes/release-highlights.asciidoc @@ -8,6 +8,26 @@ For a list of detailed changes, including bug fixes, please see the https://gith [discrete] ==== Version 8.16 * `ElasticsearchClient` is now `Closeable`. Closing a client object also closes the underlying transport - https://github.com/elastic/elasticsearch-java/pull/851[#851] +* Added option to make the response body available in case of deserialization error- https://github.com/elastic/elasticsearch-java/pull/886[#886]. + +** While it has always been possible to set the log level to `trace` and have the client print both the json bodies of the requests and responses, it's often not the best solution because of the large amount of information printed. +** To enable the feature: + + RestClientOptions options = new RestClientOptions(RequestOptions.DEFAULT, true); + ElasticsearchTransport transport = new RestClientTransport(restClient, new JacksonJsonpMapper(), options); + ElasticsearchClient esClientWithOptions = new ElasticsearchClient(transport); + +** To retrieve the original body from the TransportException that gets thrown in case of deserialization errors: + + try{ + // some code that returns faulty json + } + catch (TransportException ex){ + try (RepeatableBodyResponse repeatableResponse = (RepeatableBodyResponse) ex.response()) { + BinaryData body = repeatableResponse.body(); + } + } + [discrete] ==== Version 8.15