Fix presigned URL download progress tracking for range requests and add listener tests#6977
Open
jencymaryjoseph wants to merge 1 commit into
Conversation
RanVaknin
requested changes
May 19, 2026
| } | ||
|
|
||
| @Test | ||
| public void downloadFileWithPresignedUrl_success_shouldInvokeListener() throws Exception { |
Collaborator
There was a problem hiding this comment.
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?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Presigned URL downloads with a user-specified range and multipart enabled produced no progress tracking output. The
wrapForNonSerialFileDownloadwrapper was applied, but when range is set, multipart is skipped (single request) and split() is never called — so the wrapper's progress hooks never fire.Modifications
GenericS3TransferManager.downloadFileWithPresignedUrl: Added range == null check — uses wrapForNonSerialFileDownload for multipart without range, falls back to wrapResponseTransformer when range is set.GenericS3TransferManager.downloadWithPresignedUrl:Same range == null check.Testing
S3TransferManagerListenerTest: Added TransferListener callback verification tests for presigned URL downloads — verifies transferInitiated, bytesTransferred, transferComplete fire on success, and transferInitiated, transferFailed fire on failure.S3TransferManagerPresignedUrlDownloadIntegrationTest: Added parameterized progress tracking tests (multipart/non-multipart × range/no-range) asserting totalBytes, transferredBytes, and downloaded size correctness.Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License