Generate 8 more s3 operations #3893

peterrsongg · 2025-06-23T19:41:54Z

Description

Generates the following operations:

GetBucketTagging
GetBucketRequestPayment
GetObjectAttributes
GetObjectLegalHold
GetObjectLockConfiguration
GetObjectRetention
GetObjectTagging
GetPublicAccessBlock

I added a customization "TreatEnumsAsEnum". For List types, enum types are generated as strings, which in S3 isn't always the case. There are cases where List is written in custom code. If this customization is used it will treat the enum as an enum instead of as a string.

Motivation and Context

Testing

Dry run passes DRY_RUN-e40d50e6-0c92-4258-9f40-5829631b3698
No backwards incompatible changes (re-ran 10:58AM PST 06/24/25)

AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.ErrorDocument/TypeAdded: New type Amazon.S3.Model.ErrorDocument
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.NoSuchKeyException/TypeAdded: New type Amazon.S3.Model.NoSuchKeyException
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.Condition/TypeAdded: New type Amazon.S3.Model.Condition
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.GetObjectTaggingResponse/MethodAdded: New method System.String get_VersionId() in Amazon.S3.Model.GetObjectTaggingResponse
AssemblyComparer AWSSDK.S3.New.dll: Message Amazon.S3.Model.GetObjectTaggingResponse/MethodAdded: New method System.Void set_VersionId(System.String) in Amazon.S3.Model.GetObjectTaggingResponse

Additional sanity testing:

    var getBucketRequestPaymentResponse = await s3Client.GetBucketRequestPaymentAsync(new GetBucketRequestPaymentRequest
    {
        BucketName = bucketName,
        ExpectedBucketOwner = expectedBucketOwner
    }).ConfigureAwait(false);
    Console.WriteLine("The bucket's payer is {0}", getBucketRequestPaymentResponse.Payer);

The bucket's payer is BucketOwner

    var getBucketTaggingResponse = await s3Client.GetBucketTaggingAsync(new GetBucketTaggingRequest
    {
        BucketName = bucketName,
        ExpectedBucketOwner = expectedBucketOwner
    }).ConfigureAwait(false);
    getBucketTaggingResponse.TagSet.ForEach(x => Console.WriteLine(x.Key));

fizz
foo

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

normj · 2025-06-23T20:44:09Z

generator/ServiceModels/s3/s3.customizations.json

        }
