From 59a256de55d21ed13115b2b97c4d04cc291deb49 Mon Sep 17 00:00:00 2001 From: Matthias Kuhr Date: Mon, 17 Nov 2025 13:00:39 +0100 Subject: [PATCH 1/2] Add test covering the default retry behavior of HTTP client 5 --- .../DefaultApacheHttpClient5FactoryTest.java | 482 +++++++++--------- 1 file changed, 242 insertions(+), 240 deletions(-) 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 a114f9487..e642fc536 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 @@ -1,5 +1,6 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.ok; @@ -52,268 +53,269 @@ import lombok.SneakyThrows; -class DefaultApacheHttpClient5FactoryTest -{ - private static final int TEST_TIMEOUT = 300_000; - private static final Duration CLIENT_TIMEOUT = Duration.ofSeconds(10L); - private static final int MAX_CONNECTIONS = 10; - private static final int MAX_CONNECTIONS_PER_ROUTE = 5; - - @RegisterExtension - static final WireMockExtension WIRE_MOCK_SERVER = - WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); - @RegisterExtension - static final WireMockExtension SECOND_WIRE_MOCK_SERVER = - WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); - - private SoftAssertions softly; - private ApacheHttpClient5Factory sut; - private HttpRequestInterceptor requestInterceptor; - - @BeforeEach - @SneakyThrows - void setup() - { - softly = new SoftAssertions(); - - requestInterceptor = mock(HttpRequestInterceptor.class); - doNothing().when(requestInterceptor).process(any(), any(), any()); - - sut = - new DefaultApacheHttpClient5Factory( - CLIENT_TIMEOUT, - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); - } +class DefaultApacheHttpClient5FactoryTest { + private static final int TEST_TIMEOUT = 300_000; + private static final Duration CLIENT_TIMEOUT = Duration.ofSeconds(10L); + private static final int MAX_CONNECTIONS = 10; + private static final int MAX_CONNECTIONS_PER_ROUTE = 5; + + @RegisterExtension + static final WireMockExtension WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + @RegisterExtension + static final WireMockExtension SECOND_WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + + private SoftAssertions softly; + private ApacheHttpClient5Factory sut; + private HttpRequestInterceptor requestInterceptor; - @Test - @SneakyThrows - void testHttpClientUsesTimeout() - { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/timeout")).willReturn(ok().withFixedDelay(5_000))); + @BeforeEach + @SneakyThrows + void setup() { + softly = new SoftAssertions(); - final ApacheHttpClient5Factory factoryWithTooLittleTimeout = + requestInterceptor = mock(HttpRequestInterceptor.class); + doNothing().when(requestInterceptor).process(any(), any(), any()); + + sut = + new DefaultApacheHttpClient5Factory( + CLIENT_TIMEOUT, + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); + } + + @Test + @SneakyThrows + void testHttpClientUsesTimeout() { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/timeout")).willReturn(ok().withFixedDelay(5_000))); + + final ApacheHttpClient5Factory factoryWithTooLittleTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(3L), + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); - final ApacheHttpClient5Factory factoryWithEnoughTimeout = + final ApacheHttpClient5Factory factoryWithEnoughTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(7L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(7L), + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); - final ClassicHttpRequest request = new HttpGet(WIRE_MOCK_SERVER.url("/timeout")); + final ClassicHttpRequest request = new HttpGet(WIRE_MOCK_SERVER.url("/timeout")); - final HttpClient clientWithTooLittleTimeout = factoryWithTooLittleTimeout.createHttpClient(); - assertThatThrownBy(() -> clientWithTooLittleTimeout.execute(request, ignoreResponse())) + final HttpClient clientWithTooLittleTimeout = factoryWithTooLittleTimeout.createHttpClient(); + assertThatThrownBy(() -> clientWithTooLittleTimeout.execute(request, ignoreResponse())) .isInstanceOf(IOException.class) .hasMessageContaining("Read timed out"); - final HttpClient clientWithEnoughTimeout = factoryWithEnoughTimeout.createHttpClient(); - clientWithEnoughTimeout.execute(request, assertOk()); + final HttpClient clientWithEnoughTimeout = factoryWithEnoughTimeout.createHttpClient(); + clientWithEnoughTimeout.execute(request, assertOk()); - softly.assertAll(); - } + softly.assertAll(); + } - @Test - @Timeout( value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS ) - @SneakyThrows - void testHttpClientUsesMaxConnections() - { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-1")).willReturn(ok())); - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-2")).willReturn(ok())); + @Test + @Timeout(value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS) + @SneakyThrows + void testHttpClientUsesMaxConnections() { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-1")).willReturn(ok())); + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-2")).willReturn(ok())); - final ApacheHttpClient5Factory sut = + final ApacheHttpClient5Factory sut = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - 1, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(3L), // this timeout is also used for the connection lease + 1, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); + + final HttpClient client = sut.createHttpClient(); + final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-1")); + final ClassicHttpRequest secondRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-2")); + + assertCannotBeExecutedInParallel(firstRequest, secondRequest, client); + } + + @Test + @Timeout(value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS) + @SneakyThrows + void testHttpClientUsesMaxConnectionsPerRoute() { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); + SECOND_WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); + + final ApacheHttpClient5Factory sut = + new DefaultApacheHttpClient5Factory( + Duration.ofSeconds(3L), // this timeout is also used for the connection lease + MAX_CONNECTIONS, + 1, + requestInterceptor, + AUTOMATIC); - final HttpClient client = sut.createHttpClient(); - final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-1")); - final ClassicHttpRequest secondRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-2")); + final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-per-route")); + final ClassicHttpRequest secondRequest = new HttpGet(SECOND_WIRE_MOCK_SERVER.url("/max-connections-per-route")); + final HttpClient client = sut.createHttpClient(); - assertCannotBeExecutedInParallel(firstRequest, secondRequest, client); - } + assertCanBeExecutedInParallel(firstRequest, secondRequest, client); + assertCannotBeExecutedInParallel(firstRequest, firstRequest, client); + } - @Test - @Timeout( value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS ) - @SneakyThrows - void testHttpClientUsesMaxConnectionsPerRoute() - { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); - SECOND_WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); + @Test + @SneakyThrows + void testProxyConfigurationIsConsidered() { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/proxy")).willReturn(ok())); - final ApacheHttpClient5Factory sut = - new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - MAX_CONNECTIONS, - 1, - requestInterceptor, - AUTOMATIC); - - final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-per-route")); - final ClassicHttpRequest secondRequest = new HttpGet(SECOND_WIRE_MOCK_SERVER.url("/max-connections-per-route")); - final HttpClient client = sut.createHttpClient(); - - assertCanBeExecutedInParallel(firstRequest, secondRequest, client); - assertCannotBeExecutedInParallel(firstRequest, firstRequest, client); - } + final BasicCredentials credentials = new BasicCredentials("user", "pass"); - @Test - @SneakyThrows - void testProxyConfigurationIsConsidered() - { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/proxy")).willReturn(ok())); - - final BasicCredentials credentials = new BasicCredentials("user", "pass"); - - final DefaultHttpDestination destination = + final DefaultHttpDestination destination = DefaultHttpDestination - .builder("http://www.sap.com") - .proxyConfiguration(ProxyConfiguration.of(WIRE_MOCK_SERVER.baseUrl(), credentials)) - .build(); - final DefaultHttpDestination spiedDestination = spy(destination); - - final HttpClient httpClient = sut.createHttpClient(spiedDestination); - Mockito.verify(spiedDestination, atLeastOnce()).getProxyConfiguration(); - - doAnswer(invocation -> { - final HttpRequest request = invocation.getArgument(0); - final HttpClientContext context = invocation.getArgument(2); - - assertThat(request.getUri()).isEqualTo(URI.create("http://www.sap.com/proxy")); - assertThat(Arrays.stream(request.getHeaders()).map(NameValuePair::getName).collect(Collectors.toSet())) - .containsExactlyInAnyOrder(HttpHeaders.ACCEPT_ENCODING, HttpHeaders.PROXY_AUTHORIZATION); - assertThat(Arrays.toString(request.getHeaders(HttpHeaders.PROXY_AUTHORIZATION))) - .contains(credentials.getHttpHeaderValue()); - - final RouteInfo httpRoute = context.getHttpRoute(); - assertThat(httpRoute).isNotNull(); - assertThat(httpRoute.getHopCount()).isEqualTo(2); - assertThat(httpRoute.getHopTarget(0)).isEqualTo(HttpHost.create(WIRE_MOCK_SERVER.baseUrl())); - assertThat(httpRoute.getHopTarget(1)).isEqualTo(HttpHost.create("http://www.sap.com:80")); - - return null; - }).when(requestInterceptor).process(any(), any(), any()); - - try( final ClassicHttpResponse response = httpClient.execute(new HttpGet("/proxy"), r -> r) ) { - WIRE_MOCK_SERVER.verify(getRequestedFor(urlEqualTo("/proxy"))); - assertThat(response.getCode()).isEqualTo(HttpStatus.SC_OK); - } + .builder("http://www.sap.com") + .proxyConfiguration(ProxyConfiguration.of(WIRE_MOCK_SERVER.baseUrl(), credentials)) + .build(); + final DefaultHttpDestination spiedDestination = spy(destination); + + final HttpClient httpClient = sut.createHttpClient(spiedDestination); + Mockito.verify(spiedDestination, atLeastOnce()).getProxyConfiguration(); + + doAnswer(invocation -> { + final HttpRequest request = invocation.getArgument(0); + final HttpClientContext context = invocation.getArgument(2); + + assertThat(request.getUri()).isEqualTo(URI.create("http://www.sap.com/proxy")); + assertThat(Arrays.stream(request.getHeaders()).map(NameValuePair::getName).collect(Collectors.toSet())) + .containsExactlyInAnyOrder(HttpHeaders.ACCEPT_ENCODING, HttpHeaders.PROXY_AUTHORIZATION); + assertThat(Arrays.toString(request.getHeaders(HttpHeaders.PROXY_AUTHORIZATION))) + .contains(credentials.getHttpHeaderValue()); + + final RouteInfo httpRoute = context.getHttpRoute(); + assertThat(httpRoute).isNotNull(); + assertThat(httpRoute.getHopCount()).isEqualTo(2); + assertThat(httpRoute.getHopTarget(0)).isEqualTo(HttpHost.create(WIRE_MOCK_SERVER.baseUrl())); + assertThat(httpRoute.getHopTarget(1)).isEqualTo(HttpHost.create("http://www.sap.com:80")); + + return null; + }).when(requestInterceptor).process(any(), any(), any()); + + try (final ClassicHttpResponse response = httpClient.execute(new HttpGet("/proxy"), r -> r)) { + WIRE_MOCK_SERVER.verify(getRequestedFor(urlEqualTo("/proxy"))); + assertThat(response.getCode()).isEqualTo(HttpStatus.SC_OK); } - - @SneakyThrows - private void assertCannotBeExecutedInParallel( - @Nonnull final ClassicHttpRequest firstRequest, - @Nonnull final ClassicHttpRequest secondRequest, - @Nonnull final HttpClient client ) - { - final CountDownLatch firstResponseReceived = new CountDownLatch(1); - final CountDownLatch secondRequestFailed = new CountDownLatch(1); - - final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(firstRequest, r -> { - firstResponseReceived.countDown(); - try { - secondRequestFailed.await(); - } - catch( final InterruptedException e ) { - softly.fail("Interrupted while waiting for request to be sent", e); - } - return null; - }); - } - catch( final IOException e ) { - softly.fail("Failed to execute request", e); - } + } + + @Test + @SneakyThrows + void verifyDefaultRetryMechanism() { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/too-many-requests")) + .willReturn(aResponse().withStatus(429))); + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/temporary-error")) + .willReturn(aResponse().withStatus(503))); + + final HttpClient client = sut.createHttpClient(); + client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/too-many-requests")), ignored -> ignored); + client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/temporary-error")), ignored -> ignored); + + WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/too-many-requests"))); + WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/temporary-error"))); + } + + @SneakyThrows + private void assertCannotBeExecutedInParallel( + @Nonnull final ClassicHttpRequest firstRequest, + @Nonnull final ClassicHttpRequest secondRequest, + @Nonnull final HttpClient client) { + final CountDownLatch firstResponseReceived = new CountDownLatch(1); + final CountDownLatch secondRequestFailed = new CountDownLatch(1); + + final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(firstRequest, r -> { + firstResponseReceived.countDown(); + try { + secondRequestFailed.await(); + } catch (final InterruptedException e) { + softly.fail("Interrupted while waiting for request to be sent", e); + } + return null; }); - - firstResponseReceived.await(); - final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { - softly - .assertThatThrownBy(() -> client.execute(secondRequest, ignoreResponse())) - .isExactlyInstanceOf(ConnectionRequestTimeoutException.class); - secondRequestFailed.countDown(); + } catch (final IOException e) { + softly.fail("Failed to execute request", e); + } + }); + + firstResponseReceived.await(); + final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { + softly + .assertThatThrownBy(() -> client.execute(secondRequest, ignoreResponse())) + .isExactlyInstanceOf(ConnectionRequestTimeoutException.class); + secondRequestFailed.countDown(); + }); + + secondRequestFailed.await(); + + firstFuture.get(); + secondFuture.get(); + softly.assertAll(); + } + + @SneakyThrows + private void assertCanBeExecutedInParallel( + @Nonnull final ClassicHttpRequest firstRequest, + @Nonnull final ClassicHttpRequest secondRequest, + @Nonnull final HttpClient client) { + final CountDownLatch firstResponseReceived = new CountDownLatch(1); + final CountDownLatch secondResponseReceived = new CountDownLatch(1); + + final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(firstRequest, r -> { + firstResponseReceived.countDown(); + try { + secondResponseReceived.await(); + } catch (final InterruptedException e) { + softly.fail("Interrupted while waiting for request to be sent", e); + } + return null; }); - - secondRequestFailed.await(); - - firstFuture.get(); - secondFuture.get(); - softly.assertAll(); - } - - @SneakyThrows - private void assertCanBeExecutedInParallel( - @Nonnull final ClassicHttpRequest firstRequest, - @Nonnull final ClassicHttpRequest secondRequest, - @Nonnull final HttpClient client ) - { - final CountDownLatch firstResponseReceived = new CountDownLatch(1); - final CountDownLatch secondResponseReceived = new CountDownLatch(1); - - final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(firstRequest, r -> { - firstResponseReceived.countDown(); - try { - secondResponseReceived.await(); - } - catch( final InterruptedException e ) { - softly.fail("Interrupted while waiting for request to be sent", e); - } - return null; - }); - } - catch( final IOException e ) { - softly.fail("Failed to execute request", e); - } - }); - - firstResponseReceived.await(); - final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(secondRequest, r -> { - secondResponseReceived.countDown(); - return null; - }); - } - catch( final IOException e ) { - softly.fail("Failed to execute request", e); - } + } catch (final IOException e) { + softly.fail("Failed to execute request", e); + } + }); + + firstResponseReceived.await(); + final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(secondRequest, r -> { + secondResponseReceived.countDown(); + return null; }); - - secondResponseReceived.await(); - - firstFuture.get(); - secondFuture.get(); - softly.assertAll(); - } - - @Nonnull - private HttpClientResponseHandler ignoreResponse() - { - return r -> null; - } - - @Nonnull - private HttpClientResponseHandler assertOk() - { - return r -> { - softly.assertThat(r.getCode()).isEqualTo(HttpStatus.SC_OK); - return null; - }; - } + } catch (final IOException e) { + softly.fail("Failed to execute request", e); + } + }); + + secondResponseReceived.await(); + + firstFuture.get(); + secondFuture.get(); + softly.assertAll(); + } + + @Nonnull + private HttpClientResponseHandler ignoreResponse() { + return r -> null; + } + + @Nonnull + private HttpClientResponseHandler assertOk() { + return r -> { + softly.assertThat(r.getCode()).isEqualTo(HttpStatus.SC_OK); + return null; + }; + } } From 0a27b1f14ef794ab39ac5a025416bb73bdf50a79 Mon Sep 17 00:00:00 2001 From: Matthias Kuhr Date: Mon, 17 Nov 2025 13:02:02 +0100 Subject: [PATCH 2/2] formatting --- .../DefaultApacheHttpClient5FactoryTest.java | 498 +++++++++--------- 1 file changed, 256 insertions(+), 242 deletions(-) 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 e642fc536..80a9e6e01 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 @@ -53,269 +53,283 @@ import lombok.SneakyThrows; -class DefaultApacheHttpClient5FactoryTest { - private static final int TEST_TIMEOUT = 300_000; - private static final Duration CLIENT_TIMEOUT = Duration.ofSeconds(10L); - private static final int MAX_CONNECTIONS = 10; - private static final int MAX_CONNECTIONS_PER_ROUTE = 5; - - @RegisterExtension - static final WireMockExtension WIRE_MOCK_SERVER = - WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); - @RegisterExtension - static final WireMockExtension SECOND_WIRE_MOCK_SERVER = - WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); - - private SoftAssertions softly; - private ApacheHttpClient5Factory sut; - private HttpRequestInterceptor requestInterceptor; - - @BeforeEach - @SneakyThrows - void setup() { - softly = new SoftAssertions(); - - requestInterceptor = mock(HttpRequestInterceptor.class); - doNothing().when(requestInterceptor).process(any(), any(), any()); - - sut = +class DefaultApacheHttpClient5FactoryTest +{ + private static final int TEST_TIMEOUT = 300_000; + private static final Duration CLIENT_TIMEOUT = Duration.ofSeconds(10L); + private static final int MAX_CONNECTIONS = 10; + private static final int MAX_CONNECTIONS_PER_ROUTE = 5; + + @RegisterExtension + static final WireMockExtension WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + @RegisterExtension + static final WireMockExtension SECOND_WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + + private SoftAssertions softly; + private ApacheHttpClient5Factory sut; + private HttpRequestInterceptor requestInterceptor; + + @BeforeEach + @SneakyThrows + void setup() + { + softly = new SoftAssertions(); + + requestInterceptor = mock(HttpRequestInterceptor.class); + doNothing().when(requestInterceptor).process(any(), any(), any()); + + sut = new DefaultApacheHttpClient5Factory( - CLIENT_TIMEOUT, - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); - } - - @Test - @SneakyThrows - void testHttpClientUsesTimeout() { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/timeout")).willReturn(ok().withFixedDelay(5_000))); - - final ApacheHttpClient5Factory factoryWithTooLittleTimeout = + CLIENT_TIMEOUT, + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); + } + + @Test + @SneakyThrows + void testHttpClientUsesTimeout() + { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/timeout")).willReturn(ok().withFixedDelay(5_000))); + + final ApacheHttpClient5Factory factoryWithTooLittleTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(3L), + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); - final ApacheHttpClient5Factory factoryWithEnoughTimeout = + final ApacheHttpClient5Factory factoryWithEnoughTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(7L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(7L), + MAX_CONNECTIONS, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); - final ClassicHttpRequest request = new HttpGet(WIRE_MOCK_SERVER.url("/timeout")); + final ClassicHttpRequest request = new HttpGet(WIRE_MOCK_SERVER.url("/timeout")); - final HttpClient clientWithTooLittleTimeout = factoryWithTooLittleTimeout.createHttpClient(); - assertThatThrownBy(() -> clientWithTooLittleTimeout.execute(request, ignoreResponse())) + final HttpClient clientWithTooLittleTimeout = factoryWithTooLittleTimeout.createHttpClient(); + assertThatThrownBy(() -> clientWithTooLittleTimeout.execute(request, ignoreResponse())) .isInstanceOf(IOException.class) .hasMessageContaining("Read timed out"); - final HttpClient clientWithEnoughTimeout = factoryWithEnoughTimeout.createHttpClient(); - clientWithEnoughTimeout.execute(request, assertOk()); + final HttpClient clientWithEnoughTimeout = factoryWithEnoughTimeout.createHttpClient(); + clientWithEnoughTimeout.execute(request, assertOk()); - softly.assertAll(); - } + softly.assertAll(); + } - @Test - @Timeout(value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS) - @SneakyThrows - void testHttpClientUsesMaxConnections() { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-1")).willReturn(ok())); - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-2")).willReturn(ok())); + @Test + @Timeout( value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS ) + @SneakyThrows + void testHttpClientUsesMaxConnections() + { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-1")).willReturn(ok())); + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-2")).willReturn(ok())); - final ApacheHttpClient5Factory sut = + final ApacheHttpClient5Factory sut = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - 1, - MAX_CONNECTIONS_PER_ROUTE, - requestInterceptor, - AUTOMATIC); - - final HttpClient client = sut.createHttpClient(); - final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-1")); - final ClassicHttpRequest secondRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-2")); - - assertCannotBeExecutedInParallel(firstRequest, secondRequest, client); - } - - @Test - @Timeout(value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS) - @SneakyThrows - void testHttpClientUsesMaxConnectionsPerRoute() { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); - SECOND_WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); - - final ApacheHttpClient5Factory sut = - new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - MAX_CONNECTIONS, - 1, - requestInterceptor, - AUTOMATIC); + Duration.ofSeconds(3L), // this timeout is also used for the connection lease + 1, + MAX_CONNECTIONS_PER_ROUTE, + requestInterceptor, + AUTOMATIC); - final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-per-route")); - final ClassicHttpRequest secondRequest = new HttpGet(SECOND_WIRE_MOCK_SERVER.url("/max-connections-per-route")); - final HttpClient client = sut.createHttpClient(); + final HttpClient client = sut.createHttpClient(); + final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-1")); + final ClassicHttpRequest secondRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-2")); - assertCanBeExecutedInParallel(firstRequest, secondRequest, client); - assertCannotBeExecutedInParallel(firstRequest, firstRequest, client); - } + assertCannotBeExecutedInParallel(firstRequest, secondRequest, client); + } - @Test - @SneakyThrows - void testProxyConfigurationIsConsidered() { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/proxy")).willReturn(ok())); + @Test + @Timeout( value = TEST_TIMEOUT, unit = TimeUnit.MILLISECONDS ) + @SneakyThrows + void testHttpClientUsesMaxConnectionsPerRoute() + { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); + SECOND_WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); - final BasicCredentials credentials = new BasicCredentials("user", "pass"); + final ApacheHttpClient5Factory sut = + new DefaultApacheHttpClient5Factory( + Duration.ofSeconds(3L), // this timeout is also used for the connection lease + MAX_CONNECTIONS, + 1, + requestInterceptor, + AUTOMATIC); + + final ClassicHttpRequest firstRequest = new HttpGet(WIRE_MOCK_SERVER.url("/max-connections-per-route")); + final ClassicHttpRequest secondRequest = new HttpGet(SECOND_WIRE_MOCK_SERVER.url("/max-connections-per-route")); + final HttpClient client = sut.createHttpClient(); + + assertCanBeExecutedInParallel(firstRequest, secondRequest, client); + assertCannotBeExecutedInParallel(firstRequest, firstRequest, client); + } - final DefaultHttpDestination destination = + @Test + @SneakyThrows + void testProxyConfigurationIsConsidered() + { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/proxy")).willReturn(ok())); + + final BasicCredentials credentials = new BasicCredentials("user", "pass"); + + final DefaultHttpDestination destination = DefaultHttpDestination - .builder("http://www.sap.com") - .proxyConfiguration(ProxyConfiguration.of(WIRE_MOCK_SERVER.baseUrl(), credentials)) - .build(); - final DefaultHttpDestination spiedDestination = spy(destination); - - final HttpClient httpClient = sut.createHttpClient(spiedDestination); - Mockito.verify(spiedDestination, atLeastOnce()).getProxyConfiguration(); - - doAnswer(invocation -> { - final HttpRequest request = invocation.getArgument(0); - final HttpClientContext context = invocation.getArgument(2); - - assertThat(request.getUri()).isEqualTo(URI.create("http://www.sap.com/proxy")); - assertThat(Arrays.stream(request.getHeaders()).map(NameValuePair::getName).collect(Collectors.toSet())) - .containsExactlyInAnyOrder(HttpHeaders.ACCEPT_ENCODING, HttpHeaders.PROXY_AUTHORIZATION); - assertThat(Arrays.toString(request.getHeaders(HttpHeaders.PROXY_AUTHORIZATION))) - .contains(credentials.getHttpHeaderValue()); - - final RouteInfo httpRoute = context.getHttpRoute(); - assertThat(httpRoute).isNotNull(); - assertThat(httpRoute.getHopCount()).isEqualTo(2); - assertThat(httpRoute.getHopTarget(0)).isEqualTo(HttpHost.create(WIRE_MOCK_SERVER.baseUrl())); - assertThat(httpRoute.getHopTarget(1)).isEqualTo(HttpHost.create("http://www.sap.com:80")); - - return null; - }).when(requestInterceptor).process(any(), any(), any()); - - try (final ClassicHttpResponse response = httpClient.execute(new HttpGet("/proxy"), r -> r)) { - WIRE_MOCK_SERVER.verify(getRequestedFor(urlEqualTo("/proxy"))); - assertThat(response.getCode()).isEqualTo(HttpStatus.SC_OK); + .builder("http://www.sap.com") + .proxyConfiguration(ProxyConfiguration.of(WIRE_MOCK_SERVER.baseUrl(), credentials)) + .build(); + final DefaultHttpDestination spiedDestination = spy(destination); + + final HttpClient httpClient = sut.createHttpClient(spiedDestination); + Mockito.verify(spiedDestination, atLeastOnce()).getProxyConfiguration(); + + doAnswer(invocation -> { + final HttpRequest request = invocation.getArgument(0); + final HttpClientContext context = invocation.getArgument(2); + + assertThat(request.getUri()).isEqualTo(URI.create("http://www.sap.com/proxy")); + assertThat(Arrays.stream(request.getHeaders()).map(NameValuePair::getName).collect(Collectors.toSet())) + .containsExactlyInAnyOrder(HttpHeaders.ACCEPT_ENCODING, HttpHeaders.PROXY_AUTHORIZATION); + assertThat(Arrays.toString(request.getHeaders(HttpHeaders.PROXY_AUTHORIZATION))) + .contains(credentials.getHttpHeaderValue()); + + final RouteInfo httpRoute = context.getHttpRoute(); + assertThat(httpRoute).isNotNull(); + assertThat(httpRoute.getHopCount()).isEqualTo(2); + assertThat(httpRoute.getHopTarget(0)).isEqualTo(HttpHost.create(WIRE_MOCK_SERVER.baseUrl())); + assertThat(httpRoute.getHopTarget(1)).isEqualTo(HttpHost.create("http://www.sap.com:80")); + + return null; + }).when(requestInterceptor).process(any(), any(), any()); + + try( final ClassicHttpResponse response = httpClient.execute(new HttpGet("/proxy"), r -> r) ) { + WIRE_MOCK_SERVER.verify(getRequestedFor(urlEqualTo("/proxy"))); + assertThat(response.getCode()).isEqualTo(HttpStatus.SC_OK); + } } - } - - @Test - @SneakyThrows - void verifyDefaultRetryMechanism() { - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/too-many-requests")) - .willReturn(aResponse().withStatus(429))); - WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/temporary-error")) - .willReturn(aResponse().withStatus(503))); - - final HttpClient client = sut.createHttpClient(); - client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/too-many-requests")), ignored -> ignored); - client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/temporary-error")), ignored -> ignored); - - WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/too-many-requests"))); - WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/temporary-error"))); - } - - @SneakyThrows - private void assertCannotBeExecutedInParallel( - @Nonnull final ClassicHttpRequest firstRequest, - @Nonnull final ClassicHttpRequest secondRequest, - @Nonnull final HttpClient client) { - final CountDownLatch firstResponseReceived = new CountDownLatch(1); - final CountDownLatch secondRequestFailed = new CountDownLatch(1); - - final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(firstRequest, r -> { - firstResponseReceived.countDown(); - try { - secondRequestFailed.await(); - } catch (final InterruptedException e) { - softly.fail("Interrupted while waiting for request to be sent", e); - } - return null; + + @Test + @SneakyThrows + void verifyDefaultRetryMechanism() + { + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/too-many-requests")).willReturn(aResponse().withStatus(429))); + WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/temporary-error")).willReturn(aResponse().withStatus(503))); + + final HttpClient client = sut.createHttpClient(); + client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/too-many-requests")), ignored -> ignored); + client.execute(new HttpGet(WIRE_MOCK_SERVER.url("/temporary-error")), ignored -> ignored); + + WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/too-many-requests"))); + WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/temporary-error"))); + } + + @SneakyThrows + private void assertCannotBeExecutedInParallel( + @Nonnull final ClassicHttpRequest firstRequest, + @Nonnull final ClassicHttpRequest secondRequest, + @Nonnull final HttpClient client ) + { + final CountDownLatch firstResponseReceived = new CountDownLatch(1); + final CountDownLatch secondRequestFailed = new CountDownLatch(1); + + final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(firstRequest, r -> { + firstResponseReceived.countDown(); + try { + secondRequestFailed.await(); + } + catch( final InterruptedException e ) { + softly.fail("Interrupted while waiting for request to be sent", e); + } + return null; + }); + } + catch( final IOException e ) { + softly.fail("Failed to execute request", e); + } }); - } catch (final IOException e) { - softly.fail("Failed to execute request", e); - } - }); - - firstResponseReceived.await(); - final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { - softly - .assertThatThrownBy(() -> client.execute(secondRequest, ignoreResponse())) - .isExactlyInstanceOf(ConnectionRequestTimeoutException.class); - secondRequestFailed.countDown(); - }); - - secondRequestFailed.await(); - - firstFuture.get(); - secondFuture.get(); - softly.assertAll(); - } - - @SneakyThrows - private void assertCanBeExecutedInParallel( - @Nonnull final ClassicHttpRequest firstRequest, - @Nonnull final ClassicHttpRequest secondRequest, - @Nonnull final HttpClient client) { - final CountDownLatch firstResponseReceived = new CountDownLatch(1); - final CountDownLatch secondResponseReceived = new CountDownLatch(1); - - final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(firstRequest, r -> { - firstResponseReceived.countDown(); - try { - secondResponseReceived.await(); - } catch (final InterruptedException e) { - softly.fail("Interrupted while waiting for request to be sent", e); - } - return null; + + firstResponseReceived.await(); + final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { + softly + .assertThatThrownBy(() -> client.execute(secondRequest, ignoreResponse())) + .isExactlyInstanceOf(ConnectionRequestTimeoutException.class); + secondRequestFailed.countDown(); }); - } catch (final IOException e) { - softly.fail("Failed to execute request", e); - } - }); - - firstResponseReceived.await(); - final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { - try { - client.execute(secondRequest, r -> { - secondResponseReceived.countDown(); - return null; + + secondRequestFailed.await(); + + firstFuture.get(); + secondFuture.get(); + softly.assertAll(); + } + + @SneakyThrows + private void assertCanBeExecutedInParallel( + @Nonnull final ClassicHttpRequest firstRequest, + @Nonnull final ClassicHttpRequest secondRequest, + @Nonnull final HttpClient client ) + { + final CountDownLatch firstResponseReceived = new CountDownLatch(1); + final CountDownLatch secondResponseReceived = new CountDownLatch(1); + + final CompletableFuture firstFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(firstRequest, r -> { + firstResponseReceived.countDown(); + try { + secondResponseReceived.await(); + } + catch( final InterruptedException e ) { + softly.fail("Interrupted while waiting for request to be sent", e); + } + return null; + }); + } + catch( final IOException e ) { + softly.fail("Failed to execute request", e); + } + }); + + firstResponseReceived.await(); + final CompletableFuture secondFuture = CompletableFuture.runAsync(() -> { + try { + client.execute(secondRequest, r -> { + secondResponseReceived.countDown(); + return null; + }); + } + catch( final IOException e ) { + softly.fail("Failed to execute request", e); + } }); - } catch (final IOException e) { - softly.fail("Failed to execute request", e); - } - }); - - secondResponseReceived.await(); - - firstFuture.get(); - secondFuture.get(); - softly.assertAll(); - } - - @Nonnull - private HttpClientResponseHandler ignoreResponse() { - return r -> null; - } - - @Nonnull - private HttpClientResponseHandler assertOk() { - return r -> { - softly.assertThat(r.getCode()).isEqualTo(HttpStatus.SC_OK); - return null; - }; - } + + secondResponseReceived.await(); + + firstFuture.get(); + secondFuture.get(); + softly.assertAll(); + } + + @Nonnull + private HttpClientResponseHandler ignoreResponse() + { + return r -> null; + } + + @Nonnull + private HttpClientResponseHandler assertOk() + { + return r -> { + softly.assertThat(r.getCode()).isEqualTo(HttpStatus.SC_OK); + return null; + }; + } }