Skip to content

Commit 6d63b16

Browse files
author
Rishabh Jain
committed
test(s3): add regression test for onError cause preservation
Add ParallelMultipartDownloaderSubscriberTest verifying that onError completes the result future with the original throwable (not a CancellationException) and still cancels in-flight part requests. The test fails against the previous cancel-before-complete ordering. Restore the original onError comments.
1 parent af146f4 commit 6d63b16

2 files changed

Lines changed: 112 additions & 0 deletions

File tree

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelMultipartDownloaderSubscriber.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,9 @@ private boolean shouldProcessPendingTransformers() {
412412

413413
@Override
414414
public void onError(Throwable t) {
415+
// Signal received from the publisher this is subscribed to
416+
// (in the case of file download, that's FileAsyncResponseTransformerPublisher)
417+
// Failed state, something really wrong has happened, cancel everything
415418
// Complete the result future with the original cause before cancelling in-flight parts.
416419
// Cancelling first races a CancellationException onto resultFuture and masks t.
417420
log.debug(() -> "Error in parallel multipart download", t);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.s3.internal.multipart;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.mockito.ArgumentMatchers.any;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.when;
22+
23+
import java.util.concurrent.CancellationException;
24+
import java.util.concurrent.CompletableFuture;
25+
import org.junit.jupiter.api.BeforeEach;
26+
import org.junit.jupiter.api.Test;
27+
import org.reactivestreams.Subscription;
28+
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
29+
import software.amazon.awssdk.services.s3.S3AsyncClient;
30+
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
31+
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
32+
33+
/**
34+
* Unit tests for {@link ParallelMultipartDownloaderSubscriber}.
35+
*/
36+
class ParallelMultipartDownloaderSubscriberTest {
37+
38+
private static final int MAX_IN_FLIGHT_PARTS = 10;
39+
40+
private S3AsyncClient s3;
41+
private CompletableFuture<GetObjectResponse> resultFuture;
42+
private ParallelMultipartDownloaderSubscriber subscriber;
43+
44+
@BeforeEach
45+
void setup() {
46+
s3 = mock(S3AsyncClient.class);
47+
resultFuture = new CompletableFuture<>();
48+
GetObjectRequest request = GetObjectRequest.builder()
49+
.bucket("test-bucket")
50+
.key("test-key")
51+
.build();
52+
subscriber = new ParallelMultipartDownloaderSubscriber(s3, request, resultFuture, MAX_IN_FLIGHT_PARTS);
53+
}
54+
55+
@Test
56+
void onError_withInFlightRequests_shouldCompleteResultFutureWithOriginalCause() {
57+
CompletableFuture<GetObjectResponse> firstPartFuture = new CompletableFuture<>();
58+
CompletableFuture<GetObjectResponse> secondPartFuture = new CompletableFuture<>();
59+
when(s3.getObject(any(GetObjectRequest.class), any(AsyncResponseTransformer.class)))
60+
.thenReturn(firstPartFuture, secondPartFuture);
61+
62+
subscriber.onSubscribe(mock(Subscription.class));
63+
subscriber.onNext(mock(AsyncResponseTransformer.class));
64+
firstPartFuture.complete(GetObjectResponse.builder()
65+
.partsCount(3)
66+
.eTag("etag")
67+
.build());
68+
// Second part is now in flight and never completes on its own.
69+
subscriber.onNext(mock(AsyncResponseTransformer.class));
70+
71+
RuntimeException cause = new RuntimeException("original failure");
72+
subscriber.onError(cause);
73+
74+
assertThat(resultFuture).isCompletedExceptionally();
75+
Throwable thrown = resultFuture.handle((r, t) -> t).join();
76+
assertThat(thrown).isNotInstanceOf(CancellationException.class);
77+
assertThat(thrown).isSameAs(cause);
78+
}
79+
80+
@Test
81+
void onError_withoutInFlightRequests_shouldCompleteResultFutureWithOriginalCause() {
82+
RuntimeException cause = new RuntimeException("original failure");
83+
subscriber.onError(cause);
84+
85+
assertThat(resultFuture).isCompletedExceptionally();
86+
Throwable thrown = resultFuture.handle((r, t) -> t).join();
87+
assertThat(thrown).isSameAs(cause);
88+
}
89+
90+
@Test
91+
void onError_shouldCancelInFlightRequests() {
92+
CompletableFuture<GetObjectResponse> firstPartFuture = new CompletableFuture<>();
93+
CompletableFuture<GetObjectResponse> secondPartFuture = new CompletableFuture<>();
94+
when(s3.getObject(any(GetObjectRequest.class), any(AsyncResponseTransformer.class)))
95+
.thenReturn(firstPartFuture, secondPartFuture);
96+
97+
subscriber.onSubscribe(mock(Subscription.class));
98+
subscriber.onNext(mock(AsyncResponseTransformer.class));
99+
firstPartFuture.complete(GetObjectResponse.builder()
100+
.partsCount(3)
101+
.eTag("etag")
102+
.build());
103+
subscriber.onNext(mock(AsyncResponseTransformer.class));
104+
105+
subscriber.onError(new RuntimeException("original failure"));
106+
107+
assertThat(secondPartFuture).isCompletedExceptionally();
108+
}
109+
}

0 commit comments

Comments
 (0)