Skip to content

Commit ad30c25

Browse files
committed
Merge branch 'feature/master/apache5x' of github.com:aws/aws-sdk-java-v2 into feature/master/apache5x
2 parents 09e8ee4 + fcbc3c0 commit ad30c25

File tree

4 files changed

+77
-1
lines changed

4 files changed

+77
-1
lines changed

.brazil.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,9 @@
141141
"io.netty:netty-transport-classes-epoll": { "packageName": "Netty4", "packageVersion": "4.1" },
142142
"io.netty:netty-transport-native-unix-common": { "packageName": "Netty4", "packageVersion": "4.1" },
143143
"org.apache.httpcomponents:httpclient": { "packageName": "Apache-HttpComponents-HttpClient", "packageVersion": "4.5.x" },
144+
"org.apache.httpcomponents.client5:httpclient5": { "packageName": "Apache-HttpComponents-HttpClient5", "packageVersion": "5.0.x" },
144145
"org.apache.httpcomponents:httpcore": { "packageName": "Apache-HttpComponents-HttpCore", "packageVersion": "4.4.x" },
146+
"org.apache.httpcomponents.core5:httpcore5": { "packageName": "Apache-HttpComponents-HttpCore5", "packageVersion": "5.0.x" },
145147
"org.eclipse.jdt:org.eclipse.jdt.core": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" },
146148
"org.eclipse.text:org.eclipse.text": { "packageName": "AwsJavaSdk-Codegen-EclipseJdtDependencies", "packageVersion": "2.0" },
147149
"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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ private ConnectionManagerAwareHttpClient createClient(Apache5HttpClient.DefaultB
166166
.setUserAgent("") // SDK will set the user agent header in the pipeline. Don't let Apache waste time
167167
.setConnectionManager(ClientConnectionManagerFactory.wrap(cm))
168168
//This is done to keep backward compatibility with Apache 4.x
169-
.disableRedirectHandling();
169+
.disableRedirectHandling()
170+
// SDK handles retries , we do not need additional retries on Http clients.
171+
.disableAutomaticRetries();
170172

171173
addProxyConfig(builder, configuration);
172174

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,26 @@ public void connectionsArePooledByHostAndPort() throws InterruptedException {
172172

173173
}
174174

175+
@Test
176+
public void doesNotRetryOn429StatusCode() throws InterruptedException {
177+
server.return429OnFirstRequest = true;
178+
server.closeConnection = false;
179+
180+
HttpTestUtils.sendGetRequest(server.port(), client).join();
181+
// Wait to ensure no retries happen
182+
Thread.sleep(100);
183+
184+
// Verify only one request was made (no retries)
185+
assertThat(server.requestCount).isEqualTo(1);
186+
187+
// Send second request to verify connection reuse works after 429
188+
HttpTestUtils.sendGetRequest(server.port(), client).join();
189+
190+
// Verify connection was reused and total of 2 requests
191+
assertThat(server.channels.size()).isEqualTo(1);
192+
assertThat(server.requestCount).isEqualTo(2);
193+
}
194+
175195
private static class Server extends ChannelInitializer<Channel> {
176196
private static final byte[] CONTENT = "helloworld".getBytes(StandardCharsets.UTF_8);
177197
private ServerBootstrap bootstrap;
@@ -181,6 +201,9 @@ private static class Server extends ChannelInitializer<Channel> {
181201
private SslContext sslCtx;
182202
private boolean return500OnFirstRequest;
183203
private boolean closeConnection;
204+
private boolean return429OnFirstRequest;
205+
private volatile int requestCount = 0;
206+
184207

185208
public void init() throws Exception {
186209
SelfSignedCertificate ssc = new SelfSignedCertificate();
@@ -218,10 +241,14 @@ private class BehaviorTestChannelHandler extends ChannelDuplexHandler {
218241
@Override
219242
public void channelRead(ChannelHandlerContext ctx, Object msg) {
220243
if (msg instanceof HttpRequest) {
244+
requestCount++;
221245

222246
HttpResponseStatus status;
223247
if (ctx.channel().equals(channels.get(0)) && return500OnFirstRequest) {
224248
status = INTERNAL_SERVER_ERROR;
249+
} else if (ctx.channel().equals(channels.get(0)) && return429OnFirstRequest) {
250+
status = HttpResponseStatus.TOO_MANY_REQUESTS;
251+
return429OnFirstRequest = false; // Reset after first use
225252
} else {
226253
status = OK;
227254
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import static com.github.tomakehurst.wiremock.client.WireMock.any;
2121
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
2222
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
24+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
2325
import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching;
2426
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
2527
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
@@ -59,6 +61,7 @@ public abstract class SdkHttpClientTestSuite {
5961
private static final Logger LOG = Logger.loggerFor(SdkHttpClientTestSuite.class);
6062

6163
private static final ConnectionCountingTrafficListener CONNECTION_COUNTER = new ConnectionCountingTrafficListener();
64+
private static final int HTTP_TOO_MANY_REQUESTS = 429;
6265

6366
@Rule
6467
public WireMockRule mockServer = createWireMockRule();
@@ -218,6 +221,48 @@ public void testCustomTlsTrustManagerAndTrustAllFails() throws Exception {
218221
assertThatThrownBy(() -> createSdkHttpClient(httpClientOptions)).isInstanceOf(IllegalArgumentException.class);
219222
}
220223

224+
@Test
225+
public void doesNotRetryOn429StatusCode() throws Exception {
226+
SdkHttpClientOptions httpClientOptions = new SdkHttpClientOptions();
227+
httpClientOptions.trustAll(true);
228+
229+
try (SdkHttpClient client = createSdkHttpClient(httpClientOptions)) {
230+
// Test 429 with no retry
231+
validateStatusCodeWithRetryCheck(client, HTTP_TOO_MANY_REQUESTS, 1);
232+
233+
// Reset and test normal request works
234+
mockServer.resetAll();
235+
validateStatusCodeWithRetryCheck(client, HttpURLConnection.HTTP_OK, 1);
236+
}
237+
}
238+
239+
private void validateStatusCodeWithRetryCheck(SdkHttpClient client,
240+
int expectedStatusCode,
241+
int expectedRequestCount) throws IOException {
242+
stubForMockRequest(expectedStatusCode);
243+
SdkHttpFullRequest request = mockSdkRequest("http://localhost:" + mockServer.port(), SdkHttpMethod.POST);
244+
HttpExecuteResponse response = client.prepareRequest(
245+
HttpExecuteRequest.builder()
246+
.request(request)
247+
.contentStreamProvider(request.contentStreamProvider()
248+
.orElse(null))
249+
.build())
250+
.call();
251+
validateResponseStatusCode(response, expectedStatusCode);
252+
verifyRequestCount(expectedRequestCount);
253+
}
254+
255+
private void verifyRequestCount(int expectedCount) {
256+
mockServer.verify(expectedCount,
257+
postRequestedFor(urlEqualTo("/"))
258+
.withHeader("Host", containing("localhost")));
259+
}
260+
261+
private void validateResponseStatusCode(HttpExecuteResponse response, int expectedStatusCode) throws IOException {
262+
response.responseBody().ifPresent(IoUtils::drainInputStream);
263+
assertThat(response.httpResponse().statusCode()).isEqualTo(expectedStatusCode);
264+
}
265+
221266
protected void testForResponseCode(int returnCode) throws Exception {
222267
testForResponseCode(returnCode, SdkHttpMethod.POST);
223268
}

0 commit comments

Comments
 (0)