Generate PutBucketAnalyticsConfiguration #4126

peterrsongg · 2025-11-13T22:55:29Z

Description

2 of 5 stacked PRs. Generates PutBucketAnalyticsConfiguration.
I will create one DevConfig with all these in merge PR.

Note

I didn't change IsSet for AnalyticsId and BucketName because we already check if String.IsNullOrEmpty in the marshaller and fail the request if not set. THere is no need to change the IsSet and I want to keep customizations at a minimum. Responses don't use IsSet so it doesn't matter there.

Motivation and Context

Testing

Fuzz-Testing results good
Base-Branch 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

stack-info: PR: #4126, branch: peterrsongg/petesong/phase-3-pr-2-all/2

peterrsongg · 2025-11-14T18:32:03Z

Breaking Changes Analysis Report

Summary

Analyzed 69 files changed in this PR migrating S3 Analytics and CreateSession operations from Custom to Generated code.

Total Breaking Changes Found: 5 files with issues

BREAKING CHANGES DETECTED

1. AnalyticsConfiguration.cs

File: sdk/src/Services/S3/Generated/Model/AnalyticsConfiguration.cs

ISSUE: Internal validation method logic changed

Old Custom: return !(string.IsNullOrEmpty(this.analyticsId));
New Generated: return this._analyticsId != null;
Impact: The old code checked for both null AND empty strings, new code only checks for null. An empty string will now pass validation when it shouldn't.

2. AnalyticsExportDestination.cs

File: sdk/src/Services/S3/Generated/Model/AnalyticsExportDestination.cs

ISSUE: Added Required attribute to S3BucketDestination property

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: This property is now marked as required at the model level. This could cause validation errors for existing code that doesn't set this property.

3. AnalyticsS3BucketDestination.cs

File: sdk/src/Services/S3/Generated/Model/AnalyticsS3BucketDestination.cs

ISSUES (Multiple):

a) BucketName Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: BucketName is now required when previously it was optional

b) Format Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: Format is now required when previously it was optional

c) Internal Validation Methods Changed

BucketAccountId:
- Old: return !(string.IsNullOrEmpty(this.accountId));
- New: return !String.IsNullOrEmpty(this._bucketAccountId);
- Impact: Changed from checking both null and empty to using String.IsNullOrEmpty (functionally equivalent)
BucketName:
- Old: return !(string.IsNullOrEmpty(this.bucketName));
- New: return !String.IsNullOrEmpty(this._bucketName);
- Impact: Changed from checking both null and empty to using String.IsNullOrEmpty (functionally equivalent)
Prefix:
- Old: return !(string.IsNullOrEmpty(this.prefix));
- New: return !String.IsNullOrEmpty(this._prefix);
- Impact: Changed from checking both null and empty to using String.IsNullOrEmpty (functionally equivalent)

4. StorageClassAnalysisDataExport.cs

File: sdk/src/Services/S3/Generated/Model/StorageClassAnalysisDataExport.cs

ISSUES (Multiple):

a) Destination Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: Destination property is now required when previously it was optional

b) OutputSchemaVersion Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: OutputSchemaVersion is now required when previously it was optional

5. GetBucketAnalyticsConfigurationRequest.cs

File: sdk/src/Services/S3/Generated/Model/GetBucketAnalyticsConfigurationRequest.cs

ISSUES (Multiple):

a) AnalyticsId Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: AnalyticsId is now required when previously it was optional

b) BucketName Required Attribute Added

Old Custom: No [AWSProperty(Required=true)] attribute
New Generated: [AWSProperty(Required=true)] attribute added
Impact: BucketName is now required when previously it was optional

c) Internal Validation Methods Changed

AnalyticsId:
- Old: return !(string.IsNullOrEmpty(this.analyticsId));
- New: return this._analyticsId != null;
- Impact: Old checked for both null AND empty strings, new only checks null. Empty string will now pass validation.
BucketName:
- Old: return !(string.IsNullOrEmpty(this.bucketName));
- New: return this._bucketName != null;
- Impact: Old checked for both null AND empty strings, new only checks null. Empty string will now pass validation.

VERIFIED AS CORRECT

The following changes were verified as correctly preserving existing behavior:

Custom Marshallers: Logic properly split between Generated and Custom files using partial methods:
- GetBucketAnalyticsConfigurationRequestMarshaller - PreMarshallCustomization preserved
- PutBucketAnalyticsConfigurationRequestMarshaller - AnalyticsFilterCustomMarshall and PostMarshallCustomization preserved
- AnalyticsS3BucketDestinationUnmarshaller - FormatCustomUnmarshall preserved
SessionCredentials.cs - No breaking changes (identical behavior)
CreateSessionRequest.cs - No breaking changes (field name changes only, getters/setters identical)
StorageClassAnalysis.cs - No breaking changes (private field name change only)

Files Analyzed: 69/69 (100%)

Model Files Analyzed (16):

