Generate GetBucketMetricsConfiguration

[peterrsongg]

Description

PR no.2 of phase 3

Continuation of this PR

Generates GetBucketMetricsConfiguration

Motivation and Context

Testing

Dry run passed

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

[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.

[dscpinheiro]

I think I missed the stand-up where we discussed this, but we're using the service exception here and System.ArgumentException above.

We weren't validating the metrics ID before (https://github.com/aws/aws-sdk-net/blob/main/sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/GetBucketMetricsConfigurationRequestMarshaller.cs#L49), so would it make sense to use the same exception as missing bucket name?

[dscpinheiro]

I missed this in the other PRs but it applies to the list and delete operations marshallers as well.

[peterrsongg]

So the reason I made it a service-level exception is because from my testing, when these required parameters aren't set, the SDK will throw AmazonS3Exception. So to not make it a breaking change I purposely made it throw AmazonS3Exception (this is actually what we do for resource path params too but S3 is just weird and we don't follow that)

[dscpinheiro]

(Forgot to publish this one...)

Do we have an integration test for this operation? The manual code had a section I don't see here (or the updated metrics unmarshaller):

aws-sdk-net/sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/GetBucketMetricsConfigurationResponseUnmarshaller.cs

Lines 58 to 59 in eb13820

	if (context.IsStartOfDocument)
	targetDepth += 2;

[peterrsongg]

https://github.com/aws/aws-sdk-net/blob/main/sdk/test/Services/S3/IntegrationTests/StorageInsightsMetricsTests.cs#L65-L107\

yup we do have integ tests for this :)

[peterrsongg]

just to explain though, the difference stems from the custom code not using the MetricsConfigurationUnmarshaller and trying to unmarshall the objects inside MetricsConfiguration. so it has to go 2 levels deep. Since here we are calling MetricsConfigurationUnmarshaller, we only have to go one level deep to find the MetricsConfiguration element. Then once we find that element we just go into the MetricsConfigurationUnmarshaller logic