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);