✅ AnalyticsConfiguration.cs ✅ AnalyticsExportDestination.cs
✅ AnalyticsS3BucketDestination.cs ✅ CreateSessionRequest.cs ✅ CreateSessionResponse.cs ✅ DeleteBucketAnalyticsConfigurationRequest.cs ✅ DeleteBucketAnalyticsConfigurationResponse.cs ✅ GetBucketAnalyticsConfigurationRequest.cs ✅ GetBucketAnalyticsConfigurationResponse.cs ✅ ListBucketAnalyticsConfigurationsRequest.cs ✅ ListBucketAnalyticsConfigurationsResponse.cs ✅ PutBucketAnalyticsConfigurationRequest.cs ✅ PutBucketAnalyticsConfigurationResponse.cs ✅ SessionCredentials.cs ✅ StorageClassAnalysis.cs ✅ StorageClassAnalysisDataExport.cs

Marshaller/Unmarshaller Files Analyzed (23):

✅ All marshaller and unmarshaller files verified for custom logic preservation

Configuration Files Analyzed (2):

✅ generator/ServiceClientGeneratorLib/ServiceModel.cs ✅ generator/ServiceModels/s3/s3.customizations.json

CRITICAL IMPACT SUMMARY

The most severe breaking changes are:

String validation changes that no longer reject empty strings (AnalyticsConfiguration, GetBucketAnalyticsConfigurationRequest)
Required attributes added to previously optional properties across multiple models, which will break existing code that doesn't set these properties

These changes violate backward compatibility and will cause runtime errors for existing customer code.

Copilot

Pull Request Overview

This PR generates the PutBucketAnalyticsConfiguration operation and related models by migrating from custom handwritten code to generated code based on the S3 service model. This is part 2 of a 5-part stacked PR series, with a dev config to be added in the final merge PR.

Key Changes:

Migrates PutBucketAnalyticsConfigurationRequest, PutBucketAnalyticsConfigurationResponse, and related model classes from custom to generated code
Preserves custom marshalling logic for AnalyticsFilter through the AnalyticsFilterCustomMarshall partial method
Adds generator customizations to maintain compatibility (property name mappings, custom IsSet logic, and string-based Format handling)

Reviewed Changes

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

Show a summary per file

File	Description
`generator/ServiceClientGeneratorLib/ServiceModel.cs`	Enables code generation for PutBucketAnalyticsConfiguration by uncommenting the operation
`generator/ServiceModels/s3/s3.customizations.json`	Adds customizations for property name mappings (Id→AnalyticsId, Filter→AnalyticsFilter), custom IsSet implementations, custom marshalling injection for AnalyticsFilter, and Format property type configuration
`sdk/src/Services/S3/Custom/Model/AnalyticsConfiguration.cs`	Deleted - replaced with generated version
`sdk/src/Services/S3/Custom/Model/AnalyticsS3BucketDestination.cs`	Deleted - replaced with generated version
`sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/PutBucketAnalyticsConfigurationRequestMarshaller.cs`	Refactored to retain only custom logic: AnalyticsFilterCustomMarshall for filter marshalling and PostMarshallCustomization for checksum handling
`sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/PutBucketAnalyticsConfigurationResponseUnmarshaller.cs`	Deleted - replaced with generated version that includes error handling and partial customization hooks
`sdk/src/Services/S3/Generated/Model/AnalyticsConfiguration.cs`	New generated model with properly documented properties and Required attributes
`sdk/src/Services/S3/Generated/Model/AnalyticsExportDestination.cs`	Updated to generated version with improved documentation and standardized field naming
`sdk/src/Services/S3/Generated/Model/AnalyticsS3BucketDestination.cs`	New generated model replacing custom implementation, with Format changed to string type (maintains compatibility via implicit conversion)
`sdk/src/Services/S3/Generated/Model/PutBucketAnalyticsConfigurationRequest.cs`	Updated to generated version with enhanced documentation, standardized field naming (_fieldName convention), and preserved validation logic
`sdk/src/Services/S3/Generated/Model/PutBucketAnalyticsConfigurationResponse.cs`	Updated to generated version with cleaner structure
`sdk/src/Services/S3/Generated/Model/StorageClassAnalysis.cs`	Updated to generated version with improved documentation and standardized field naming
`sdk/src/Services/S3/Generated/Model/StorageClassAnalysisDataExport.cs`	Updated to generated version with improved documentation, Required attributes, and reordered properties (Destination before OutputSchemaVersion)
`sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/PutBucketAnalyticsConfigurationRequestMarshaller.cs`	New generated marshaller that calls custom AnalyticsFilterCustomMarshall and PostMarshallCustomization methods, with updated parameter handling using request.Parameters.Add + UseQueryString pattern
`sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/PutBucketAnalyticsConfigurationResponseUnmarshaller.cs`	New generated unmarshaller with comprehensive error handling and partial customization support

Copilot · 2025-11-14T18:36:56Z

...d/Model/Internal/MarshallTransformations/PutBucketAnalyticsConfigurationRequestMarshaller.cs