+
+    },
+    "treatEnumsAsEnums":{


Instead of making up the new term treatEnumsAsEnums which by itself sounds weird because if set it false then what does it treat enums if not enums. I suggest reusing the existing term and possible add an Override prefix. For example have a customization called OverrideTreatEnumsAsString and then add ObjectAttributesList with a value false.

thanks for the suggestion, I like that name much better. Changed it and updated customizations.json file as well.

generator/ServiceClientGeneratorLib/Customizations.cs

normj · 2025-06-24T15:19:41Z

generator/ServiceClientGeneratorLib/Customizations.cs

+        /// <summary>
+        /// For Lists of enums, the generator automatically treats the enum as a string via passing in true to "treatEnumAsString".
+        /// This customization overrides that so that the enum is treated as an enum. 
+        ///     "treatEnumsAsEnums":{


You need to update the example to match the rename to OverrideTreatEnumsAsString

normj · 2025-06-24T15:23:10Z

generator/ServiceModels/s3/s3.customizations.json

+
+    },
+    "overrideTreatEnumsAsString":{
+        "ObjectAttributesList": true


Shouldn't this be false since we want to turn off treating the enum as a string. I'm viewing this customization is allowing the user manually specify the exact value for the treatEnumAsString parameter on DetermineType.

since DetermineType() by default passes in false for treatEnumsAsString and there is no way to call DetermineType(JsonData extendedData, bool treatEnumsAsString, bool useNullable = true) outside the Member.cs class I was thinking that this customization would override the default value which is why I passed in true here meaning. "Override the default value of false".

normj · 2025-06-24T15:24:38Z

generator/ServiceClientGeneratorLib/Customizations.cs

+        {
+            var data = _documentRoot[OverrideTreatEnumsAsStringKey];
+            if (data == null) return false;
+            return data[shapeName]  == null ? false : true;


Since the objects in the list have a value of true or false whether we should treat enums as strings we should check the value here and not just whether it exists.

since we're now changing what OverrideTreatEnumsAsString represents, I think we have to return null if the value doesn't exist, and otherwise just return the value.

normj · 2025-06-24T15:26:29Z

generator/ServiceClientGeneratorLib/Member.cs

                case "list":
-                    var listType = DetermineType(memberShape["member"], true, false);
+                    // if customization exists to treat the enum as an enum instead of a string, we pass in false
+                    var listType = this.model.Customizations.OverrideTreatEnumsAsString(this.Extends) ? DetermineType(memberShape["member"], false, false) : DetermineType(memberShape["member"], true, false) ;


This ?: can be reduced down DetermineType(memberShape["member"], this.model.Customizations.OverrideTreatEnumsAsString(this.Extends), false)

dscpinheiro · 2025-06-24T22:02:10Z

generator/ServiceClientGeneratorLib/Member.cs

                case "map":
-                    var keyType = DetermineType(memberShape["key"], true, false);
-                    var valueType = DetermineType(memberShape["value"], true, false);
+                    bool overrideMapTreatEnumsAsString = this.model.Customizations.OverrideTreatEnumsAsString(this.Extends) ?? true;


Why do we need to use Extends here?

I used Extends here b/c I'm just trying to grab the name of the shape that the member targets rather than than the name of the member itself since this is a shape-level modification and not a member-level modification

…es, GetObjectLegalHold, GetObjectLockConfiguration

Copilot

Pull Request Overview

This pull request generates 8 additional S3 operations: GetBucketTagging, GetBucketRequestPayment, GetObjectAttributes, GetObjectLegalHold, GetObjectLockConfiguration, GetObjectRetention, GetObjectTagging, and GetPublicAccessBlock. The changes involve removing custom implementation files to enable code generation for these operations and introducing a new customization "TreatEnumsAsEnum" to handle List types properly in S3.

Key changes:

Removed custom model and marshaller files for several S3 operations to enable automatic code generation
Added "TreatEnumsAsEnum" customization to override enum handling for specific shapes
Updated service model configuration to enable generation of the 8 new operations

Reviewed Changes

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

Show a summary per file

File	Description
PublicAccessBlockConfiguration.cs	Removed custom model to enable code generation
GetPublicAccessBlockRequestMarshaller.cs	Removed custom marshaller to enable code generation
GetObjectTaggingRequestMarshaller.cs	Removed custom marshaller to enable code generation
GetBucketTaggingRequestMarshaller.cs	Removed custom marshaller to enable code generation
GetBucketRequestPaymentRequestMarshaller.cs	Removed custom marshaller to enable code generation
GetObjectTaggingResponse.cs	Removed custom response model to enable code generation
GetBucketTaggingResponse.cs	Removed custom response model to enable code generation
s3.customizations.json	Added customizations for enum handling and property naming
ServiceModel.cs	Enabled 8 new S3 operations and updated exception handling lists
Member.cs	Added logic to respect TreatEnumsAsString override customization
GeneratorDriver.cs	Minor whitespace cleanup
Customizations.cs	Added OverrideTreatEnumsAsString customization support
816a028b-eb1b-4f17-9d9c-a0373bcd8ced.json	Added changelog entry for the new operations

Copilot · 2025-07-30T17:43:27Z

generator/ServiceModels/s3/s3.customizations.json

            "renameShape": "S3StorageClass"
+        },
+        "Payer":{
+            "renameShape": "string"


The shape rename from 'Payer' to 'string' appears to contradict the PR's goal of treating enums as enums rather than strings. This should be consistent with the new TreatEnumsAsEnum customization approach.

Suggested change

"renameShape": "string"

"renameShape": "PayerType"

peterrsongg changed the title ~~Generate~~ Generate 8 more s3 operations Jun 23, 2025

normj reviewed Jun 23, 2025

View reviewed changes

peterrsongg requested a review from normj June 23, 2025 22:01

normj reviewed Jun 24, 2025

View reviewed changes

peterrsongg requested a review from normj June 24, 2025 17:04

boblodgett requested a review from dscpinheiro June 24, 2025 17:28

normj approved these changes Jun 24, 2025

View reviewed changes

dscpinheiro reviewed Jun 24, 2025

View reviewed changes

peterrsongg requested a review from dscpinheiro June 25, 2025 04:13

dscpinheiro approved these changes Jun 25, 2025

View reviewed changes

peterrsongg added 7 commits June 25, 2025 14:48

generate GetBucketRequestPayment, GetBucketTagging, GetObjectAttribut…

4fffe50

…es, GetObjectLegalHold, GetObjectLockConfiguration

Generate GetObjectRetention and GetObjectTagging, GetPublicAccessBlock

a791745

update devconfig

6d9ccb7

address PR feedback

b84955a

update overrideTreatEnumsAsString customization

a5779fb

Update value from true to false

5e51201

Add overrideTreatEnumsAsString logic to map as well

e70b14d

peterrsongg force-pushed the petesong/generate-s3-phase-1-pr-5 branch from cb07319 to e70b14d Compare June 25, 2025 21:52

peterrsongg merged commit be37954 into development Jun 26, 2025
3 checks passed

peterrsongg deleted the petesong/generate-s3-phase-1-pr-5 branch June 26, 2025 02:36

GarrettBeatty requested a review from Copilot July 30, 2025 17:43

Copilot AI reviewed Jul 30, 2025

View reviewed changes

Generate 8 more s3 operations #3893

Generate 8 more s3 operations #3893

Uh oh!

Conversation

peterrsongg commented Jun 23, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

peterrsongg Jun 24, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterrsongg Jun 24, 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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peterrsongg commented Jun 23, 2025 •

edited

Loading

peterrsongg Jun 24, 2025 •

edited

Loading

peterrsongg Jun 24, 2025 •

edited

Loading