Handle Unions with "type" members

[alextwoods]

Handle Unions with "type" members

Motivation and Context

When we generate Union shapes, we always add a special, SDK generated Type type member to the Pojo. The type member is a Union specific enum that is used as a type discriminator allowing customers (and internal code) to check which variant of the union is set and avoid unsafe casting. If the modeled type has a member named Type then this conflicts with our internally added Type field, causing the build to fail.

Given the following C2J Union definition:

"MyUnionShape":{
      "type":"structure",
      "members":{
        "NormalMember":{"shape":"String"},
        "Type":{"shape":"String"},
      },
      "union":true
    }

We will generate:

// compact class, un-related parts excluded
public final class MyUnion implements SdkPojo, Serializable,
                                                     ToCopyableBuilder<AllTypesUnionStructure.Builder, AllTypesUnionStructure> {

    private final String normalMember;

    private final String type; // generated from the "Type" member of the model
    private final Type type; // type descriminator field - auto-added by our codegen to every union pojo
    
    public final String normalMember() { return normalMember; }

    // getter method for member "type" - already detects conflict and uses "Value" suffix.
    public final String typeValue() { return type; }

    // normal getter for the codegen added "Type", always on every Union
    public final Type type() { return type; }
}

Modifications

We introduce a new getVariableName method that takes the name and the parentShape. The parentShape argument is required for us to limit the scope of the reserved words - specifically “type” should only be considered a reserved word for members in a union. This limits the blast radius of the change, ensuring that it is non-breaking for existing services.

The new getVariableName(String name, Shape parentShape) uses the existing isDisallowedNameForShape method, which does correctly consider “type” a reserved word for Union parentShapes.

Is this change breaking?

No - because we limit checking “type” as a reserved word to Unions only, no existing cases will change. Any existing model that has a union member named “type” does not currently build. Changing the variable name does NOT change the serialization or deserialization code - they will still use “type” - it is only the name of the member that is changed.

Is this change in-line with other SDKs/smithy recommendations?

Yes - Smithy based codegen uses the concept of Symbols with SymbolProviders and SymbolWriters (See: Decoupling Codegen with Symbols) which automatically support handling of “reserved words”. Smithy recommends limiting reserved word handling to the most minimal context:

Reserved words handling should be as granular as possible. If a symbol is only reserved in certain contexts, then that word should only be treated as reserved in that context.

Testing

• Added new test

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.

License

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

[sonarqubecloud]

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud