Skip to content

Commit 60fc9a5

Browse files
authored
Supported nested union fields (#689)
Issue #, if available: Description of changes: This PR adds support for nested union types to the code-generator. Prior to this change if a `union` shape contained a member that was itself a `union` the code-generator would produce non-compiling code in two ways: 1. When setting the values from the API response the generated code would not use a type switch for the nested union member and attempt directly access fields as if they were a `structure`. 2. When setting the SDK payload the generated code would attempt to dereference the union member. To address this this PR applies two fixes. 1. In `setResourceForUnion` check if the member is a union and force `Shape.Type` to `union` before recursing 2. In `setSDKForUnion` prevent dereference of member value if member is a union itself. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent a9e2cea commit 60fc9a5

6 files changed

Lines changed: 23163 additions & 1 deletion

File tree

pkg/generate/code/set_resource.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2473,6 +2473,9 @@ func setResourceForUnion(
24732473
if sourceMemberShapeRef == nil {
24742474
continue
24752475
}
2476+
if sourceMemberShapeRef.Shape.RealType == "union" {
2477+
sourceMemberShapeRef.Shape.Type = "union"
2478+
}
24762479

24772480
sourceMemberIndex, err := GetMemberIndex(sourceShape, targetMemberName)
24782481
if err != nil {
@@ -2561,6 +2564,9 @@ func setResourceForUnion(
25612564
true,
25622565
)
25632566
}
2567+
if sourceMemberShapeRef.Shape.RealType == "union" {
2568+
sourceMemberShapeRef.Shape.Type = "structure"
2569+
}
25642570
out += fmt.Sprintf("%s\t}\n", indent)
25652571
}
25662572
out += fmt.Sprintf("%s}\n", indent)

pkg/generate/code/set_resource_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5524,3 +5524,58 @@ func TestSetResource_QuickSight_DataSet_ReadOne(t *testing.T) {
55245524
// not &.Time (pointer type).
55255525
assert.Contains(got, "f6elemf0f0f0elem = f6elemf0f0f0iter.Time")
55265526
}
5527+
5528+
// TestSetResource_BedrockAgentCoreControl_GatewayTarget_NestedUnionTypeSwitch
5529+
// tests that when a union type (TargetConfiguration) contains a member whose
5530+
// Value is itself a union type (McpTargetConfiguration), the generated
5531+
// set-output code uses a type switch on the nested union interface instead of
5532+
// directly accessing struct fields.
5533+
//
5534+
// Before the fix, the generated code accessed f12f0.Value.ApiGateway,
5535+
// f12f0.Value.Lambda, etc. directly — but f12f0.Value is of type
5536+
// McpTargetConfiguration (an interface), which has no such fields. The fix
5537+
// ensures a proper `switch f12f0.Value.(type)` is generated.
5538+
func TestSetResource_BedrockAgentCoreControl_GatewayTarget_NestedUnionTypeSwitch(t *testing.T) {
5539+
assert := assert.New(t)
5540+
require := require.New(t)
5541+
5542+
g := testutil.NewModelForServiceWithOptions(t, "bedrock-agentcore-control", &testutil.TestingModelOptions{
5543+
GeneratorConfigFile: "generator-with-nested-union.yaml",
5544+
})
5545+
5546+
crd := testutil.GetCRDByName(t, g, "GatewayTarget")
5547+
require.NotNil(crd)
5548+
assert.NotNil(crd.Ops.ReadOne)
5549+
5550+
got, err := code.SetResource(crd.Config(), crd, model.OpTypeGet, "resp", "ko", 1)
5551+
require.NoError(err)
5552+
5553+
// --- Outer union: TargetConfiguration ---
5554+
// The outer union should use a type switch
5555+
assert.Contains(got, "switch resp.TargetConfiguration.(type) {")
5556+
assert.Contains(got, "case *svcsdktypes.TargetConfigurationMemberMcp:")
5557+
5558+
// --- Nested union: McpTargetConfiguration ---
5559+
// The nested union inside TargetConfigurationMemberMcp.Value should also
5560+
// use a type switch, NOT direct field access
5561+
assert.Contains(got, "switch f12f0.Value.(type) {")
5562+
assert.Contains(got, "case *svcsdktypes.McpTargetConfigurationMemberApiGateway:")
5563+
assert.Contains(got, "case *svcsdktypes.McpTargetConfigurationMemberLambda:")
5564+
assert.Contains(got, "case *svcsdktypes.McpTargetConfigurationMemberMcpServer:")
5565+
assert.Contains(got, "case *svcsdktypes.McpTargetConfigurationMemberOpenApiSchema:")
5566+
assert.Contains(got, "case *svcsdktypes.McpTargetConfigurationMemberSmithyModel:")
5567+
5568+
// Ensure we do NOT get direct field access on the union interface
5569+
// (this was the original compiler error)
5570+
assert.NotContains(got, "f12f0.Value.ApiGateway")
5571+
assert.NotContains(got, "f12f0.Value.Lambda")
5572+
assert.NotContains(got, "f12f0.Value.McpServer")
5573+
assert.NotContains(got, "f12f0.Value.OpenApiSchema")
5574+
assert.NotContains(got, "f12f0.Value.SmithyModel")
5575+
5576+
// Doubly-nested unions (e.g., ApiSchemaConfiguration inside OpenApiSchema
5577+
// member) should also use type switches
5578+
assert.Contains(got, "switch f12f0f0f3.Value.(type) {")
5579+
assert.Contains(got, "case *svcsdktypes.ApiSchemaConfigurationMemberInlinePayload:")
5580+
assert.Contains(got, "case *svcsdktypes.ApiSchemaConfigurationMemberS3:")
5581+
}

pkg/generate/code/set_sdk.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1937,7 +1937,11 @@ func setSDKForUnion(
19371937
return "", err
19381938
}
19391939
out += containerOut
1940-
if memberShape.Type == "list" {
1940+
// TODO (knottnt) refactor union Shape.Type handling
1941+
// Union type check is coupled to mutating Shape.Type in prior recursions.
1942+
// While this does work and test coverage validates the behavior this pattern of
1943+
// assign Shape.Type to union is confusing and makes function logic difficult to follow.
1944+
if memberShape.Type == "list" || memberShape.Type == "union" {
19411945
out += fmt.Sprintf("%s\t%s.Value = %s\n", indent, elemVarName, indexedVarName)
19421946
} else {
19431947
out += fmt.Sprintf("%s\t%s.Value = *%s\n", indent, elemVarName, indexedVarName)

pkg/generate/code/set_sdk_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6708,3 +6708,64 @@ func TestSetSDK_QuickSight_Analysis_Update(t *testing.T) {
67086708
assert.Contains(got, "= *f2f6elemf1f0f1iter")
67096709
assert.NotContains(got, "= f2f6elemf1f0f1iter\n")
67106710
}
6711+
6712+
// TestSetSDK_BedrockAgentCoreControl_GatewayTarget_NestedUnionNoDereference
6713+
// tests that when a union type (TargetConfiguration) contains a member whose
6714+
// Value is itself a union type (McpTargetConfiguration), the generated code:
6715+
// - Declares the nested union as a var (interface), not a struct pointer
6716+
// - Uses Member* wrapper types for each variant of the nested union
6717+
// - Does NOT dereference the nested union interface when assigning to
6718+
// the parent's .Value (interfaces are not pointers)
6719+
//
6720+
// This was a compiler error before the fix: "cannot indirect f5f0 (variable
6721+
// of interface type McpTargetConfiguration)"
6722+
func TestSetSDK_BedrockAgentCoreControl_GatewayTarget_NestedUnionNoDereference(t *testing.T) {
6723+
assert := assert.New(t)
6724+
require := require.New(t)
6725+
6726+
g := testutil.NewModelForServiceWithOptions(t, "bedrock-agentcore-control", &testutil.TestingModelOptions{
6727+
GeneratorConfigFile: "generator-with-nested-union.yaml",
6728+
})
6729+
6730+
crd := testutil.GetCRDByName(t, g, "GatewayTarget")
6731+
require.NotNil(crd)
6732+
assert.NotNil(crd.Ops.Create)
6733+
6734+
got, err := code.SetSDK(crd.Config(), crd, model.OpTypeCreate, "r.ko", "res", 1)
6735+
require.NoError(err)
6736+
6737+
// --- Outer union: TargetConfiguration ---
6738+
// The outer union variable should be declared as an interface type
6739+
assert.Contains(got, "var f7 svcsdktypes.TargetConfiguration")
6740+
assert.NotContains(got, "f7 := &svcsdktypes.TargetConfiguration{}")
6741+
6742+
// --- Nested union: McpTargetConfiguration ---
6743+
// The nested union inside TargetConfigurationMemberMcp should also be
6744+
// declared as an interface variable, not a struct pointer
6745+
assert.Contains(got, "var f7f0 svcsdktypes.McpTargetConfiguration")
6746+
assert.NotContains(got, "f7f0 := &svcsdktypes.McpTargetConfiguration{}")
6747+
6748+
// Each nested union member should use the Member* wrapper type
6749+
assert.Contains(got, "f7f0f0Parent := &svcsdktypes.McpTargetConfigurationMemberApiGateway{}")
6750+
assert.Contains(got, "f7f0f1Parent := &svcsdktypes.McpTargetConfigurationMemberLambda{}")
6751+
assert.Contains(got, "f7f0f2Parent := &svcsdktypes.McpTargetConfigurationMemberMcpServer{}")
6752+
assert.Contains(got, "f7f0f3Parent := &svcsdktypes.McpTargetConfigurationMemberOpenApiSchema{}")
6753+
assert.Contains(got, "f7f0f4Parent := &svcsdktypes.McpTargetConfigurationMemberSmithyModel{}")
6754+
6755+
// The nested union (McpTargetConfiguration) must NOT be dereferenced when
6756+
// assigned to the parent wrapper's .Value field. Interfaces cannot be
6757+
// dereferenced with *.
6758+
assert.Contains(got, "f7f0Parent.Value = f7f0\n")
6759+
assert.NotContains(got, "f7f0Parent.Value = *f7f0\n")
6760+
6761+
// Similarly, doubly-nested unions (ApiSchemaConfiguration inside
6762+
// McpTargetConfiguration) must not be dereferenced
6763+
assert.Contains(got, "f7f0f3Parent.Value = f7f0f3\n")
6764+
assert.NotContains(got, "f7f0f3Parent.Value = *f7f0f3\n")
6765+
assert.Contains(got, "f7f0f4Parent.Value = f7f0f4\n")
6766+
assert.NotContains(got, "f7f0f4Parent.Value = *f7f0f4\n")
6767+
6768+
// Non-union struct members inside the nested union SHOULD still be
6769+
// dereferenced (e.g., ApiGatewayTargetConfiguration is a regular struct)
6770+
assert.Contains(got, "f7f0f0Parent.Value = *f7f0f0\n")
6771+
}

0 commit comments

Comments
 (0)