Generate 8 more S3 Operations #3895

peterrsongg · 2025-06-27T04:42:28Z

Description

Generates 9 more operations to conclude phase 1. I had to add a customization to exclude members from property generation but not from the marshallers because of PartDetail which inherits from PartETag. PartETag contains the excluded members.

I also updated the RestXmlExceptionUnmarshaller.tt file to get the Id2 and CfId which is unique to S3, as well as update the exception shapes themselves to add the constructor which takes these values in its constructor.

One change from this is that PutBucketPolicy's payload is set to request.Content instead of request.ContentStream. I've done a sanity check to make sure it still works. Per the model since the payload is a string, there is no need to wrap it in a memory stream. Also the payload member doesn't target the streaming trait so there is no need to stream it either.

One other minor change is that Tagging in PutBucketTagging was automatically initialized if it was null. This was a one-off customization we had that I intentionally left out to remove another customization hook. If we feel we want this added back, we can do that, but I will call this out as a breaking change.

Motivation and Context

Testing

DRY_RUN-2df7b27d-b938-44c2-aa9d-64c1ce797bf7

AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.NoSuchUploadException/MethodAdded: New method System.Void .ctor(System.String, System.Exception, Amazon.Runtime.ErrorType, System.String, System.String, System.Net.HttpStatusCode, System.String, System.String) in Amazon.S3.Model.NoSuchUploadException
AssemblyComparer AWSSDK.S3.New.dll: Error Amazon.S3.Model.PartDetail/MethodRemoved: Missing method System.DateTime get_LastModified() in Amazon.S3.Model.PartDetail
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.PartDetail/MethodAdded: New method System.Nullable<System.DateTime> get_LastModified() in Amazon.S3.Model.PartDetail
AssemblyComparer AWSSDK.S3.New.dll: Error Amazon.S3.Model.PartDetail/MethodRemoved: Missing method System.Void set_LastModified(System.DateTime) in Amazon.S3.Model.PartDetail
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.PartDetail/MethodAdded: New method System.Void set_LastModified(System.Nullable<System.DateTime>) in Amazon.S3.Model.PartDetail
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.PutObjectTaggingRequest/MethodAdded: New method System.Void set_ContentMD5(System.String) in Amazon.S3.Model.PutObjectTaggingRequest
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.PutObjectTaggingRequest/MethodAdded: New method System.String get_ContentMD5() in Amazon.S3.Model.PutObjectTaggingRequest
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.NoSuchKeyException/MethodAdded: New method System.Void .ctor(System.String, System.Exception, Amazon.Runtime.ErrorType, System.String, System.String, System.Net.HttpStatusCode, System.String, System.String) in Amazon.S3.Model.NoSuchKeyException

    var putBucketPolicyResponse = await s3Client.PutBucketPolicyAsync(new PutBucketPolicyRequest
    {
        BucketName = bucketName,
        Policy = 
"""
{
"Version":"2008-10-17",
"Id":"test-put-bucket-policy",
"Statement" : [
    {
        "Effect":"Allow",
        "Sid":"1", 
        "Principal" : {
            "AWS": "arn:aws:iam::<account_id>:***"
        },
        "Action":["s3:*"],
        "Resource":"arn:aws:s3:::<bucket_name>*"
    }
    ]
}
"""
    }).ConfigureAwait(false);

executed successfully with flipping request.ContentStream to request.Content

Screenshots (if appropriate)

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

generator/ServiceClientGeneratorLib/Generators/Marshallers/RestXmlRequestMarshaller.tt

generator/.DevConfigs/c881e931-b1ff-45ff-9521-9653e179466e.json

generator/ServiceClientGeneratorLib/Operation.cs

dscpinheiro · 2025-07-02T20:11:00Z

sdk/src/Services/S3/Generated/Model/Condition.cs

