Added PutObjectResponse to TransferUtilityUploadResponse mapping #4045

GarrettBeatty · 2025-10-14T16:27:07Z

Description

This change creates a TransferUtilityUploadResponse class. This class contains the common fields between PutObjectResponse and CompleteMultipartUploadResponse. The reason it contains the common fields between PutObjectResponse and CompleteMultipartUploadResponse is because this same object should be utilized in both regular upload and multi part upload (as per the SEP).

This class will be used in #4059 and #4061 Where for progress events (e.g. UploadCompleted), we return a TransferUtilityUploadResponse object as part of it. This class will also be used when we create new apis as part of SEP compliance (UploadWithResponseAsync). it will be the return value of this function (instead of void like the existing UploadAsync).

TranserUtilityUploadRequest req = ... 
TransferUtilityUploadResponse response = transferUtility.UploadWithResponse(req)

In order to create this TransferUtilityUploadResponse class, I have created a conversion function MapPutObjectResponse which maps the PutObjectResponse to the TransferUtilityUploadResponse. Eventually, this class will also contain a MapCompletemultipartResponse function as well #4060

In order to comply with the SEP and ensure that all fields are mapped, I have added unit tests which read the mapping.json file from the SEP and validate that each of them get mapped in my conversion function between the two classes.

Motivation and Context

DOTNET-8275

Testing

Unit tests
4728749f-fc1d-45b4-a2a3-3605bf5dcd60 dry run pass

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

Copilot

Pull Request Overview

This PR adds support for mapping PutObjectResponse to TransferUtilityUploadResponse in the AWS S3 SDK, enabling consistent property exposure across upload operations.

Key changes:

Creates a new TransferUtilityUploadResponse class with comprehensive property mappings
Implements ResponseMapper utility for PutObjectResponse to TransferUtilityUploadResponse conversion
Adds extensive test coverage with JSON configuration-driven validation

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
TransferUtilityUploadResponse.cs	New response class with properties mapped from S3 upload responses
ResponseMapper.cs	Internal utility class for mapping PutObjectResponse to TransferUtilityUploadResponse
ResponseMapperTests.cs	Comprehensive test suite with configuration-driven validation
mapping.json	Configuration defining field mappings between S3 request/response types
property-aliases.json	Property name aliases for backward compatibility
mapping-schema.json	JSON schema for validating mapping configuration structure
AssemblyInfo.cs	Added InternalsVisibleTo attributes for test access
433a9a6d-b8ea-4676-b763-70711e8288e4.json	Dev config file specifying patch-level change

sdk/test/Services/S3/UnitTests/Custom/TestData/mapping-schema.json

GarrettBeatty · 2025-10-17T13:50:42Z

generator/ServiceClientGeneratorLib/Generators/SourceFiles/AssemblyInfo.tt

 [assembly: InternalsVisibleTo("AWSSDK.UnitTests.DynamoDBv2.NetFramework, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db5f59f098d27276c7833875a6263a3cc74ab17ba9a9df0b52aedbe7252745db7274d5271fd79c1f08f668ecfa8eaab5626fa76adc811d3c8fc55859b0d09d3bc0a84eecd0ba891f2b8a2fc55141cdcc37c2053d53491e650a479967c3622762977900eddbf1252ed08a2413f00a28f3a0752a81203f03ccb7f684db373518b4")]
 [assembly: InternalsVisibleTo("AWSSDK.UnitTests.NetFramework, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db5f59f098d27276c7833875a6263a3cc74ab17ba9a9df0b52aedbe7252745db7274d5271fd79c1f08f668ecfa8eaab5626fa76adc811d3c8fc55859b0d09d3bc0a84eecd0ba891f2b8a2fc55141cdcc37c2053d53491e650a479967c3622762977900eddbf1252ed08a2413f00a28f3a0752a81203f03ccb7f684db373518b4")]
 <# } #>
+<# if (this.Config.AssemblyTitle=="AWSSDK.S3") { #>


this is added so we can test the internal ResponseMapper class i made

GarrettBeatty · 2025-10-17T13:52:22Z

sdk/src/Services/S3/Custom/Transfer/TransferUtilityUploadResponse.cs

+    /// Contains unified response fields from both simple uploads (PutObjectResponse) 
+    /// and multipart uploads (CompleteMultipartUploadResponse).
+    /// </summary>
+    public class TransferUtilityUploadResponse : AmazonWebServiceResponse


right now if we release this change to development, the customer can access this class even though it wont really be used for anything until the next PRs. which i think should go in the feature branch. wondering if we should target feature/transfermanager for this change even though its not a runtime change?

