* This is an optional parameter. By default, the maximum number of parallel connections per route is set to * 100. @@ -145,6 +151,63 @@ public ApacheHttpClient5FactoryBuilder maxConnectionsPerRoute( final int maxConn return this; } + /** + * Sets the maximum time a connection may sit idle in the pool before being evicted. Connections idle longer than + * this value will not be reused, preventing stale connection issues caused by NAT gateways or load balancers + * silently dropping idle TCP flows (e.g. on BTP Cloud Foundry). + *
+ * This is an optional parameter. Not set by default. + *
+ * + * @param idleTimeout + * The maximum idle duration. Must be positive. + * @return This builder. + */ + @Nonnull + public ApacheHttpClient5FactoryBuilder idleTimeout( @Nonnull final Duration idleTimeout ) + { + this.idleTimeout = idleTimeout; + return this; + } + + /** + * Sets the maximum total lifetime of a connection regardless of activity. Connections older than this value will be + * discarded and a fresh connection opened, ensuring that long-lived connections do not persist through + * infrastructure changes such as NAT gateway replacements. + *+ * This is an optional parameter. Not set by default. + *
+ * + * @param timeToLive + * The maximum connection lifetime. Must be positive. + * @return This builder. + */ + @Nonnull + public ApacheHttpClient5FactoryBuilder timeToLive( @Nonnull final Duration timeToLive ) + { + this.timeToLive = timeToLive; + return this; + } + + /** + * Sets the period of inactivity after which a pooled connection must be validated before reuse. If the connection + * fails validation it is discarded and a fresh one opened, avoiding stale connection errors on the first request + * after an idle period. + *+ * This is an optional parameter. Not set by default (Apache HttpClient 5 default is 2 seconds). + *
+ * + * @param validateAfterInactivity + * The inactivity period after which validation is required. Must be positive. + * @return This builder. + */ + @Nonnull + public ApacheHttpClient5FactoryBuilder validateAfterInactivity( @Nonnull final Duration validateAfterInactivity ) + { + this.validateAfterInactivity = validateAfterInactivity; + return this; + } + /** * Builds a new {@link ApacheHttpClient5Factory} instance with the previously configured parameters. * @@ -158,6 +221,9 @@ public ApacheHttpClient5Factory build() maxConnectionsTotal, maxConnectionsPerRoute, null, - tlsUpgrade); + tlsUpgrade, + idleTimeout, + timeToLive, + validateAfterInactivity); } } diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java index 7b7c0350e..1052a1531 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java @@ -52,6 +52,12 @@ class DefaultApacheHttpClient5Factory implements ApacheHttpClient5Factory private final Timeout timeout; private final int maxConnectionsTotal; private final int maxConnectionsPerRoute; + @Nullable + private final Timeout idleTimeout; + @Nullable + private final TimeValue timeToLive; + @Nullable + private final TimeValue validateAfterInactivity; @Nullable private final HttpRequestInterceptor requestInterceptor; @@ -65,12 +71,48 @@ class DefaultApacheHttpClient5Factory implements ApacheHttpClient5Factory final int maxConnectionsPerRoute, @Nullable final HttpRequestInterceptor requestInterceptor, @Nonnull final ApacheHttpClient5FactoryBuilder.TlsUpgrade tlsUpgrade ) + { + this(timeout, maxConnectionsTotal, maxConnectionsPerRoute, requestInterceptor, tlsUpgrade, null, null, null); + } + + DefaultApacheHttpClient5Factory( + @Nonnull final Duration timeout, + final int maxConnectionsTotal, + final int maxConnectionsPerRoute, + @Nullable final HttpRequestInterceptor requestInterceptor, + @Nonnull final ApacheHttpClient5FactoryBuilder.TlsUpgrade tlsUpgrade, + @Nullable final Duration idleTimeout ) + { + this( + timeout, + maxConnectionsTotal, + maxConnectionsPerRoute, + requestInterceptor, + tlsUpgrade, + idleTimeout, + null, + null); + } + + DefaultApacheHttpClient5Factory( + @Nonnull final Duration timeout, + final int maxConnectionsTotal, + final int maxConnectionsPerRoute, + @Nullable final HttpRequestInterceptor requestInterceptor, + @Nonnull final ApacheHttpClient5FactoryBuilder.TlsUpgrade tlsUpgrade, + @Nullable final Duration idleTimeout, + @Nullable final Duration timeToLive, + @Nullable final Duration validateAfterInactivity ) { this.timeout = toTimeout(timeout); this.maxConnectionsTotal = maxConnectionsTotal; this.maxConnectionsPerRoute = maxConnectionsPerRoute; this.requestInterceptor = requestInterceptor; this.tlsUpgrade = tlsUpgrade; + this.idleTimeout = idleTimeout != null ? toTimeout(idleTimeout) : null; + this.timeToLive = timeToLive != null ? TimeValue.ofMilliseconds(timeToLive.toMillis()) : null; + this.validateAfterInactivity = + validateAfterInactivity != null ? TimeValue.ofMilliseconds(validateAfterInactivity.toMillis()) : null; } @Nonnull @@ -121,8 +163,7 @@ private HttpClientConnectionManager getConnectionManager( @Nullable final HttpDe .create() .setTlsSocketStrategy(getTlsSocketStrategy(destination)) .setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout).build()) - .setDefaultConnectionConfig( - ConnectionConfig.custom().setConnectTimeout(timeout).setSocketTimeout(timeout).build()) + .setDefaultConnectionConfig(buildConnectionConfig()) .setMaxConnTotal(maxConnectionsTotal) .setMaxConnPerRoute(maxConnectionsPerRoute) .build(); @@ -132,6 +173,25 @@ private HttpClientConnectionManager getConnectionManager( @Nullable final HttpDe } } + @Nonnull + private ConnectionConfig buildConnectionConfig() + { + final ConnectionConfig.Builder builder = + ConnectionConfig.custom().setConnectTimeout(timeout).setSocketTimeout(timeout); + + if( idleTimeout != null ) { + builder.setIdleTimeout(idleTimeout); + } + if( timeToLive != null ) { + builder.setTimeToLive(timeToLive); + } + if( validateAfterInactivity != null ) { + builder.setValidateAfterInactivity(validateAfterInactivity); + } + + return builder.build(); + } + @Nonnull private static Timeout toTimeout( @Nonnull final Duration duration ) { diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java index 80a9e6e01..26e321975 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java @@ -30,6 +30,10 @@ import org.apache.hc.client5.http.RouteInfo; import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.config.ConnectionConfig; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -41,6 +45,7 @@ import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.NameValuePair; import org.apache.hc.core5.http.io.HttpClientResponseHandler; +import org.apache.hc.core5.util.TimeValue; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -48,6 +53,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.Mockito; +import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.sap.cloud.sdk.cloudplatform.security.BasicCredentials; @@ -214,6 +220,72 @@ void testProxyConfigurationIsConsidered() } } + @Test + @Timeout( value = 10, unit = TimeUnit.SECONDS ) + @SneakyThrows + void testStaleConnectionCausesRetry() + { + final WireMockServer server = new WireMockServer(wireMockConfig().dynamicPort()); + server.start(); + server.stubFor(get(urlEqualTo("/ping")).willReturn(ok())); + + final DefaultApacheHttpClient5Factory factory = + new DefaultApacheHttpClient5Factory(Duration.ofSeconds(3), 10, 5, null, AUTOMATIC); + final HttpClient client = factory.createHttpClient(); + final String url = server.baseUrl() + "/ping"; + + client.execute(new HttpGet(url), r -> r); + + server.stop(); + Thread.sleep(100); + + assertThatThrownBy(() -> client.execute(new HttpGet(url), r -> r)).isInstanceOf(IOException.class); + } + + @Test + @Timeout( value = 10, unit = TimeUnit.SECONDS ) + @SneakyThrows + void testIdleTimeoutPreventsStaleConnectionReuse() + { + final WireMockServer server = new WireMockServer(wireMockConfig().dynamicPort()); + server.start(); + server.stubFor(get(urlEqualTo("/ping")).willReturn(ok())); + final int port = server.port(); + + final ApacheHttpClient5Factory factory = destination -> { + final ConnectionConfig connConfig = + ConnectionConfig + .custom() + .setConnectTimeout(org.apache.hc.core5.util.Timeout.ofSeconds(3)) + .setSocketTimeout(org.apache.hc.core5.util.Timeout.ofSeconds(3)) + .setIdleTimeout(org.apache.hc.core5.util.Timeout.ofMilliseconds(200)) + .setTimeToLive(TimeValue.ofMilliseconds(500)) + .build(); + final PoolingHttpClientConnectionManager cm = + PoolingHttpClientConnectionManagerBuilder.create().setDefaultConnectionConfig(connConfig).build(); + return HttpClients.custom().setConnectionManager(cm).build(); + }; + final HttpClient client = factory.createHttpClient(null); + final String url = "http://localhost:" + port + "/ping"; + + client.execute(new HttpGet(url), r -> r); + + server.stop(); + Thread.sleep(300); + + final WireMockServer newServer = new WireMockServer(wireMockConfig().port(port)); + newServer.start(); + newServer.stubFor(get(urlEqualTo("/ping")).willReturn(ok())); + + try { + final int status = client.execute(new HttpGet(url), r -> r.getCode()); + assertThat(status).isEqualTo(200); + } + finally { + newServer.stop(); + } + } + @Test @SneakyThrows void verifyDefaultRetryMechanism()