Skip to content

Conversation

@GarrettBeatty
Copy link
Contributor

@GarrettBeatty GarrettBeatty commented Oct 13, 2025

Same as #4039 but for multi part upload command

This change creates 3 event progress trackers for MultipartUploadCommand. UploadInitiatedEvent, UploadedCompletedEvent, and UploadFailedEvent. This allows the user to subscribe to events like doing below

        private void uploadStarted(object sender, UploadInitiatedEventArgs args)
         {
             Console.WriteLine($"Upload started: {args.FilePath}");
             Console.WriteLine($"Total size: {args.TotalBytes} bytes");
             Console.WriteLine($"Bucket: {args.Request.BucketName}");
             Console.WriteLine($"Key: {args.Request.Key}");
         }


         TransferUtilityUploadRequest request = new TransferUtilityUploadRequest();
         request.UploadInitiatedEvent += uploadStarted;

These new events should only be fired 1 time during the entire lifecycle of the upload. The details of what each event should contain is in the SEP.

Some of the events (UploadCompleted) also have new TranserUtilityUploadResponse object as per the SEP requirements. This contains all of the common fields between the low level PutObectResponse and CompleteMultipartUploadResponse.

Just as an FYI will also eventually be returning TransferUtilityUploadResponse object to the user when we create the new apis UploadWithResponseAsync


TranserUtilityUploadResponse = transerutiltiy.UploadWithResponseAsync(TransferUtilityUploadRequest req) // in the future

As per the SEP requirements, I have added unit tests using the mapping.json file from the SEP, which verify that all required fields in CompleteMultipartUploadResponse are mapped to TransferUtilityUploadResponse.

Description

  1. DOTNET-8276

Motivation and Context

This is being done as part of the SEP compliance.

Testing

  1. Integration tests/unit tests which demonstrate the event handler behavior
  2. dry run 64013b1c-44b4-43bd-b559-a92830effa41

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty requested a review from Copilot October 13, 2025 18:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive multipart upload lifecycle event tracking to the S3 Transfer Utility, including initiated, completed, and failed events for both seekable and unseekable streams. It also introduces response mapping functionality to convert CompleteMultipartUploadResponse to TransferUtilityUploadResponse for consistent response handling.

Key changes:

  • Added lifecycle event handling (initiated, completed, failed) for multipart uploads
  • Implemented response mapping between CompleteMultipartUploadResponse and TransferUtilityUploadResponse
  • Enhanced progress tracking with comprehensive test coverage for various upload scenarios

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ResponseMapperTests.cs Added comprehensive unit tests for CompleteMultipartUploadResponse mapping validation
TransferUtilityTests.cs Added extensive integration tests for multipart upload lifecycle events
MultipartUploadCommand.async.cs Integrated lifecycle event firing and response mapping in async multipart upload flow
ResponseMapper.cs Implemented mapping logic from CompleteMultipartUploadResponse to TransferUtilityUploadResponse
MultipartUploadCommand.cs Added event firing helper methods and enhanced progress tracking
dev config Added patch-level configuration with appropriate changelog messages
Comments suppressed due to low confidence (1)

sdk/src/Services/S3/Custom/Transfer/Internal/_async/MultipartUploadCommand.async.cs:1

  • The UploadProgressArgs constructor call has been modified to include an additional parameter (_fileTransporterRequest). This appears to be a breaking change to the UploadProgressArgs constructor signature. Breaking changes should be avoided in patch releases and require justification or a major version bump.
/*

};

// Use invalid bucket name to force failure with multipart upload size
var invalidBucketName = "invalid-bucket-name-" + Guid.NewGuid().ToString();
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The invalid bucket name generation logic is duplicated across multiple test methods. Consider extracting this into a helper method to improve maintainability and ensure consistency.

Copilot uses AI. Check for mistakes.
};

// Use invalid bucket name to force failure with multipart upload size
var invalidBucketName = "invalid-bucket-name-" + Guid.NewGuid().ToString();
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The invalid bucket name generation logic is duplicated across multiple test methods. Consider extracting this into a helper method to improve maintainability and ensure consistency.

Copilot uses AI. Check for mistakes.
@GarrettBeatty GarrettBeatty changed the title Gcbeatty/multiuplaodtracking Add UploadIniaited, Completed, and Failed event progress trackers to SimpleUploadCommand Oct 13, 2025
@GarrettBeatty GarrettBeatty changed the title Add UploadIniaited, Completed, and Failed event progress trackers to SimpleUploadCommand Add UploadIniaited, Completed, and Failed event progress trackers to MultipartUploadCommand Oct 13, 2025
@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/simpleuploadprogresstracking branch 2 times, most recently from 0f0e9f4 to 352afda Compare October 14, 2025 16:32
@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/multiuplaodtracking branch 2 times, most recently from cb37e30 to 0a51a22 Compare October 14, 2025 16:52
@GarrettBeatty GarrettBeatty changed the base branch from gcbeatty/simpleuploadprogresstracking to gcbeatty/tuuploadresponsemulti October 14, 2025 16:52
@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/tuuploadresponsemulti branch from c9687e7 to 4f32bc1 Compare October 17, 2025 18:07
@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/multiuplaodtracking branch 2 times, most recently from 957e76a to 92b56de Compare October 18, 2025 02:46
@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/multiuplaodtracking branch from 92b56de to be0d3b4 Compare October 18, 2025 02:50
@GarrettBeatty GarrettBeatty deleted the gcbeatty/multiuplaodtracking branch October 23, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant