Generate PutBucketMetricsConfiguration and add required check for query string parameters

[peterrsongg]

…equired query string params are set. Remove visitor pattern and move custom logic for putlifecycleconfiguration marshaller to custom partial method

Description

This PR generates the S3 operation PutBucketMetricsConfiguration. It also adds a check to see if a required query string member is set in the marshallers, which also affects other services as well. This is necessary so that a different operation isn't called if a required query string parameter isn't set.

This also moves some code to partial methods for PutLifecycleConfigurationRequest, since I removed the ImplementsVisitorPattern customization hook since generating PutBucketMetricsConfiguration using that customization hook logic wasn't possible. I decided to just move that logic to a custom partial so we can have an exact 1:1 mapping of that previous logic.

Best way to review is by pulling up the custom version of the file on one side and the generated version of the file on the other side and compare.

Motivation and Context

Testing

#4090 dry run passed (contains 4 generated operations and this branch is branched off of this PR)

Screenshots (if appropriate)

fuzz-tests for the larger PR all passed.
See output of larger PR's AI prompt (all passed)

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
• All new and existing tests passed

License

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

[peterrsongg]

since we are validating query string params now we must populate UploadId. Also, this test was only validating the resource path and not that the operation worked. If this was actually to be called, it would've turned into a DeleteObjects call.

[peterrsongg]

Throwing the base exception to be consistent with what we do for resource path members

[GarrettBeatty]

nit: can you make it consistent with having { for the if statements. im having a hard time figuring out where the if (<#=variableName#>.<#=member.PropertyName#> == null) case is applying. if its inside an if or else block i cant tell easily

[peterrsongg]

This is the code we will generate and not the actual logic, so I kept the brackets off to be consistent with other places where we throw exceptions

[GarrettBeatty]

is there any reason to not ship this change (query string required field) as its own thing? in case there is some issue that arises.

[peterrsongg]

hmm, PutBucketMetricsConfiguration has a required query string so even this operation uses this logic. I do understand the concern though. We do really need to ship the change for AbortMPU though since that is the one where the operation can actually change, so I'd rather just ship it in this PR😅

[GarrettBeatty]

i was thinking if its not strictly required, then could just do it separate to make it easier to review and rollback if ever required in the future. will let david/muhammad decide though

[dscpinheiro]

Who's David? 🤔

I prefer the smaller PRs, they're easier to review and make me more confident we're not shipping anything we shouldn't.

[GarrettBeatty]

lol woops daniel :)

[peterrsongg]

gotcha, just to be clear we're talking about shipping it for S3 then all other services separately?

I can make the change but I would have to

1. add a condition in the generator to only do this for S3
2. Have a separate PR that just removes that condition.

If we're okay with that I'll go ahead and do it

[Copilot]

Pull Request Overview

This PR generates code for S3's PutBucketMetricsConfiguration operation and adds validation for required query string parameters across the AWS SDK for .NET. The changes include:

• Generating the PutBucketMetricsConfiguration operation for S3
• Replacing hand-written marshaller/unmarshaller code with generated code
• Adding validation to ensure required query string parameters are set before making requests
• Refactoring the code generation approach from a visitor pattern to an injection-based approach for custom marshalling logic

Reviewed Changes

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

Show a summary per file

File	Description
sdk/test/Services/S3/UnitTests/Custom/S3AccessPointTests.cs	Adds missing UploadId property to test request
sdk/src/Services/S3/Generated/Model/PutBucketMetricsConfiguration*.cs	Generated request/response models with updated documentation and field naming conventions
sdk/src/Services/S3/Generated/Model/MetricsConfiguration.cs	Generated model with updated documentation and field naming
sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/*.cs	Generated marshallers with required field validation for query string parameters
sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/PutLifecycleConfigurationRequestMarshaller.cs	Extracts filter marshalling logic to a custom method
sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/PutBucketMetricsConfiguration*.cs	Custom marshaller/unmarshaller with filter marshalling and checksum logic
generator/ServiceModels/s3/s3.customizations.json	Configuration updates for code generation, removes visitor pattern
generator/ServiceClientGeneratorLib/*.cs	Updates code generator to use injection-based approach instead of visitor pattern
generator/.DevConfigs/*.json	Dev config files documenting the changes

[peterrsongg]

I'm pasting the results of the AI analysis done on the larger PR here
Updated AI prompt run on latest revision.

The added validation isn't a breaking change because the request will fail and throw AmazonS3Exception and since we are throwing the same exception but just earlier in the api call it isn't a breaking change. I intentionally made it the base service exception instead of a System.ArgumentException for that reason.

Now I have all the information needed to provide a comprehensive analysis. Let me compare the deleted custom files with the generated ones and examine all the marshaller changes:

Task Completed

Breaking Changes Analysis - S3 Custom to Generated Migration

After analyzing all deleted custom files, their generated replacements, and modified marshallers, I have identified the following breaking changes:

BREAKING CHANGES IDENTIFIED:

1. AbortMultipartUploadRequestMarshaller.cs - NEW VALIDATION ADDED

Issue: Added new required field validation that didn't exist before:

if (string.IsNullOrEmpty(publicRequest.UploadId))
    throw new AmazonS3Exception("Request object does not have required field UploadId set");

Impact: Customer code that previously could make AbortMultipartUpload requests without UploadId set will now throw an exception.

2. CopyPartRequestMarshaller.cs - NEW VALIDATIONS ADDED

Issue: Added two new required field validations:

if (publicRequest.PartNumber == null)
    throw new AmazonS3Exception("Request object does not have required field PartNumber set");
if (string.IsNullOrEmpty(publicRequest.UploadId))
    throw new AmazonS3Exception("Request object does not have required field UploadId set");

Impact: Customer code that previously could make CopyPart requests without PartNumber or UploadId set will now throw exceptions.

3. ListPartsRequestMarshaller.cs - NEW VALIDATION ADDED

Issue: Added new required field validation:

if (string.IsNullOrEmpty(publicRequest.UploadId))
    throw new AmazonS3Exception("Request object does not have required field UploadId set");

Impact: Customer code that previously could make ListParts requests without UploadId set will now throw an exception.

NO BREAKING CHANGES FOUND IN:

Model Files Migration (✅ SAFE)

• DeleteBucketMetricsConfiguration Request/Response
• GetBucketMetricsConfiguration Request/Response
• ListBucketMetricsConfigurations Request/Response
• MetricsConfiguration
• PutBucketMetricsConfiguration Request/Response

Differences are non-breaking:

• Private field naming changes (bucketName → _bucketName)
• Added [AWSProperty(Required=true)] attributes (not breaking for existing code)
• Minor documentation text changes
• Collection initialization patterns updated to newer SDK standards

Marshaller Logic Preservation (✅ SAFE)

• GetBucketMetricsConfigurationRequestMarshaller: Custom PreMarshallCustomization (Suppress404Exceptions) properly preserved
• PutBucketMetricsConfigurationRequestMarshaller: Custom MetricsFilterCustomMarshall and PostMarshallCustomization (checksum logic) properly preserved through partial methods
• PutLifecycleConfigurationRequestMarshaller: Filter marshalling logic properly moved to FilterCustomMarshall method

SUMMARY:

• Files analyzed: 30 out of 30 total changed files
• Breaking changes found: 3 files with new required field validations
• Safe migrations: 27 files with logic properly preserved

The breaking changes are all related to new validation logic being added to existing marshallers rather than logic being lost during migration. These validations enforce that required fields are set before making API calls, which technically makes the SDK more strict than before.