Skip to content

Fix presigned URL download progress tracking for range requests and add listener tests #6977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jencymaryjoseph wants to merge 1 commit into
base: feature/master/pre-signed-url-getobject
Choose a base branch
from
Open

Fix presigned URL download progress tracking for range requests and add listener tests #6977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions ...tware/amazon/awssdk/transfer/s3/S3TransferManagerPresignedUrlDownloadIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import software.amazon.awssdk.testutils.RandomTempFile;
import software.amazon.awssdk.transfer.s3.model.CompletedDownload;
import software.amazon.awssdk.transfer.s3.model.CompletedFileDownload;
import software.amazon.awssdk.transfer.s3.model.Download;
import software.amazon.awssdk.transfer.s3.model.FileDownload;
import software.amazon.awssdk.transfer.s3.model.PresignedDownloadFileRequest;
import software.amazon.awssdk.transfer.s3.model.PresignedDownloadRequest;
Expand Down Expand Up @@ -109,6 +110,75 @@ void downloadWithPresignedUrl_toBytes_shouldReturnCorrectData(S3TransferManager
assertThat(completed.result().asByteArray()).hasSize(objSize);
}

static Stream<Arguments> progressTestCases() {
return Stream.of(
Arguments.of("multipart", tmJava, LARGE_KEY, null, LARGE_OBJ_SIZE),
Arguments.of("multipart", tmJava, LARGE_KEY, "bytes=0-1048575", 1048576),
Arguments.of("nonMultipart", tmNonMultipartJava, LARGE_KEY, null, LARGE_OBJ_SIZE),
Arguments.of("nonMultipart", tmNonMultipartJava, LARGE_KEY, "bytes=0-1048575", 1048576)
);
}

@ParameterizedTest(name = "downloadFileWithPresignedUrl_progress_{0}_range={3}")
@MethodSource("progressTestCases")
void downloadFileWithPresignedUrl_progressTracking(String tmType, S3TransferManager tm, String key,
String range, int expectedSize) throws Exception {
Path downloadPath = RandomTempFile.randomUncreatedFile().toPath();

PresignedUrlDownloadRequest.Builder requestBuilder = PresignedUrlDownloadRequest.builder()
.presignedUrl(createPresignedRequest(key).url());
if (range != null) {
requestBuilder.range(range);
}

FileDownload download = tm.downloadFileWithPresignedUrl(
PresignedDownloadFileRequest.builder()
.presignedUrlDownloadRequest(requestBuilder.build())
.destination(downloadPath)
.addTransferListener(LoggingTransferListener.create())
.build());

download.completionFuture().join();

// Verify progress tracking worked - totalBytes is set correctly
assertThat(download.progress().snapshot().totalBytes()).isPresent();
assertThat(download.progress().snapshot().totalBytes().getAsLong()).isEqualTo(expectedSize);

// Verify transferredBytes reached expectedSize
assertThat(download.progress().snapshot().transferredBytes()).isEqualTo(expectedSize);

// Verify file size matches expected
assertThat(downloadPath.toFile().length()).isEqualTo(expectedSize);
}

@ParameterizedTest(name = "downloadWithPresignedUrl_toBytes_progress_{0}_range={3}")
@MethodSource("progressTestCases")
void downloadWithPresignedUrl_toBytes_progressTracking(String tmType, S3TransferManager tm, String key,
String range, int expectedSize) throws Exception {

PresignedUrlDownloadRequest.Builder requestBuilder = PresignedUrlDownloadRequest.builder()
.presignedUrl(createPresignedRequest(key).url());
if (range != null) {
requestBuilder.range(range);
}

Download<ResponseBytes<GetObjectResponse>> download = tm.downloadWithPresignedUrl(
PresignedDownloadRequest.<ResponseBytes<GetObjectResponse>>builder()
.presignedUrlDownloadRequest(requestBuilder.build())
.responseTransformer(AsyncResponseTransformer.toBytes())
.addTransferListener(LoggingTransferListener.create())
.build());

CompletedDownload<ResponseBytes<GetObjectResponse>> completed = download.completionFuture().join();

assertThat(download.progress().snapshot().totalBytes()).isPresent();
assertThat(download.progress().snapshot().totalBytes().getAsLong()).isEqualTo(expectedSize);

assertThat(download.progress().snapshot().transferredBytes()).isEqualTo(expectedSize);

assertThat(completed.result().asByteArray()).hasSize(expectedSize);
}

