Skip to content

Conversation

@peterrsongg
Copy link
Contributor

@peterrsongg peterrsongg commented Nov 24, 2025

Description

Generates GetBucketInventoryConfiguration

Nothing from the Assembly comparer.

Motivation and Context

Testing

DRY_RUN Build id: 207f32c4-32c7-4ed1-bb1e-fea3028bf563 succeeded

FUZZ_TEST results
One difference is how we treat empty responses. However, the docs here: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketInventoryConfiguration.html say that the root-level tag is required so this scenario won't happen.

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: #4154, branch: peterrsongg/petesong/phase-3-pr4-3/1
@peterrsongg peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr4-3/2 branch from cda4f5e to ce45e13 Compare November 24, 2025 06:10
@peterrsongg
Copy link
Contributor Author

Breaking Changes Analysis - GetBucketInventoryConfiguration Migration

Summary

I have thoroughly analyzed all 11 files that were migrated from Custom to Generated code for the GetBucketInventoryConfiguration operation.

RESULT: NO BREAKING CHANGES FOUND


DETAILED ANALYSIS

Files Analyzed: 11/11

Request/Response Models (2)

  1. GetBucketInventoryConfigurationRequest.cs

    • Property rename: IdInventoryId (intentional, configured in customizations.json)
    • IsSetExpectedBucketOwner(): Custom code used !string.IsNullOrEmpty, Generated code uses !String.IsNullOrEmpty (identical logic, just case difference)
    • IsSetInventoryId(): Both use != null check
    • Status: No breaking changes
  2. GetBucketInventoryConfigurationResponse.cs

    • Contains InventoryConfiguration property
    • Logic identical between custom and generated
    • Status: No breaking changes

Marshallers (2)

  1. GetBucketInventoryConfigurationRequestMarshaller.cs

    • Generated version includes PreMarshallCustomization and PostMarshallCustomization partial methods for extensibility
    • Marshalling logic is functionally equivalent
    • Properly handles InventoryId parameter with query string
    • Status: No breaking changes
  2. GetBucketInventoryConfigurationResponseUnmarshaller.cs

    • Generated version includes PostUnmarshallCustomization partial method for extensibility
    • Unmarshalling logic is functionally equivalent
    • Status: No breaking changes

Unmarshallers (7)

  1. InventoryConfigurationUnmarshaller.cs

    • Generated version includes XmlStructureUnmarshallCustomization partial method
    • All property unmarshalling preserved (Destination, IncludedObjectVersions, InventoryFilter, InventoryId, InventoryOptionalFields, IsEnabled, Schedule)
    • Status: No breaking changes
  2. InventoryDestinationUnmarshaller.cs

    • Unmarshalls S3BucketDestination property
    • Logic identical, includes customization hook
    • Status: No breaking changes
  3. InventoryEncryptionUnmarshaller.cs

    • Unmarshalls SSEKMS and SSES3 properties
    • Logic identical to custom version
    • Status: No breaking changes
  4. InventoryS3BucketDestinationUnmarshaller.cs

    • Custom: condition.InventoryFormat = InventoryFormat.FindValue(StringUnmarshaller.GetInstance().Unmarshall(context));
    • Generated: unmarshalledObject.InventoryFormat = unmarshaller.Unmarshall(context);
    • Note: Both are functionally equivalent. AWS SDK enum types (like InventoryFormat) support implicit string conversion, so direct assignment works the same as FindValue()
    • All properties preserved: AccountId, BucketName, InventoryEncryption, InventoryFormat, Prefix
    • Status: No breaking changes
  5. InventoryScheduleUnmarshaller.cs

    • Custom: condition.Frequency = InventoryFrequency.FindValue(StringUnmarshaller.GetInstance().Unmarshall(context));
    • Generated: unmarshalledObject.Frequency = unmarshaller.Unmarshall(context);
    • Note: Both are functionally equivalent due to implicit string conversion in InventoryFrequency type
    • Status: No breaking changes
  6. SSEKMSUnmarshaller.cs

    • Unmarshalls KeyId property
    • Logic identical to custom version
    • Status: No breaking changes
  7. SSES3Unmarshaller.cs

    • Empty shape unmarshaller (SSES3 has no properties)
    • Logic identical to custom version
    • Status: No breaking changes