GarrettBeatty · 2025-10-17T13:53:36Z

sdk/test/Services/S3/UnitTests/Custom/EmbeddedResource/property-aliases.json

@@ -0,0 +1,10 @@
+{
+  "PropertyAliases": {


.net's property values are slightly different than whats in the mapping so i made another mapping to the .net values. the reason i didnt modify the original mapping.json is so we can copy and paste it from the SEP code directly if we ever need to update it

GarrettBeatty · 2025-10-17T14:01:54Z

sdk/test/Services/S3/UnitTests/Custom/ResponseMapperTests.cs

+        {
+            ValidateResponseDefinitionCompleteness<GetObjectResponse>(
+                new[] { "Definition", "DownloadResponse", "GetObjectResponse" },
+                "GetObjectResponse",


wondering if we should be updating GetObjectResponse soon/now instead of doing this to match the model? e.g. ContentType is in the HeaderCollection property currently. but the model https://github.com/aws/aws-sdk-net/blob/main/generator/ServiceModels/s3/s3-2006-03-01.api.json#L4575 i think maps it to its own attribute @philasmar @peterrsongg ?

also this test case isnt really needed as part of this work. i added it just for completeness sake. its validating that getobjectresponse has all of the expected fields in the mapping.json

@GarrettBeatty yeah, if it is mapped to a property on the output then we should follow the model, especially if it isn't there right now.

i think i might just remove this test case because we dont need to copy content type to transferutilityuploadresponse anyway, so this test case really isnt validating anything important

ended up removing this since if we update the getobjectresponse model anyway it will be backward compatible so the fields we have now are fine and we dont need a test for it

peterrsongg

Change looks good, but I have a question.

You're not actually calling MapPutObjectResponse anywhere in this PR. So where does the mapping actually happen? Is that going to be part of another PR?

peterrsongg · 2025-10-17T21:19:53Z

Approved, based on assumption that the actual mapping will happen in another PR (unless my read of this is mistaken)

GarrettBeatty · 2025-10-18T01:52:12Z

Change looks good, but I have a question.

You're not actually calling MapPutObjectResponse anywhere in this PR. So where does the mapping actually happen? Is that going to be part of another PR?

yes - description of the PR has links to the ones that will call it

update assemblyinfo fix assembly info Add debug logging embed resource add debug logging fix test data remove schema remove uneeded tests

GarrettBeatty requested a review from Copilot October 14, 2025 17:42

Copilot AI reviewed Oct 14, 2025

View reviewed changes

sdk/test/Services/S3/UnitTests/Custom/TestData/mapping-schema.json Outdated Show resolved Hide resolved

GarrettBeatty changed the base branch from getobject to development October 14, 2025 22:28

GarrettBeatty force-pushed the gcbeatty/tuuploadresponse branch 2 times, most recently from 72d85c3 to 2dffdee Compare October 16, 2025 19:14

GarrettBeatty requested a review from philasmar October 17, 2025 13:50

GarrettBeatty marked this pull request as ready for review October 17, 2025 13:50

GarrettBeatty commented Oct 17, 2025

View reviewed changes

GarrettBeatty mentioned this pull request Oct 17, 2025

add CompleteMultipartUploadResponse mapping to TransferUtilityResponse object #4046

Closed

10 tasks

GarrettBeatty requested a review from peterrsongg October 17, 2025 17:27

GarrettBeatty changed the base branch from development to feature/transfermanager October 17, 2025 18:02

GarrettBeatty changed the base branch from feature/transfermanager to development October 17, 2025 18:02

peterrsongg approved these changes Oct 17, 2025

View reviewed changes

Added PutObjectResponse to TransferUtilityUploadResponse mapping

4f9bdfa

update assemblyinfo fix assembly info Add debug logging embed resource add debug logging fix test data remove schema remove uneeded tests

GarrettBeatty force-pushed the gcbeatty/tuuploadresponse branch from 35dddc2 to 4f9bdfa Compare October 18, 2025 03:10

Added PutObjectResponse to TransferUtilityUploadResponse mapping #4045

Are you sure you want to change the base?

Added PutObjectResponse to TransferUtilityUploadResponse mapping #4045

Uh oh!

Conversation

GarrettBeatty commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Testing

Types of changes

Checklist

License

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

GarrettBeatty Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

peterrsongg Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

peterrsongg left a comment

Choose a reason for hiding this comment

Uh oh!

peterrsongg commented Oct 17, 2025

Uh oh!

GarrettBeatty commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GarrettBeatty commented Oct 14, 2025 •

edited

Loading

GarrettBeatty Oct 18, 2025 •

edited

Loading