Skip to content

Commit 95f389d

Browse files
joviegasjoviegas
committed
Updated test cases and changes the Exception handling when HeadBucket Call fails
1 parent 9cc4bdf commit 95f389d

File tree

6 files changed

+207
-47
lines changed

6 files changed

+207
-47
lines changed

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClient.java

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@
2626
import java.util.function.Function;
2727
import java.util.function.Supplier;
2828
import software.amazon.awssdk.annotations.SdkInternalApi;
29+
import software.amazon.awssdk.awscore.exception.AwsServiceException;
2930
import software.amazon.awssdk.regions.Region;
3031
import software.amazon.awssdk.services.s3.DelegatingS3AsyncClient;
3132
import software.amazon.awssdk.services.s3.S3AsyncClient;
3233
import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
3334
import software.amazon.awssdk.services.s3.model.S3Exception;
3435
import software.amazon.awssdk.services.s3.model.S3Request;
3536
import software.amazon.awssdk.utils.CompletableFutureUtils;
36-
import software.amazon.awssdk.utils.StringUtils;
3737

3838
@SdkInternalApi
3939
public final class S3CrossRegionAsyncClient extends DelegatingS3AsyncClient {
@@ -79,31 +79,24 @@ protected <T extends S3Request, ReturnT> CompletableFuture<ReturnT> invokeOperat
7979

8080

8181
private Supplier<Region> regionSupplier(String bucket) {
82-
return () -> bucketToRegionCache.computeIfAbsent(bucket, this::regionCompletableFuture).join();
82+
CompletableFuture<Region> completableFuture = bucketToRegionCache.computeIfAbsent(bucket, this::regionCompletableFuture);
83+
return () -> completableFuture.join();
8384
}
8485

8586
private CompletableFuture<Region> regionCompletableFuture(String bucketName) {
86-
StringBuilder stringBuilder = new StringBuilder();
87-
return CompletableFuture.supplyAsync(
88-
() -> ((S3AsyncClient) delegate()).headBucket(HeadBucketRequest.builder()
89-
.bucket(bucketName)
90-
.build())
91-
.exceptionally(exception -> {
92-
if (isS3RedirectException(exception.getCause())) {
93-
getBucketRegionFromException(
94-
(S3Exception) exception.getCause()).ifPresent(
95-
stringBuilder::append);
96-
} else {
97-
CompletableFutureUtils.failedFuture(exception);
98-
}
99-
return null;
100-
}))
101-
.thenApplyAsync(headResponse -> {
102-
headResponse.join();
103-
if (headResponse != null && StringUtils.isNotBlank(stringBuilder.toString())) {
104-
return Region.of(stringBuilder.toString());
105-
}
106-
return null;
107-
});
87+
return CompletableFuture.supplyAsync(() -> {
88+
try {
89+
((S3AsyncClient) delegate()).headBucket(HeadBucketRequest.builder().bucket(bucketName).build()).join();
90+
} catch (Exception exception) {
91+
if (isS3RedirectException(exception.getCause())) {
92+
String region = getBucketRegionFromException((S3Exception) exception.getCause())
93+
.orElseThrow(() -> AwsServiceException.create("Region name not found in Redirect error",
94+
exception));
95+
return Region.of(region);
96+
}
97+
throw exception;
98+
}
99+
return ((S3AsyncClient) delegate()).serviceClientConfiguration().region();
100+
});
108101
}
109102
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import java.util.concurrent.CompletableFuture;
2525
import java.util.concurrent.CompletionException;
26+
import java.util.function.Consumer;
2627
import org.junit.jupiter.api.BeforeEach;
2728
import org.mockito.ArgumentCaptor;
2829
import org.mockito.Mockito;
@@ -101,7 +102,9 @@ protected void stubApiWithNoBucketField() {
101102
@Override
102103
protected void stubHeadBucketRedirect() {
103104
when(mockDelegateAsyncClient.headBucket(any(HeadBucketRequest.class)))
104-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(301, CROSS_REGION, null, null))));
105+
.thenThrow(new CompletionException(redirectException(301,CROSS_REGION, null, null)));
106+
when(mockDelegateAsyncClient.headBucket(any(Consumer.class)))
107+
.thenThrow(new CompletionException(redirectException(301,CROSS_REGION, null, null)));
105108
}
106109

107110
@Override

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616
package software.amazon.awssdk.services.s3.internal.crossregion;
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
1920
import static software.amazon.awssdk.services.s3.internal.crossregion.S3DecoratorRedirectBaseTest.CROSS_REGION;
2021
import static software.amazon.awssdk.services.s3.internal.crossregion.S3DecoratorRedirectBaseTest.X_AMZ_BUCKET_REGION;
2122

