Disable Client based retries and define httpcore5 httpclient5 in .brazil.json (#6191)

* Fix architecture test failures for apache5.x

* Checkstyle issues

* Update to use PoolingHttpClientConnectionManager class reference that is implementation of HttpClientConnectionManager

* Fix stream reset failure in RepeatableInputStreamRequestEntity by storing content reference to avoid multiple ContentStreamProvider.newStream() calls that cause IOException when retrying requests with non-resettable streams

* writeTo_ConcurrentWrites_HandlesCorrectly no longer needed since even Apache 4.x doesnot suports this

* Fix connectionPoolingWorks by setting  skipping setConnectionTimeToLive is value is set to 0 since 0 is treated as Infinite timeToLive in Sdk and Apache 4.x but treated as immediate closeConnection in apache 5.x

* disableAutomaticRetries in Apache 5.x since SDK handles retries , also define Apache5 dependencies in .brazil.json

* Added Test case for Async , handled review ocmments

Author: joviegas

.brazil.json

@@ -141,7 +141,9 @@
         "io.netty:netty-transport-classes-epoll": { "packageName": "Netty4", "packageVersion": "4.1" },
         "io.netty:netty-transport-native-unix-common": { "packageName": "Netty4", "packageVersion": "4.1" },
         "org.apache.httpcomponents:httpclient": { "packageName": "Apache-HttpComponents-HttpClient", "packageVersion": "4.5.x" },
+        "org.apache.httpcomponents.client5:httpclient5": { "packageName": "Apache-HttpComponents-HttpClient5", "packageVersion": "5.0.x" },
         "org.apache.httpcomponents:httpcore": { "packageName": "Apache-HttpComponents-HttpCore", "packageVersion": "4.4.x" },
+        "org.apache.httpcomponents.core5:httpcore5": { "packageName": "Apache-HttpComponents-HttpCore5", "packageVersion": "5.0.x" },
         "org.eclipse.jdt:org.eclipse.jdt.core": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" },
         "org.eclipse.text:org.eclipse.text": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" },
         "org.reactivestreams:reactive-streams": { "packageName": "Maven-org-reactivestreams_reactive-streams", "packageVersion": "1.x" },

http-clients/apache5-client/src/main/java/software/amazon/awssdk/http/apache5/Apache5HttpClient.java

@@ -166,7 +166,9 @@ private ConnectionManagerAwareHttpClient createClient(Apache5HttpClient.DefaultB
                .setUserAgent("") // SDK will set the user agent header in the pipeline. Don't let Apache waste time
                .setConnectionManager(ClientConnectionManagerFactory.wrap(cm))
                //This is done to keep backward compatibility with Apache 4.x
-               .disableRedirectHandling();
+               .disableRedirectHandling()
+               // SDK handles retries , we do not need additional retries on Http clients.
+               .disableAutomaticRetries();
 
         addProxyConfig(builder, configuration);
 

test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkAsyncHttpClientH1TestSuite.java

@@ -172,6 +172,26 @@ public void connectionsArePooledByHostAndPort() throws InterruptedException {
 
     }
 
+    @Test
+    public void doesNotRetryOn429StatusCode() throws InterruptedException {
+        server.return429OnFirstRequest = true;
+        server.closeConnection = false;
+
+        HttpTestUtils.sendGetRequest(server.port(), client).join();
+        // Wait to ensure no retries happen
+        Thread.sleep(100);
+
+        // Verify only one request was made (no retries)
+        assertThat(server.requestCount).isEqualTo(1);
+
+        // Send second request to verify connection reuse works after 429
+        HttpTestUtils.sendGetRequest(server.port(), client).join();
+
+        // Verify connection was reused and total of 2 requests
+        assertThat(server.channels.size()).isEqualTo(1);
+        assertThat(server.requestCount).isEqualTo(2);
+    }
+
     private static class Server extends ChannelInitializer<Channel> {
         private static final byte[] CONTENT = "helloworld".getBytes(StandardCharsets.UTF_8);
         private ServerBootstrap bootstrap;
@@ -181,6 +201,9 @@ private static class Server extends ChannelInitializer<Channel> {
         private SslContext sslCtx;
         private boolean return500OnFirstRequest;
         private boolean closeConnection;
+        private boolean return429OnFirstRequest;
+        private volatile int requestCount = 0;
+
 
         public void init() throws Exception {
             SelfSignedCertificate ssc = new SelfSignedCertificate();
@@ -218,10 +241,14 @@ private class BehaviorTestChannelHandler extends ChannelDuplexHandler {
             @Override
             public void channelRead(ChannelHandlerContext ctx, Object msg) {
                 if (msg instanceof HttpRequest) {
+                    requestCount++;
 
                     HttpResponseStatus status;
                     if (ctx.channel().equals(channels.get(0)) && return500OnFirstRequest) {
                         status = INTERNAL_SERVER_ERROR;
+                    } else if (ctx.channel().equals(channels.get(0)) && return429OnFirstRequest) {
+                        status = HttpResponseStatus.TOO_MANY_REQUESTS;
+                        return429OnFirstRequest = false; // Reset after first use
                     } else {
                         status = OK;
                     }

test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java

@@ -20,6 +20,8 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.any;
 import static com.github.tomakehurst.wiremock.client.WireMock.containing;
 import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
 import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
@@ -59,6 +61,7 @@ public abstract class SdkHttpClientTestSuite {
     private static final Logger LOG = Logger.loggerFor(SdkHttpClientTestSuite.class);
 
     private static final ConnectionCountingTrafficListener CONNECTION_COUNTER = new ConnectionCountingTrafficListener();
+    private static final int HTTP_TOO_MANY_REQUESTS = 429;
 
     @Rule
     public WireMockRule mockServer = createWireMockRule();
@@ -218,6 +221,48 @@ public void testCustomTlsTrustManagerAndTrustAllFails() throws Exception {
         assertThatThrownBy(() -> createSdkHttpClient(httpClientOptions)).isInstanceOf(IllegalArgumentException.class);
     }
 
+    @Test
+    public void doesNotRetryOn429StatusCode() throws Exception {
+        SdkHttpClientOptions httpClientOptions = new SdkHttpClientOptions();
+        httpClientOptions.trustAll(true);
+
+        try (SdkHttpClient client = createSdkHttpClient(httpClientOptions)) {
+            // Test 429 with no retry
+            validateStatusCodeWithRetryCheck(client, HTTP_TOO_MANY_REQUESTS, 1);
+
+            // Reset and test normal request works
+            mockServer.resetAll();
+            validateStatusCodeWithRetryCheck(client, HttpURLConnection.HTTP_OK, 1);
+        }
+    }
+
+    private void validateStatusCodeWithRetryCheck(SdkHttpClient client,
+                                                  int expectedStatusCode,
+                                                  int expectedRequestCount) throws IOException {
+        stubForMockRequest(expectedStatusCode);
+        SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
+        HttpExecuteResponse response = client.prepareRequest(
+                                                 HttpExecuteRequest.builder()
+                                                                   .request(request)
+                                                                   .contentStreamProvider(request.contentStreamProvider()
+                                                                                                 .orElse(null))
+                                                                   .build())
+                                             .call();
+        validateResponseStatusCode(response, expectedStatusCode);
+        verifyRequestCount(expectedRequestCount);
+    }
+
+    private void verifyRequestCount(int expectedCount) {
+        mockServer.verify(expectedCount,
+                          postRequestedFor(urlEqualTo("/"))
+                              .withHeader("Host", containing("localhost")));
+    }
+
+    private void validateResponseStatusCode(HttpExecuteResponse response, int expectedStatusCode) throws IOException {
+        response.responseBody().ifPresent(IoUtils::drainInputStream);
+        assertThat(response.httpResponse().statusCode()).isEqualTo(expectedStatusCode);
+    }
+
     protected void testForResponseCode(int returnCode) throws Exception {
         testForResponseCode(returnCode, SdkHttpMethod.POST);
     }