+                    AnalyticsFilterCustomMarshall(publicRequest, xmlWriter);
+                    if(publicRequest.AnalyticsConfiguration.IsSetAnalyticsId())
+                        xmlWriter.WriteElementString("Id", StringUtils.FromString(publicRequest.AnalyticsConfiguration.AnalyticsId));
+
+                    if (publicRequest.AnalyticsConfiguration.StorageClassAnalysis != null)
+                    {
+                        xmlWriter.WriteStartElement("StorageClassAnalysis");
+                        if (publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport != null)
+                        {
+                            xmlWriter.WriteStartElement("DataExport");
+                            if (publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination != null)
+                            {
+                                xmlWriter.WriteStartElement("Destination");
+                                if (publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination != null)
+                                {
+                                    xmlWriter.WriteStartElement("S3BucketDestination");
+                                    if(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.IsSetBucketAccountId())
+                                        xmlWriter.WriteElementString("BucketAccountId", StringUtils.FromString(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.BucketAccountId));
+                                    if(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.IsSetBucketName())
+                                        xmlWriter.WriteElementString("Bucket", StringUtils.FromString(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.BucketName));
+                                    if(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.IsSetFormat())
+                                        xmlWriter.WriteElementString("Format", StringUtils.FromString(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.Format));
+                                    if(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.IsSetPrefix())
+                                        xmlWriter.WriteElementString("Prefix", StringUtils.FromString(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.Destination.S3BucketDestination.Prefix));
+                                    xmlWriter.WriteEndElement();
+                                }
+                                xmlWriter.WriteEndElement();
+                            }
+                            if(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.IsSetOutputSchemaVersion())
+                                xmlWriter.WriteElementString("OutputSchemaVersion", StringUtils.FromString(publicRequest.AnalyticsConfiguration.StorageClassAnalysis.DataExport.OutputSchemaVersion));


[nitpick] The XML element ordering has changed from the custom marshaller implementation. The old order was: Id → Filter → StorageClassAnalysis → DataExport → OutputSchemaVersion → Destination. The new generated order is: Filter → Id → StorageClassAnalysis → DataExport → Destination → OutputSchemaVersion.

While XML is generally order-independent and your testing confirms this works, consider documenting this ordering change in the PR description for future reference, especially since you mentioned this is part of a migration from custom to generated code.

We have tests for this, right? But seems like something we should verify - I may be misremembering but did we have issues for ordering?

yup we have tests here:

aws-sdk-net/sdk/test/Services/S3/IntegrationTests/StorageInsightsAnalyticsTests.cs

Line 63 in 3cc57db

public void BucketAnalyticsConfigurationsAndFilterTest()

The issue we had for ordering was only for xmlAttributes, we have to write the attribute immediately after writing the element. The ordering of the elements themselves do not matter

Generate PutBucketAnalyticsConfiguration

42efdb7

stack-info: PR: #4126, branch: peterrsongg/petesong/phase-3-pr-2-all/2

peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr-2-all/1 branch from 839af32 to 6a201fb Compare November 13, 2025 22:56

peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr-2-all/2 branch from 46a57f7 to 42efdb7 Compare November 13, 2025 22:56

peterrsongg marked this pull request as ready for review November 14, 2025 17:55

dscpinheiro self-requested a review November 14, 2025 18:20

philasmar requested a review from GarrettBeatty November 14, 2025 18:25

GarrettBeatty requested review from Copilot and removed request for GarrettBeatty November 14, 2025 18:27

Copilot AI reviewed Nov 14, 2025

View reviewed changes

dscpinheiro approved these changes Nov 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Generate PutBucketAnalyticsConfiguration #4126

Generate PutBucketAnalyticsConfiguration #4126

Uh oh!

peterrsongg commented Nov 13, 2025 •

edited

Loading

Uh oh!

peterrsongg commented Nov 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

dscpinheiro Nov 14, 2025

Uh oh!

peterrsongg Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Generate PutBucketAnalyticsConfiguration #4126

Are you sure you want to change the base?

Generate PutBucketAnalyticsConfiguration #4126

Uh oh!

Conversation

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

Uh oh!

Description

Note

Motivation and Context

Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

peterrsongg commented Nov 14, 2025

Breaking Changes Analysis Report

Summary

BREAKING CHANGES DETECTED

1. AnalyticsConfiguration.cs

2. AnalyticsExportDestination.cs

3. AnalyticsS3BucketDestination.cs

4. StorageClassAnalysisDataExport.cs

5. GetBucketAnalyticsConfigurationRequest.cs

VERIFIED AS CORRECT

Files Analyzed: 69/69 (100%)

Model Files Analyzed (16):

Marshaller/Unmarshaller Files Analyzed (23):

Configuration Files Analyzed (2):

CRITICAL IMPACT SUMMARY

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

dscpinheiro Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

peterrsongg Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

peterrsongg commented Nov 13, 2025 •

edited

Loading