2223
import java.net.URI;
2324
import java.util.Arrays;
2425
import java.util.List;
2526
import java.util.concurrent.CompletableFuture;
27+
import java.util.concurrent.CompletionException;
2628
import java.util.function.Consumer;
2729
import java.util.stream.Collectors;
2830
import java.util.stream.Stream;
@@ -32,10 +34,12 @@
3234
import org.junit.jupiter.params.provider.Arguments;
3335
import org.junit.jupiter.params.provider.MethodSource;
3436
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
37+
import software.amazon.awssdk.core.exception.SdkClientException;
3538
import software.amazon.awssdk.core.interceptor.Context;
3639
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
3740
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
3841
import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
42+
import software.amazon.awssdk.endpoints.Endpoint;
3943
import software.amazon.awssdk.endpoints.EndpointProvider;
4044
import software.amazon.awssdk.http.AbortableInputStream;
4145
import software.amazon.awssdk.http.HttpExecuteResponse;
@@ -47,10 +51,13 @@
4751
import software.amazon.awssdk.services.s3.S3AsyncClient;
4852
import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
4953
import software.amazon.awssdk.services.s3.S3Client;
54+
import software.amazon.awssdk.services.s3.endpoints.S3EndpointParams;
55+
import software.amazon.awssdk.services.s3.endpoints.S3EndpointProvider;
5056
import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointProvider;
5157
import software.amazon.awssdk.services.s3.internal.crossregion.endpointprovider.BucketEndpointProvider;
5258
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
5359
import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
60+
import software.amazon.awssdk.services.s3.model.S3Exception;
5461
import software.amazon.awssdk.services.s3.paginators.ListObjectsV2Publisher;
5562
import software.amazon.awssdk.testutils.service.http.MockAsyncHttpClient;
5663
import software.amazon.awssdk.utils.StringInputStream;
@@ -174,6 +181,57 @@ void crossRegionClient_CallsHeadObject_when_regionNameNotPresentInFallBackCall()
174181

175182
}
176183

184+
@Test
185+
void crossRegionClient_CallsHeadObjectErrors_shouldTerminateTheAPI() {
186+
mockAsyncHttpClient.stubResponses(customHttpResponse(301, null ),
187+
customHttpResponse(400, null ),
188+
successHttpResponse(), successHttpResponse());
189+
S3AsyncClient crossRegionClient =
190+
clientBuilder().endpointOverride(null).region(Region.US_WEST_2)
191+
.serviceConfiguration(c -> c.crossRegionAccessEnabled(true)).build();
192+
193+
assertThatExceptionOfType(CompletionException.class)
194+
.isThrownBy(() -> crossRegionClient.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join())
195+
.withMessageContaining("Endpoint resolution failed");
196+
197+
List<SdkHttpRequest> requests = mockAsyncHttpClient.getRequests();
198+
assertThat(requests).hasSize(2);
199+
200+
assertThat(requests.stream().map(req -> req.host().substring(10,req.host().length() - 14 )).collect(Collectors.toList()))
201+
.isEqualTo(Arrays.asList(Region.US_WEST_2.toString(),
202+
Region.US_WEST_2.toString()));
203+
204+
assertThat(requests.stream().map(req -> req.method()).collect(Collectors.toList()))
205+
.isEqualTo(Arrays.asList(SdkHttpMethod.GET,
206+
SdkHttpMethod.HEAD));
207+
}
208+
209+
@Test
210+
void crossRegionClient_CallsHeadObjectWithNoRegion_shouldTerminateHeadBucketAPI() {
211+
mockAsyncHttpClient.stubResponses(customHttpResponse(301, null ),
212+
customHttpResponse(301, null ),
213+
successHttpResponse(), successHttpResponse());
214+
S3AsyncClient crossRegionClient =
215+
clientBuilder().endpointOverride(null).region(Region.US_WEST_2)
216+
.serviceConfiguration(c -> c.crossRegionAccessEnabled(true)).build();
217+
218+
assertThatExceptionOfType(CompletionException.class)
219+
.isThrownBy(() -> crossRegionClient.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join())
220+
.withMessageContaining("Endpoint resolution failed")
221+
.withCauseInstanceOf(SdkClientException.class).withRootCauseExactlyInstanceOf(S3Exception.class);
222+
223+
List<SdkHttpRequest> requests = mockAsyncHttpClient.getRequests();
224+
assertThat(requests).hasSize(2);
225+
226+
assertThat(requests.stream().map(req -> req.host().substring(10,req.host().length() - 14 )).collect(Collectors.toList()))
227+
.isEqualTo(Arrays.asList(Region.US_WEST_2.toString(),
228+
Region.US_WEST_2.toString()));
229+
230+
assertThat(requests.stream().map(req -> req.method()).collect(Collectors.toList()))
231+
.isEqualTo(Arrays.asList(SdkHttpMethod.GET,
232+
SdkHttpMethod.HEAD));
233+
}
234+
177235
private S3AsyncClientBuilder clientBuilder() {
178236
return S3AsyncClient.builder()
179237
.httpClient(mockAsyncHttpClient)

0 commit comments

Comments
 (0)