Generate GetBucketMetadataConfiguration

[peterrsongg]

Description

Generates GetBucketMetadataConfiguration

Assembly comparison output empty

PS C:\Scripts> .\assembly-comparison.ps1
Switched to branch 'peterrsongg/petesong/phase-3-pr4-3/8'
  AWSSDK.Core.NetStandard net8.0 succeeded (24.8s) → C:\Dev\Repos\aws-sdk-net-staging\sdk\src\Core\bin\Debug\net8.0\AWSSDK.Core.dll
  AWSSDK.S3.NetStandard net8.0 succeeded (11.5s) → bin\Debug\net8.0\AWSSDK.S3.dll

Build succeeded in 39.8s

Workload updates are available. Run `dotnet workload list` for more information.
Switched to branch 'peterrsongg/petesong/phase-3-pr4-3/7'
  AWSSDK.Core.NetStandard net8.0 succeeded (11.3s) → C:\Dev\Repos\aws-sdk-net-staging\sdk\src\Core\bin\Debug\net8.0\AWSSDK.Core.dll
  AWSSDK.S3.NetStandard net8.0 succeeded (8.8s) → bin\Debug\net8.0\AWSSDK.S3.dll

Build succeeded in 21.8s

New property added. Non-breaking

Motivation and Context

Testing

DRY_RUN Build id: 207f32c4-32c7-4ed1-bb1e-fea3028bf563 succeeded
Reviewed Fuzz Test Results

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]

Read THIS!

I've reviewed the breaking change report.

1. The resource path handling change we made here is the same as the one for CreateSession we shouldn't append the bucketName to the resource path since s3 includes the bucketname as part of the host name itself. The operation was still working though.
2. Calling base.ConstructException is the correct pattern
   3 and 4. ErrorUnmarshaller and ErrorDetailUnmarshaller have the same exact logic
3. Using the NullableIntUnmarshaller is the right thing to do since Days is Days?

Breaking Changes Analysis Report

Summary

Total files analyzed: 27 out of 27 files changed in PR Breaking changes found: 5

---

BREAKING CHANGES IDENTIFIED

1. GetBucketMetadataConfigurationRequestMarshaller.cs

Location: sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/GetBucketMetadataConfigurationRequestMarshaller.cs

Issue: Missing bucket name path resource handling in generated marshaller

Custom Implementation (DELETED):

request.AddPathResource("{Bucket}", StringUtils.FromString(publicRequest.BucketName));
request.ResourcePath = "/{Bucket}";

Generated Implementation (NEW):

request.ResourcePath = "/";

Impact: The generated marshaller does NOT add the bucket name to the path resource. This will cause the API request to be malformed and fail. The bucket name must be included in the request path as /{Bucket} but the generated code only sets it to "/".

Breaking Change Type: Request marshalling logic changed - API calls will fail due to missing bucket parameter in URL path.

---

2. GetBucketMetadataConfigurationResponseUnmarshaller.cs

Location: sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/GetBucketMetadataConfigurationResponseUnmarshaller.cs

Issue: Error handling logic has changed in exception unmarshalling

Custom Implementation (DELETED):

return new AmazonS3Exception(errorResponse.Message, innerException, errorResponse.Type, errorResponse.Code, errorResponse.RequestId, statusCode);

Generated Implementation (NEW):

return base.ConstructS3Exception(context, errorResponse, innerException, statusCode);

Impact: The exception construction mechanism has changed from directly creating AmazonS3Exception to using the base class method ConstructS3Exception. While this may not break functionality immediately, it changes how exceptions are constructed and could alter exception properties or behavior that client code depends on.

Breaking Change Type: Exception handling behavior changed.

---

3. InventoryTableConfigurationResultUnmarshaller.cs

Location: sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/InventoryTableConfigurationResultUnmarshaller.cs

Issue: Different unmarshaller used for Error property

Custom Implementation (DELETED):

if (context.TestExpression("Error", targetDepth))
{
    var unmarshaller = ErrorUnmarshaller.Instance;
    unmarshalledObject.Error = unmarshaller.Unmarshall(context);
    continue;
}

Generated Implementation (NEW):

if (context.TestExpression("Error", targetDepth))
{
    var unmarshaller = ErrorDetailsUnmarshaller.Instance;
    unmarshalledObject.Error = unmarshaller.Unmarshall(context);
    continue;
}

Impact: The Error property unmarshaller has changed from ErrorUnmarshaller to ErrorDetailsUnmarshaller. This changes how error details are parsed and could result in different error information being populated or missing fields.

Breaking Change Type: Response unmarshalling logic changed - different unmarshaller for Error property.

---

4. JournalTableConfigurationResultUnmarshaller.cs

Location: sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/JournalTableConfigurationResultUnmarshaller.cs