private static PresignedDownloadFileRequest createFileDownloadRequest(String key, Path destination) {
return PresignedDownloadFileRequest.builder()
.presignedUrlDownloadRequest(PresignedUrlDownloadRequest.builder()
Expand Down
6 changes: 4 additions & 2 deletions ...r/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,9 @@ public final FileDownload downloadFileWithPresignedUrl(PresignedDownloadFileRequ
progressUpdater.transferInitiated();

responseTransformer = isS3ClientMultipartEnabled()
&& presignedDownloadFileRequest.presignedUrlDownloadRequest().range() == null
? progressUpdater.wrapForNonSerialFileDownload(
responseTransformer, GetObjectRequest.builder().build())
responseTransformer, GetObjectRequest.builder().build())
: progressUpdater.wrapResponseTransformer(responseTransformer);
progressUpdater.registerCompletion(returnFuture);

Expand Down Expand Up @@ -652,8 +653,9 @@ public final <ResultT> Download<ResultT> downloadWithPresignedUrl(
progressUpdater.transferInitiated();

responseTransformer = isS3ClientMultipartEnabled()
&& presignedDownloadRequest.presignedUrlDownloadRequest().range() == null
? progressUpdater.wrapForNonSerialFileDownload(
responseTransformer, GetObjectRequest.builder().build())
responseTransformer, GetObjectRequest.builder().build())
: progressUpdater.wrapResponseTransformer(responseTransformer);
progressUpdater.registerCompletion(returnFuture);

Expand Down
150 changes: 150 additions & 0 deletions .../test/java/software/amazon/awssdk/transfer/s3/internal/S3TransferManagerListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.file.FileSystem;
import java.nio.file.Files;
Expand All @@ -51,24 +52,33 @@
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension;
import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest;
import software.amazon.awssdk.transfer.s3.model.CompletedFileUpload;
import software.amazon.awssdk.transfer.s3.model.Download;
import software.amazon.awssdk.transfer.s3.model.DownloadFileRequest;
import software.amazon.awssdk.transfer.s3.model.DownloadRequest;
import software.amazon.awssdk.transfer.s3.model.FileDownload;
import software.amazon.awssdk.transfer.s3.model.FileUpload;
import software.amazon.awssdk.transfer.s3.S3TransferManager;
import software.amazon.awssdk.transfer.s3.model.PresignedDownloadFileRequest;
import software.amazon.awssdk.transfer.s3.model.PresignedDownloadRequest;
import software.amazon.awssdk.transfer.s3.model.TransferObjectRequest;
import software.amazon.awssdk.transfer.s3.model.Upload;
import software.amazon.awssdk.transfer.s3.model.UploadFileRequest;
import software.amazon.awssdk.transfer.s3.model.UploadRequest;
import software.amazon.awssdk.transfer.s3.progress.TransferListener;
import software.amazon.awssdk.utils.CompletableFutureUtils;

public class S3TransferManagerListenerTest {
private final FileSystem fs = Jimfs.newFileSystem(Configuration.unix());
private S3CrtAsyncClient s3Crt;
private S3TransferManager tm;
private long contentLength;
private static final String PRESIGNED_URL = "https://test-bucket.s3.amazonaws.com/test-key"
+ "?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKID"
+ "&X-Amz-Date=20260101T000000Z&X-Amz-Expires=600"
+ "&X-Amz-SignedHeaders=host&X-Amz-Signature=abc123";

@BeforeEach
public void methodSetup() {
Expand Down Expand Up @@ -330,6 +340,126 @@ public void listener_exception_shouldBeSuppressed() throws Exception {
verifyNoMoreInteractions(listener);
}

@Test
public void downloadFileWithPresignedUrl_success_shouldInvokeListener() throws Exception {
Copy link
Copy Markdown
Collaborator

@RanVaknin RanVaknin May 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These new tests are using the mock S3CrtAsyncClient which is not a multipart client mock. I think these would pass regardless of the fix?

Are these related to the last PR? Why did we ship these separately?

stubPresignedUrlExtension();
TransferListener listener = mock(TransferListener.class);

PresignedDownloadFileRequest downloadRequest = PresignedDownloadFileRequest.builder()
.presignedUrlDownloadRequest(presignedUrlDownloadRequest())
.destination(newTempFile())
.addTransferListener(listener)
.build();

FileDownload download = tm.downloadFileWithPresignedUrl(downloadRequest);

ArgumentCaptor<TransferListener.Context.TransferInitiated> captor1 =
ArgumentCaptor.forClass(TransferListener.Context.TransferInitiated.class);
verify(listener, timeout(1000).times(1)).transferInitiated(captor1.capture());
TransferListener.Context.TransferInitiated ctx1 = captor1.getValue();
assertThat(ctx1.request()).isSameAs(downloadRequest);
assertThat(ctx1.progressSnapshot().totalBytes()).isNotPresent();
assertThat(ctx1.progressSnapshot().transferredBytes()).isZero();

ArgumentCaptor<TransferListener.Context.BytesTransferred> captor2 =
ArgumentCaptor.forClass(TransferListener.Context.BytesTransferred.class);
verify(listener, timeout(1000).atLeastOnce()).bytesTransferred(captor2.capture());
TransferListener.Context.BytesTransferred ctx2 = captor2.getValue();
assertThat(ctx2.request()).isSameAs(downloadRequest);
assertThat(ctx2.progressSnapshot().totalBytes()).hasValue(contentLength);
assertThat(ctx2.progressSnapshot().transferredBytes()).isPositive();

ArgumentCaptor<TransferListener.Context.TransferComplete> captor3 =
ArgumentCaptor.forClass(TransferListener.Context.TransferComplete.class);
verify(listener, timeout(1000).times(1)).transferComplete(captor3.capture());
TransferListener.Context.TransferComplete ctx3 = captor3.getValue();
assertThat(ctx3.request()).isSameAs(downloadRequest);
assertThat(ctx3.progressSnapshot().totalBytes()).hasValue(contentLength);
assertThat(ctx3.progressSnapshot().transferredBytes()).isEqualTo(contentLength);
assertThat(ctx3.completedTransfer()).isSameAs(download.completionFuture().get());

download.completionFuture().join();
verifyNoMoreInteractions(listener);
}

@Test
public void downloadWithPresignedUrl_success_shouldInvokeListener() throws Exception {
stubPresignedUrlExtension();
TransferListener listener = mock(TransferListener.class);

PresignedDownloadRequest<ResponseBytes<GetObjectResponse>> downloadRequest =
PresignedDownloadRequest.<ResponseBytes<GetObjectResponse>>builder()
.presignedUrlDownloadRequest(presignedUrlDownloadRequest())
.responseTransformer(AsyncResponseTransformer.toBytes())
.addTransferListener(listener)
.build();

Download<ResponseBytes<GetObjectResponse>> download = tm.downloadWithPresignedUrl(downloadRequest);

ArgumentCaptor<TransferListener.Context.TransferInitiated> captor1 =
ArgumentCaptor.forClass(TransferListener.Context.TransferInitiated.class);
verify(listener, timeout(1000).times(1)).transferInitiated(captor1.capture());
TransferListener.Context.TransferInitiated ctx1 = captor1.getValue();
assertThat(ctx1.request()).isSameAs(downloadRequest);
assertThat(ctx1.progressSnapshot().totalBytes()).isNotPresent();
assertThat(ctx1.progressSnapshot().transferredBytes()).isZero();

ArgumentCaptor<TransferListener.Context.BytesTransferred> captor2 =
ArgumentCaptor.forClass(TransferListener.Context.BytesTransferred.class);
verify(listener, timeout(1000).atLeastOnce()).bytesTransferred(captor2.capture());
TransferListener.Context.BytesTransferred ctx2 = captor2.getValue();
assertThat(ctx2.request()).isSameAs(downloadRequest);
assertThat(ctx2.progressSnapshot().totalBytes()).hasValue(contentLength);
assertThat(ctx2.progressSnapshot().transferredBytes()).isPositive();

ArgumentCaptor<TransferListener.Context.TransferComplete> captor3 =
ArgumentCaptor.forClass(TransferListener.Context.TransferComplete.class);
verify(listener, timeout(1000).times(1)).transferComplete(captor3.capture());
TransferListener.Context.TransferComplete ctx3 = captor3.getValue();
assertThat(ctx3.request()).isSameAs(downloadRequest);
assertThat(ctx3.progressSnapshot().totalBytes()).hasValue(contentLength);
assertThat(ctx3.progressSnapshot().transferredBytes()).isEqualTo(contentLength);
assertThat(ctx3.completedTransfer()).isSameAs(download.completionFuture().get());

download.completionFuture().join();
verifyNoMoreInteractions(listener);
}

@Test
public void downloadFileWithPresignedUrl_failure_shouldInvokeListener() throws Exception {
SdkClientException sdkClientException = SdkClientException.create("download failed");
stubPresignedUrlExtensionWithFailure(sdkClientException);
TransferListener listener = mock(TransferListener.class);

PresignedDownloadFileRequest downloadRequest = PresignedDownloadFileRequest.builder()
.presignedUrlDownloadRequest(presignedUrlDownloadRequest())
.destination(newTempFile())
.addTransferListener(listener)
.build();

FileDownload download = tm.downloadFileWithPresignedUrl(downloadRequest);
assertThatThrownBy(() -> download.completionFuture().join())
.isInstanceOf(CompletionException.class)
.hasCause(sdkClientException);

ArgumentCaptor<TransferListener.Context.TransferInitiated> captor1 =
ArgumentCaptor.forClass(TransferListener.Context.TransferInitiated.class);
verify(listener, timeout(1000).times(1)).transferInitiated(captor1.capture());
TransferListener.Context.TransferInitiated ctx1 = captor1.getValue();
assertThat(ctx1.request()).isSameAs(downloadRequest);
assertThat(ctx1.progressSnapshot().transferredBytes()).isZero();

ArgumentCaptor<TransferListener.Context.TransferFailed> captor2 =
ArgumentCaptor.forClass(TransferListener.Context.TransferFailed.class);
verify(listener, timeout(1000).times(1)).transferFailed(captor2.capture());
TransferListener.Context.TransferFailed ctx2 = captor2.getValue();
assertThat(ctx2.request()).isSameAs(downloadRequest);
assertThat(ctx2.progressSnapshot().transferredBytes()).isZero();
assertThat(ctx2.exception()).isEqualTo(sdkClientException);

verifyNoMoreInteractions(listener);
}

private static TransferListener throwingListener() {
TransferListener listener = mock(TransferListener.class);
RuntimeException e = new RuntimeException("Intentional exception for testing purposes");
Expand All @@ -340,6 +470,26 @@ private static TransferListener throwingListener() {
return listener;
}

private void stubPresignedUrlExtension() {
AsyncPresignedUrlExtension mockExtension = mock(AsyncPresignedUrlExtension.class);
when(s3Crt.presignedUrlExtension()).thenReturn(mockExtension);
when(mockExtension.getObject(any(PresignedUrlDownloadRequest.class), any(AsyncResponseTransformer.class)))
.thenAnswer(randomGetResponseBody(contentLength));
}

private void stubPresignedUrlExtensionWithFailure(Throwable error) {
AsyncPresignedUrlExtension mockExtension = mock(AsyncPresignedUrlExtension.class);
when(s3Crt.presignedUrlExtension()).thenReturn(mockExtension);
when(mockExtension.getObject(any(PresignedUrlDownloadRequest.class), any(AsyncResponseTransformer.class)))
.thenReturn(CompletableFutureUtils.failedFuture(error));
}

private PresignedUrlDownloadRequest presignedUrlDownloadRequest() throws Exception {
return PresignedUrlDownloadRequest.builder()
.presignedUrl(new URL(PRESIGNED_URL))
.build();
}

private static Answer<CompletableFuture<PutObjectResponse>> drainPutRequestBody() {
return invocationOnMock -> {
AsyncRequestBody requestBody = invocationOnMock.getArgument(1, AsyncRequestBody.class);
Expand Down
Loading