@@ -1,97 +0,0 @@
-/*


Is this removal intentional? It's in the generated folder...

This is only used in GetBucketWebsite so I'm not sure how it got here...

it doesn't even exist here..
https://github.com/aws/aws-sdk-net/tree/main/sdk/src/Services/S3/Generated/Model

maybe a github UI thing? not sure how that happened

It's gone from the last diff, so maybe? But yeah need to make sure we don't remove generated code moving forward.

…PutObjectLockConfiguration, PutObjectRetention, PutObjectTagging, and PutPublicAccessBlock. Update s3 exception tt files

muhammad-othman · 2025-07-02T23:36:17Z

sdk/src/Services/S3/Generated/Model/PartDetail.cs

+        /// Date and time at which the part was uploaded.
+        /// </para>
+        /// </summary>
+        public DateTime? LastModified


Looks like LastModified to isn't using nullable DateTime in v4, probably we missed it.
Changing it now is a breaking change, are we okay with that?

Yup, in my previous PRs Norm said as long as we call it out in the DevConfig it is fine, but just for these nullable type changes. So I included it in the dev config

It is not great to make these breaking changes but we are still early in V4 adoption and these fields missed so far are likely not the common fields used. I would prefer to take the hit of taking on the minor breaking change in this early part of V4 then add more complexity in our generator to support some fields not being nullable.

@peterrsongg If you find a property that looks like it would be frequent property being accessed like a bucket name then we should discuss.

Gotcha, if i feel like the property is very commonly used I will bring it up for discussion

dscpinheiro · 2025-07-02T23:42:43Z

generator/ServiceClientGeneratorLib/Generators/Marshallers/RestXmlRequestMarshaller.tt

@@ -729,6 +730,8 @@ WriteXmlAttributeString(level + 1, member, variableName, isPayload: true, operat
 		{
 #>
 <#=new string(' ', level * 4)#>request.Content = Encoding.UTF8.GetBytes(<#=this.Operation.RequestPayloadMember.PrimitiveMarshaller#>(publicRequest.<#=this.Operation.RequestPayloadMember.PropertyName#>));
+<#+


These lines (733 and 734) can be removed too now, right?

dscpinheiro · 2025-07-02T23:43:54Z

sdk/src/Services/S3/Generated/Model/Condition.cs

@@ -1,97 +0,0 @@
-/*


It's gone from the last diff, so maybe? But yeah need to make sure we don't remove generated code moving forward.

Need to validate a potential regression in another API that was auto-generated

peterrsongg added the v4 label Jun 27, 2025

boblodgett requested review from dscpinheiro and muhammad-othman June 27, 2025 17:32

dscpinheiro reviewed Jul 2, 2025

View reviewed changes

peterrsongg added 4 commits July 2, 2025 13:36

Generate HeadBucket, ListParts, PutBucketPolicy, PutObjectLegalHold, …

a5ef97c

…PutObjectLockConfiguration, PutObjectRetention, PutObjectTagging, and PutPublicAccessBlock. Update s3 exception tt files

update ObjectTaggingTests

6759aad

Update devConfig message

0cce950

Address PR Feedback

a25b031

peterrsongg force-pushed the petesong/generate-s3-phase-1-pr6 branch from f765e2a to a25b031 Compare July 2, 2025 21:33

peterrsongg changed the title ~~Generate 9 more S3 Operations~~ Generate 8 more S3 Operations Jul 2, 2025

peterrsongg requested a review from dscpinheiro July 2, 2025 21:36

muhammad-othman reviewed Jul 2, 2025

View reviewed changes

dscpinheiro previously approved these changes Jul 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Generate 8 more S3 Operations #3895

Generate 8 more S3 Operations #3895

Uh oh!

peterrsongg commented Jun 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dscpinheiro Jul 2, 2025

Uh oh!

peterrsongg Jul 2, 2025

Uh oh!

peterrsongg Jul 2, 2025

Uh oh!

dscpinheiro Jul 2, 2025

Uh oh!

muhammad-othman Jul 2, 2025

Uh oh!

peterrsongg Jul 3, 2025

Uh oh!

normj Jul 3, 2025

Uh oh!

peterrsongg Jul 3, 2025

Uh oh!

dscpinheiro Jul 2, 2025

Uh oh!

dscpinheiro Jul 2, 2025

Uh oh!

Uh oh!

Generate 8 more S3 Operations #3895

Are you sure you want to change the base?

Generate 8 more S3 Operations #3895

Uh oh!

Conversation

peterrsongg commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterrsongg commented Jun 27, 2025 •

edited

Loading