From 22690ebb9e13d6815b10b52f444400c5793cafed Mon Sep 17 00:00:00 2001 From: Leo Date: Thu, 19 Jun 2025 13:00:06 +0900 Subject: [PATCH 1/6] fix default connectTimeout of transportBuilder Signed-off-by: Leo --- .../transport/httpclient5/ApacheHttpClient5TransportBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java index 334a48a89b..b7ed116c80 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java @@ -328,7 +328,6 @@ public static ApacheHttpClient5TransportBuilder builder(HttpHost... hosts) { private CloseableHttpAsyncClient createHttpClient() { // default timeouts are all infinite RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() - .setConnectTimeout(Timeout.ofMilliseconds(DEFAULT_CONNECT_TIMEOUT_MILLIS)) .setResponseTimeout(Timeout.ofMilliseconds(DEFAULT_RESPONSE_TIMEOUT_MILLIS)); if (requestConfigCallback != null) { From 6313ab8e7f83396d2d0cb173b948a3de83210378 Mon Sep 17 00:00:00 2001 From: Leo Date: Thu, 19 Jun 2025 13:01:11 +0900 Subject: [PATCH 2/6] test connectTimeout of ConnectionConfig Signed-off-by: Leo --- ...ApacheHttpClient5TransportBuilderTest.java | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java diff --git a/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java b/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java new file mode 100644 index 0000000000..912d0efbd9 --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java @@ -0,0 +1,46 @@ +package org.opensearch.client.transport.httpclient5; + +import com.fasterxml.jackson.databind.JsonNode; +import org.apache.hc.client5.http.ConnectTimeoutException; +import org.apache.hc.client5.http.config.ConnectionConfig; +import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; +import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.util.Timeout; +import org.junit.Test; +import org.opensearch.client.opensearch.OpenSearchClient; +import org.opensearch.client.opensearch.core.SearchRequest; + +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class ApacheHttpClient5TransportBuilderTest { + + @Test + public void timeOutTest() { + + int expectTime = 10; + String expectMessage = expectTime + " MILLISECONDS"; + + ApacheHttpClient5Transport apacheHttpClient5Transport = ApacheHttpClient5TransportBuilder.builder(new HttpHost("10.255.255.1", 9200)) + .setHttpClientConfigCallback(httpClientBuilder -> { + + ConnectionConfig connectionConfig = ConnectionConfig.custom() + .setConnectTimeout(Timeout.ofMilliseconds(expectTime)) + .build(); + + PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder + .create() + .setDefaultConnectionConfig(connectionConfig) + .build(); + + return httpClientBuilder.setConnectionManager(connectionManager); + }).build(); + + OpenSearchClient osClient = new OpenSearchClient(apacheHttpClient5Transport); + + ConnectTimeoutException exception = assertThrows(ConnectTimeoutException.class, () -> osClient.search(SearchRequest.of(sb -> sb.index("index")), JsonNode.class)); + + assertTrue("Expected timeout value not found in message", exception.getMessage().contains(expectMessage)); + } +} \ No newline at end of file From 62cf14f5a0afb5e1c081341493096c049a16f007 Mon Sep 17 00:00:00 2001 From: Leo Date: Thu, 19 Jun 2025 13:11:38 +0900 Subject: [PATCH 3/6] fix changelog Signed-off-by: Leo --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index baa6473cf0..9e4cabda5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed - Fixed ScoreCombination's `parameters` structure ([#1594](https://github.com/opensearch-project/opensearch-java/pull/1594)) +- Fixed default connectTimeout of transportBuilder ([#1633](https://github.com/opensearch-project/opensearch-java/pull/1633)) ## [3.0.0] - 05/16/2025 ### ⚠️ Breaking Changes ⚠️ From f469469fba75b1169bb2689482d2fd98af6906b3 Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 23 Jun 2025 14:59:35 +0900 Subject: [PATCH 4/6] fix remove static value Signed-off-by: Leo --- .../httpclient5/ApacheHttpClient5TransportBuilder.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java index b7ed116c80..1b1a591968 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java @@ -43,11 +43,6 @@ import org.opensearch.client.transport.httpclient5.internal.NodeSelector; public class ApacheHttpClient5TransportBuilder { - /** - * The default connection timeout in milliseconds. - */ - public static final int DEFAULT_CONNECT_TIMEOUT_MILLIS = 1000; - /** * The default response timeout in milliseconds. */ From 3b0cb76b3c71a3f6fea0cf9d72720e10f72a396d Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 4 Jul 2025 11:52:39 +0900 Subject: [PATCH 5/6] Add connectionConfigCallback of TransportBuilder Signed-off-by: Leo --- .../ApacheHttpClient5TransportBuilder.java | 54 +++++++++++++------ ...ApacheHttpClient5TransportBuilderTest.java | 19 ++----- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java index 1b1a591968..22b046707a 100644 --- a/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java +++ b/java-client/src/main/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilder.java @@ -8,6 +8,7 @@ package org.opensearch.client.transport.httpclient5; +import javax.net.ssl.SSLContext; import java.security.AccessController; import java.security.NoSuchAlgorithmException; import java.security.PrivilegedAction; @@ -16,19 +17,17 @@ import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; -import org.apache.hc.client5.http.auth.CredentialsProvider; +import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder; +import org.apache.hc.client5.http.impl.async.InternalHttpAsyncClient; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder; -import org.apache.hc.core5.function.Factory; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.nio.ssl.TlsStrategy; @@ -65,6 +64,7 @@ public class ApacheHttpClient5TransportBuilder { private ApacheHttpClient5Transport.FailureListener failureListener; private HttpClientConfigCallback httpClientConfigCallback; private RequestConfigCallback requestConfigCallback; + private ConnectionConfigCallback connectionConfigCallback; private String pathPrefix; private NodeSelector nodeSelector = NodeSelector.ANY; private boolean strictDeprecationMode = false; @@ -144,6 +144,18 @@ public ApacheHttpClient5TransportBuilder setRequestConfigCallback(RequestConfigC return this; } + /** + * Sets the {@link ConnectionConfigCallback} to be used to customize http client configuration + * + * @param connectionConfigCallback the {@link ConnectionConfigCallback} to be used + * @throws NullPointerException if {@code connectionConfigCallback} is {@code null}. + */ + public ApacheHttpClient5TransportBuilder setConnectionConfigCallback(ConnectionConfigCallback connectionConfigCallback) { + Objects.requireNonNull(connectionConfigCallback, "connectionConfigCallback must not be null"); + this.connectionConfigCallback = connectionConfigCallback; + return this; + } + /** * Sets the path's prefix for every request used by the http client. *

@@ -324,24 +336,25 @@ private CloseableHttpAsyncClient createHttpClient() { // default timeouts are all infinite RequestConfig.Builder requestConfigBuilder = RequestConfig.custom() .setResponseTimeout(Timeout.ofMilliseconds(DEFAULT_RESPONSE_TIMEOUT_MILLIS)); + ConnectionConfig.Builder connectionConfigBuilder = ConnectionConfig.custom(); if (requestConfigCallback != null) { requestConfigBuilder = requestConfigCallback.customizeRequestConfig(requestConfigBuilder); } + if (connectionConfigCallback != null) { + connectionConfigBuilder = connectionConfigCallback.customizeConnectionConfig(connectionConfigBuilder); + } + try { final TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create() .setSslContext(SSLContext.getDefault()) // See https://issues.apache.org/jira/browse/HTTPCLIENT-2219 - .setTlsDetailsFactory(new Factory() { - @Override - public TlsDetails create(final SSLEngine sslEngine) { - return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol()); - } - }) + .setTlsDetailsFactory(sslEngine -> new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol())) .build(); final PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder.create() + .setDefaultConnectionConfig(connectionConfigBuilder.build()) .setMaxConnPerRoute(DEFAULT_MAX_CONN_PER_ROUTE) .setMaxConnTotal(DEFAULT_MAX_CONN_TOTAL) .setTlsStrategy(tlsStrategy) @@ -379,16 +392,27 @@ public interface RequestConfigCallback { } /** - * Callback used to customize the {@link CloseableHttpClient} instance used by a {@link RestClient} instance. + * Callback used to customize the default {@link ConnectionConfig} that will be set on the {@link CloseableHttpClient}. + * @see PoolingAsyncClientConnectionManagerBuilder#setDefaultConnectionConfig + */ + public interface ConnectionConfigCallback { + /** + * Allows to customize the {@link ConnectionConfig} used by the connection manager of the {@link InternalHttpAsyncClient}. + * Commonly used to adjust connection-related settings without losing other default values + * + * @param connectionConfigBuilder the {@link ConnectionConfig.Builder} for customizing the connection configuration. + */ + ConnectionConfig.Builder customizeConnectionConfig(ConnectionConfig.Builder connectionConfigBuilder); + } + + /** + * Callback used to customize the {@link CloseableHttpClient} instance used by a {@link InternalHttpAsyncClient} instance. * Allows to customize default {@link RequestConfig} being set to the client and any parameter that * can be set through {@link HttpClientBuilder} */ public interface HttpClientConfigCallback { /** - * Allows to customize the {@link CloseableHttpAsyncClient} being created and used by the {@link RestClient}. - * Commonly used to customize the default {@link CredentialsProvider} for authentication for communication - * through TLS/SSL without losing any other useful default value that the {@link RestClientBuilder} internally - * sets, like connection pooling. + * Allows to customize the {@link CloseableHttpAsyncClient} being created and used by the {@link InternalHttpAsyncClient}. * * @param httpClientBuilder the {@link HttpClientBuilder} for customizing the client instance. */ diff --git a/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java b/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java index 912d0efbd9..2610267efb 100644 --- a/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java +++ b/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java @@ -2,9 +2,6 @@ import com.fasterxml.jackson.databind.JsonNode; import org.apache.hc.client5.http.ConnectTimeoutException; -import org.apache.hc.client5.http.config.ConnectionConfig; -import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; -import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.util.Timeout; import org.junit.Test; @@ -23,19 +20,9 @@ public void timeOutTest() { String expectMessage = expectTime + " MILLISECONDS"; ApacheHttpClient5Transport apacheHttpClient5Transport = ApacheHttpClient5TransportBuilder.builder(new HttpHost("10.255.255.1", 9200)) - .setHttpClientConfigCallback(httpClientBuilder -> { - - ConnectionConfig connectionConfig = ConnectionConfig.custom() - .setConnectTimeout(Timeout.ofMilliseconds(expectTime)) - .build(); - - PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder - .create() - .setDefaultConnectionConfig(connectionConfig) - .build(); - - return httpClientBuilder.setConnectionManager(connectionManager); - }).build(); + .setConnectionConfigCallback(connectionConfigBuilder -> connectionConfigBuilder + .setConnectTimeout(Timeout.ofMilliseconds(expectTime))) + .build(); OpenSearchClient osClient = new OpenSearchClient(apacheHttpClient5Transport); From aff280fa49121c5bab15619ebcc71e83a47e4cb0 Mon Sep 17 00:00:00 2001 From: Leo Date: Fri, 4 Jul 2025 12:02:14 +0900 Subject: [PATCH 6/6] fix CHANGELOG Signed-off-by: Leo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f9efe8b5b..46b3c99761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed - Fixed building instances of `Explanation` by making `details` optional ([#1620](https://github.com/opensearch-project/opensearch-java/pull/1620)) +- Fixed connectTimeout option of `ApacheHttpClient5TransportBuilder` ([#1633](https://github.com/opensearch-project/opensearch-java/pull/1633)) ### Security @@ -68,7 +69,6 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed - Fixed ScoreCombination's `parameters` structure ([#1594](https://github.com/opensearch-project/opensearch-java/pull/1594)) -- Fixed default connectTimeout of transportBuilder ([#1633](https://github.com/opensearch-project/opensearch-java/pull/1633)) ## [3.0.0] - 05/16/2025 ### ⚠️ Breaking Changes ⚠️