KEY FINDINGS

Intentional Changes (Non-Breaking)

  1. Property Naming: IdInventoryId in GetBucketInventoryConfigurationRequest

    • Explicitly configured in s3.customizations.json: "Id":{"emitPropertyName":"InventoryId"}
    • Public API maintains consistency
  2. Code Patterns: Generated code uses modern AWS SDK patterns:

    • Partial methods for customization hooks (PreMarshallCustomization, PostMarshallCustomization, XmlStructureUnmarshallCustomization)
    • Singleton pattern with static Instance property instead of GetInstance() method
    • Implicit enum conversion instead of explicit FindValue() calls

Verification Against Breaking Change Criteria

  1. Logical conditions changed/removed: None found
  2. Getter/setter code changed: None found (private variable names don't count)
  3. Properties no longer set: All properties preserved
  4. Response output changes: None found
  5. IsSet method changes from !string.IsNullOrEmpty to != null: None found (IsSetExpectedBucketOwner uses same logic in both versions)
  6. Public property backward compatibility issues: None found (intentional rename documented in customizations)

CONCLUSION

All 11 files have been successfully migrated from Custom to Generated code with NO BREAKING CHANGES.

The migration correctly:

  • Preserves all business logic
  • Maintains backward compatibility
  • Uses modern generated code patterns
  • Follows configured customizations
  • Adds extensibility through partial methods

This PR is safe to merge from a breaking changes perspective.

peterrsongg added a commit that referenced this pull request Nov 24, 2025
stack-info: PR: #4155, branch: peterrsongg/petesong/phase-3-pr4-3/2
stack-info: PR: #4154, branch: peterrsongg/petesong/phase-3-pr4-3/1
@peterrsongg peterrsongg marked this pull request as ready for review November 24, 2025 09:05
{
IRequest request = new DefaultRequest(getInventoryConfigurationRequest, "Amazon.S3");

request.Suppress404Exceptions = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this Suppress404Exceptions in the generated marshaller. Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nope that wasn't intentional, i just missed adding it. I'll add it here!

peterrsongg added a commit that referenced this pull request Nov 25, 2025
stack-info: PR: #4155, branch: peterrsongg/petesong/phase-3-pr4-3/2
stack-info: PR: #4155, branch: peterrsongg/petesong/phase-3-pr4-3/2
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR generates the GetBucketInventoryConfiguration operation for the AWS S3 SDK, replacing handwritten code with generated code. The change migrates several unmarshaller classes from the Custom folder to the Generated folder, standardizing them with the code generation pattern used throughout the SDK. The PR notes that fuzz testing showed differences in handling empty responses, but this scenario won't occur in practice since the root-level tag is required per AWS documentation.

Key changes:

  • Enables code generation for GetBucketInventoryConfiguration operation
  • Migrates custom unmarshallers to generated versions with updated patterns (singleton initialization, naming conventions, partial methods)
  • Adds customizations in s3.customizations.json for property name mappings

Reviewed changes

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

Show a summary per file
File Description
ServiceModel.cs Uncomments GetBucketInventoryConfiguration in S3 allowlist to enable generation
s3.customizations.json Adds property name customizations for InventoryId and ExpectedBucketOwner
GetBucketInventoryConfigurationRequest.cs Updated to generated version with improved documentation and validation
GetBucketInventoryConfigurationResponse.cs Updated to generated version with consistent field naming
GetBucketInventoryConfigurationRequestMarshaller.cs Replaces custom marshaller with generated version
GetBucketInventoryConfigurationResponseUnmarshaller.cs Replaces custom unmarshaller with generated version
InventoryConfigurationUnmarshaller.cs Migrated from Custom to Generated folder with updated pattern
SSES3Unmarshaller.cs New generated unmarshaller (previously in Custom folder, now deleted)
SSEKMSUnmarshaller.cs Updated to generated pattern with consistent formatting
InventoryScheduleUnmarshaller.cs Updated to generated pattern with implicit conversion for enums
InventoryS3BucketDestinationUnmarshaller.cs Updated to generated pattern with reordered field processing
InventoryEncryptionUnmarshaller.cs Updated to generated pattern with consistent formatting
InventoryDestinationUnmarshaller.cs Updated to generated pattern with consistent formatting
Custom/Model/Internal/MarshallTransformations/* Deleted custom versions replaced by generated code

if (string.IsNullOrEmpty(publicRequest.BucketName))
throw new System.ArgumentException("BucketName is a required property and must be set before making this call.", "GetBucketInventoryConfigurationRequest.BucketName");
if (string.IsNullOrEmpty(publicRequest.InventoryId))
throw new AmazonS3Exception("Request object does not have required field InventoryId set");
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message for missing InventoryId is inconsistent with the error message for BucketName on line 68. The BucketName error uses ArgumentException with a descriptive message including the parameter name, while InventoryId uses AmazonS3Exception with a less descriptive message. For consistency and better error handling, consider using: throw new System.ArgumentException(\"InventoryId is a required property and must be set before making this call.\", \"GetBucketInventoryConfigurationRequest.InventoryId\");

Suggested change
throw new AmazonS3Exception("Request object does not have required field InventoryId set");
throw new System.ArgumentException("InventoryId is a required property and must be set before making this call.", "GetBucketInventoryConfigurationRequest.InventoryId");

Copilot uses AI. Check for mistakes.
@peterrsongg peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr4-3/2 branch from ce45e13 to 0880c10 Compare November 26, 2025 17:53
@peterrsongg peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr4-3/2 branch from 5044106 to 533a0b0 Compare November 26, 2025 19:24
@peterrsongg peterrsongg force-pushed the peterrsongg/petesong/phase-3-pr4-3/1 branch from 55e70ac to 135b1f6 Compare November 26, 2025 19:33
@peterrsongg peterrsongg changed the base branch from peterrsongg/petesong/phase-3-pr4-3/1 to petesong/phase-3-pr4-base November 26, 2025 20:22
@peterrsongg peterrsongg merged commit 47bb5fe into petesong/phase-3-pr4-base Nov 26, 2025
1 check passed
peterrsongg added a commit that referenced this pull request Nov 26, 2025
* Generate PutBucketInventoryConfiguration

stack-info: PR: #4154, branch: peterrsongg/petesong/phase-3-pr4-3/1

* Generate GetBucketInventoryConfiguration

stack-info: PR: #4155, branch: peterrsongg/petesong/phase-3-pr4-3/2

* Generate ListBucketInventoryConfigurations

stack-info: PR: #4156, branch: peterrsongg/petesong/phase-3-pr4-3/3

* Geneate DeleteBucketInventoryConfiguration

stack-info: PR: #4157, branch: peterrsongg/petesong/phase-3-pr4-3/4

* Generate PutBucketAcclerateConfiguration

stack-info: PR: #4158, branch: peterrsongg/petesong/phase-3-pr4-3/5
peterrsongg added a commit that referenced this pull request Nov 26, 2025
* Generate PutBucketInventoryConfiguration

stack-info: PR: #4154, branch: peterrsongg/petesong/phase-3-pr4-3/1

* Generate GetBucketInventoryConfiguration

stack-info: PR: #4155, branch: peterrsongg/petesong/phase-3-pr4-3/2

* Generate ListBucketInventoryConfigurations

stack-info: PR: #4156, branch: peterrsongg/petesong/phase-3-pr4-3/3

* Geneate DeleteBucketInventoryConfiguration

stack-info: PR: #4157, branch: peterrsongg/petesong/phase-3-pr4-3/4

* Generate PutBucketAcclerateConfiguration

stack-info: PR: #4158, branch: peterrsongg/petesong/phase-3-pr4-3/5

* Generate PutBucketRequestPayment

stack-info: PR: #4159, branch: peterrsongg/petesong/phase-3-pr4-3/6
@dscpinheiro dscpinheiro deleted the peterrsongg/petesong/phase-3-pr4-3/2 branch November 27, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants