[Backport 9.6] Per request timeout setting fix

docs/reference/transport/rest5-client/config/timeouts.md

@@ -27,19 +27,26 @@ Rest5ClientBuilder builder = Rest5Client
 
 1. Specify `https` for TLS.
 
-You can also set per-request timeouts using `RequestOptions`, which override the builder defaults. The following example sets a response timeout of 60 seconds, as well as a connection request timeout of 1 second (to limit pooled connection wait time):
+You can also set per-request timeouts using `RequestOptions`, which override the builder defaults. The following example sets a response timeout of 60 seconds, as well as a connection request timeout of 1 second (to limit pooled connection wait time), for a single search request:
 
 % :::{include-code} src={{doc-tests-src}}/rest5_client/RestClientDocumentation.java tag=rest-client-config-request-options-timeouts
 ```java
-RequestConfig requestConfig = RequestConfig.custom()   
+RequestConfig requestConfig = RequestConfig.custom()
     .setResponseTimeout(Timeout.ofSeconds(60))
     .setConnectionRequestTimeout(Timeout.ofSeconds(1))
     .build();
 
-RequestOptions options = RequestOptions.DEFAULT.toBuilder()
-    .setRequestConfig(requestConfig)
-    .build();
+Rest5ClientOptions options = new Rest5ClientOptions.Builder(
+    RequestOptions.DEFAULT.toBuilder().setRequestConfig(requestConfig)
+).build();
 
-ElasticsearchTransport transport = new Rest5ClientTransport(
-    restClient, new JacksonJsonpMapper(), options);
+SearchResponse<Product> response = esClient
+    .withTransportOptions(options) // <1>
+    .search(s -> s.index("products"), Product.class);
 ```