Issue: Different unmarshaller used for Error property

Custom Implementation (DELETED):

if (context.TestExpression("Error", targetDepth))
{
    var unmarshaller = ErrorUnmarshaller.Instance;
    unmarshalledObject.Error = unmarshaller.Unmarshall(context);
    continue;
}

Generated Implementation (NEW):

if (context.TestExpression("Error", targetDepth))
{
    var unmarshaller = ErrorDetailsUnmarshaller.Instance;
    unmarshalledObject.Error = unmarshaller.Unmarshall(context);
    continue;
}

Impact: Same as issue 3 - The Error property unmarshaller has changed from ErrorUnmarshaller to ErrorDetailsUnmarshaller. This changes how error details are parsed.

Breaking Change Type: Response unmarshalling logic changed - different unmarshaller for Error property.

---

5. RecordExpirationUnmarshaller.cs

Location: sdk/src/Services/S3/Generated/Model/Internal/MarshallTransformations/RecordExpirationUnmarshaller.cs

Issue: Different unmarshaller used for Days property

Custom Implementation (DELETED):

if (context.TestExpression("Days", targetDepth))
{
    var unmarshaller = IntUnmarshaller.Instance;
    unmarshalledObject.Days = unmarshaller.Unmarshall(context);
    continue;
}

Generated Implementation (NEW):

if (context.TestExpression("Days", targetDepth))
{
    var unmarshaller = NullableIntUnmarshaller.Instance;
    unmarshalledObject.Days = unmarshaller.Unmarshall(context);
    continue;
}

Impact: The Days property unmarshaller has changed from IntUnmarshaller to NullableIntUnmarshaller. This changes the nullable behavior of the Days property:

• Old: Used non-nullable int unmarshaller (would default to 0 if missing)
• New: Uses nullable int unmarshaller (will be null if missing)

This is a breaking change because the behavior when Days is absent from the response has changed.

Breaking Change Type: Response unmarshalling logic changed - nullable behavior of Days property changed.

---

Files Analyzed (27 Total)

Model Files Compared (7):

• DestinationResult.cs - NO ISSUES
• GetBucketMetadataConfigurationRequest.cs - NO ISSUES
• GetBucketMetadataConfigurationResponse.cs - NO ISSUES
• GetBucketMetadataConfigurationResult.cs - NO ISSUES
• InventoryTableConfigurationResult.cs - NO ISSUES
• JournalTableConfigurationResult.cs - NO ISSUES
• MetadataConfigurationResult.cs - NO ISSUES

Marshaller/Unmarshaller Files Compared (13):

• DestinationResultUnmarshaller.cs - NO ISSUES
• GetBucketMetadataConfigurationRequestMarshaller.cs - BREAKING CHANGE 1
• GetBucketMetadataConfigurationResponseUnmarshaller.cs - BREAKING CHANGE 2
• GetBucketMetadataConfigurationResultUnmarshaller.cs - NO ISSUES
• InventoryTableConfigurationResultUnmarshaller.cs - BREAKING CHANGE 3
• JournalTableConfigurationResultUnmarshaller.cs - BREAKING CHANGE 4
• MetadataConfigurationResultUnmarshaller.cs - NO ISSUES
• RecordExpirationUnmarshaller.cs - BREAKING CHANGE 5

Configuration Files Analyzed (2):

• generator/ServiceClientGeneratorLib/ServiceModel.cs - Added GetBucketMetadataConfiguration operation
• generator/ServiceModels/s3/s3.customizations.json - Added IsSet customizations for BucketName and ExpectedBucketOwner

Other Files (5):

• RecordExpiration model (referenced but not modified in this PR)
• ErrorDetails model (referenced but not modified in this PR)
• S3TablesBucketType enum (referenced but not modified in this PR)
• InventoryConfigurationState enum (referenced but not modified in this PR)
• Error vs ErrorDetails unmarshallers (behavior difference)

---

Recommendations

CRITICAL - Issue 1 MUST be fixed immediately: The GetBucketMetadataConfigurationRequestMarshaller is missing critical path resource handling. This will cause all API calls to fail.

HIGH PRIORITY - Issues 3, 4, 5: The unmarshaller changes need review to ensure backward compatibility. The Error property and Days property unmarshalling behavior has changed.

MEDIUM PRIORITY - Issue 2: The exception handling change should be reviewed to ensure no behavioral differences that client code depends on.

[dscpinheiro]

I don't remember seeing XmlStructureUnmarshallCustomization in the other PRs I reviewed. What does it do?

[peterrsongg]

It was in other PRs but probably missed because there weren't many structure unmarshallers. It is just a partial method defined for any customizations that apply to unmarshalling structures that use the rest-xml protocol. I have yet to use it though 😅