diff --git a/CHANGELOG.md b/CHANGELOG.md index a2f503603d..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 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..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; @@ -43,11 +42,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. */ @@ -70,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; @@ -149,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. *

@@ -328,26 +335,26 @@ 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)); + 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) @@ -385,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 new file mode 100644 index 0000000000..2610267efb --- /dev/null +++ b/java-client/src/test/java/org/opensearch/client/transport/httpclient5/ApacheHttpClient5TransportBuilderTest.java @@ -0,0 +1,33 @@ +package org.opensearch.client.transport.httpclient5; + +import com.fasterxml.jackson.databind.JsonNode; +import org.apache.hc.client5.http.ConnectTimeoutException; +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)) + .setConnectionConfigCallback(connectionConfigBuilder -> connectionConfigBuilder + .setConnectTimeout(Timeout.ofMilliseconds(expectTime))) + .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