+
+1. `withTransportOptions` returns a client that uses these options, sharing the transport of `esClient`. To use these options for all requests instead, pass them to the `Rest5ClientTransport` constructor.
+
+::::{note}
+Per-request timeouts are ignored in versions 9.0.0 to 9.3.x of the client. In those versions, set timeouts when building the `Rest5Client`. Refer to [Known issues](/release-notes/known-issues.md#known-issues-9-x-up-to-9-5-6) for details.
+::::

docs/release-notes/known-issues.md

@@ -7,6 +7,25 @@ navigation_title: "Known issues"
 
 For detailed issues, refer to the [Java client repo](https://github.com/elastic/elasticsearch-java/issues).
 
+## 9.x (up to 9.5.6) [known-issues-9-x-up-to-9-5-6]
+
+Original report on [GitHub](https://github.com/elastic/elasticsearch-java/issues/1378).  
+All 9.x versions of the client previous to, respectively, 9.4.10 and 9.5.7 ignore the `RequestConfig` set in per-request `RequestOptions`, such as response and connection request timeouts: requests always use the `RequestConfig` set when building the `Rest5Client`.
+The bug only concerns the `Rest5Client`, not the legacy `RestClient`.  
+This problem was fixed in 9.4.10, 9.5.7 and 9.6.0.
+Detailed version info:
+
+| Version | Affected Releases | Update to |
+|---------|------------------|-----------|
+| 9.0     | all              | 9.4.10    |
+| 9.1     | all              | 9.4.10    |
+| 9.2     | all              | 9.4.10    |
+| 9.3     | all              | 9.4.10    |
+| 9.4     | 9.4.0 -> 9.4.9   | 9.4.10    |
+| 9.5     | 9.5.0 -> 9.5.6   | 9.5.7     |
+
+To work around this issue in affected versions, set timeouts when building the `Rest5Client`, as described in [Timeouts](/reference/transport/rest5-client/config/timeouts.md).
+
 ## 9.x (up to 9.4.4) [known-issues-9-x-up-to-9-4-4]
 
 Original report on [GitHub](https://github.com/elastic/elasticsearch-java/issues/1289).  

java-client/src/test/java/co/elastic/clients/documentation/rest5_client/RestClientDocumentation.java

@@ -19,6 +19,12 @@
 
 package co.elastic.clients.documentation.rest5_client;
 
+import co.elastic.clients.documentation.usage.Product;
+import co.elastic.clients.elasticsearch.ElasticsearchClient;
+import co.elastic.clients.elasticsearch.core.SearchResponse;
+import co.elastic.clients.json.jackson.JacksonJsonpMapper;
+import co.elastic.clients.transport.rest5_client.Rest5ClientOptions;
+import co.elastic.clients.transport.rest5_client.Rest5ClientTransport;
 import co.elastic.clients.transport.rest5_client.low_level.Cancellable;
 import co.elastic.clients.transport.rest5_client.low_level.HttpAsyncResponseConsumerFactory;
 import co.elastic.clients.transport.rest5_client.low_level.Node;
@@ -323,15 +329,22 @@ public void commonConfiguration() throws Exception {
             //end::rest-client-config-timeouts
         }
         {
+            ElasticsearchClient esClient = new ElasticsearchClient(new Rest5ClientTransport(
+                Rest5Client.builder(new HttpHost("localhost", 9200)).build(), new JacksonJsonpMapper()));
+
             //tag::rest-client-config-request-options-timeouts
             RequestConfig requestConfig = RequestConfig.custom()
-                .setConnectTimeout(Timeout.ofMilliseconds(5000))
-                .setConnectionRequestTimeout(Timeout.ofMilliseconds(60000))
+                .setResponseTimeout(Timeout.ofSeconds(60))
+                .setConnectionRequestTimeout(Timeout.ofSeconds(1))
                 .build();
 
-            RequestOptions options = RequestOptions.DEFAULT.toBuilder()
-                .setRequestConfig(requestConfig)
-                .build();
+            Rest5ClientOptions options = new Rest5ClientOptions.Builder(
+                RequestOptions.DEFAULT.toBuilder().setRequestConfig(requestConfig)
+            ).build();
+
+            SearchResponse<Product> response = esClient
+                .withTransportOptions(options) // <1>
+                .search(s -> s.index("products"), Product.class);
             //end::rest-client-config-request-options-timeouts
         }
         {

rest5-client/src/main/java/co/elastic/clients/transport/rest5_client/low_level/Rest5Client.java

@@ -895,6 +895,10 @@ private static class RequestContext {
                 .getHttpAsyncResponseConsumerFactory()
                 .createHttpAsyncResponseConsumer();
             this.context = HttpClientContext.create();
+            RequestConfig requestConfig = request.httpRequest.getConfig();
+            if (requestConfig != null) {
+                this.context.setRequestConfig(requestConfig);
+            }
         }
     }
 

rest5-client/src/test/java/co/elastic/clients/transport/rest5_client/low_level/RestClientRequestConfigTests.java

@@ -0,0 +1,125 @@
+/*
+ * 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.rest5_client.low_level;
+
+import com.sun.net.httpserver.HttpServer;
+import org.apache.hc.client5.http.config.RequestConfig;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.util.Timeout;
+import org.elasticsearch.mocksocket.MockHttpServer;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketTimeoutException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests that the {@link RequestConfig} of {@link RequestOptions} applies to the request, overriding the client's
+ * default request config.
+ */
+public class RestClientRequestConfigTests extends RestClientTestCase {
+
+    // Timeouts are checked by the I/O reactor once per second: the delay must leave some margin
+    private static final long RESPONSE_DELAY_MILLIS = 3000;
+
+    private HttpServer httpServer;
+    private final CountDownLatch stopping = new CountDownLatch(1);
+
+    @BeforeEach
+    public void startHttpServer() throws Exception {
+        httpServer = MockHttpServer.createHttp(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
+        httpServer.createContext("/", exchange -> {
+            try {
+                stopping.await(RESPONSE_DELAY_MILLIS, TimeUnit.MILLISECONDS);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+            exchange.sendResponseHeaders(200, -1);
+            exchange.close();
+        });
+        httpServer.start();
+    }
+
+    @AfterEach
+    public void stopHttpServer() {
+        stopping.countDown();
+        httpServer.stop(0);
+    }
+
+    @Test
+    public void testShorterResponseTimeout() throws Exception {
+        try (Rest5Client restClient = newRestClient(Timeout.ofSeconds(30))) {
+            Request request = newRequest(Timeout.ofMilliseconds(100));
+            assertThrows(SocketTimeoutException.class, () -> restClient.performRequest(request));
+        }
+    }
+
+    @Test
+    public void testShorterResponseTimeoutAsync() throws Exception {
+        try (Rest5Client restClient = newRestClient(Timeout.ofSeconds(30))) {
+            CompletableFuture<Response> future = new CompletableFuture<>();
+            restClient.performRequestAsync(newRequest(Timeout.ofMilliseconds(100)), new ResponseListener() {
+                @Override
+                public void onSuccess(Response response) {
+                    future.complete(response);
+                }
+
+                @Override
+                public void onFailure(Exception exception) {
+                    future.completeExceptionally(exception);
+                }
+            });
+            ExecutionException e = assertThrows(ExecutionException.class, () -> future.get(10, TimeUnit.SECONDS));
+            assertTrue(e.getCause().toString(), e.getCause() instanceof SocketTimeoutException);
+        }
+    }
+
+    @Test
+    public void testLongerResponseTimeout() throws Exception {
+        try (Rest5Client restClient = newRestClient(Timeout.ofMilliseconds(100))) {
+            Response response = restClient.performRequest(newRequest(Timeout.ofSeconds(30)));
+            assertEquals(200, response.getStatusCode());
+        }
+    }
+
+    private Rest5Client newRestClient(Timeout defaultResponseTimeout) {
+        return Rest5Client.builder(new HttpHost("http", httpServer.getAddress().getHostString(),
+                httpServer.getAddress().getPort()))
+            .setRequestConfigCallback(r -> r.setResponseTimeout(defaultResponseTimeout))
+            .build();
+    }
+
+    private static Request newRequest(Timeout responseTimeout) {
+        Request request = new Request("GET", "/");
+        request.setOptions(RequestOptions.DEFAULT.toBuilder()
+            .setRequestConfig(RequestConfig.custom().setResponseTimeout(responseTimeout).build()));
+        return request;
+    }
+}