Skip to content

Commit cefbb77

Browse files
authored
codegen: make union collection traversal deterministic (#3904)
Sort object attributes before collecting union types in service and HTTP codegen so NameScope-assigned union identifiers do not oscillate across runs. Add regression tests that assert union naming remains stable when object field declaration order is reversed.
1 parent 8f7035a commit cefbb77

4 files changed

Lines changed: 215 additions & 3 deletions

File tree

codegen/service/service_data.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,7 +1073,7 @@ func collectUnionTypes(att *expr.AttributeExpr, scope *codegen.NameScope, loc *c
10731073
seen[dt.ID()] = struct{}{}
10741074
collectUnionTypes(dt.Attribute(), scope, codegen.UserTypeLocation(dt), unions, seen)
10751075
case *expr.Object:
1076-
for _, nat := range *dt {
1076+
for _, nat := range sortedNamedAttributes(*dt) {
10771077
collectUnionTypes(nat.Attribute, scope, loc, unions, seen)
10781078
}
10791079
case *expr.Array:
@@ -1152,7 +1152,7 @@ func collectViewUnionTypes(att *expr.AttributeExpr, scope *codegen.NameScope, lo
11521152
seen[dt.ID()] = struct{}{}
11531153
collectViewUnionTypes(dt.Attribute(), scope, loc, unions, seen)
11541154
case *expr.Object:
1155-
for _, nat := range *dt {
1155+
for _, nat := range sortedNamedAttributes(*dt) {
11561156
collectViewUnionTypes(nat.Attribute, scope, loc, unions, seen)
11571157
}
11581158
case *expr.Array:
@@ -1208,6 +1208,21 @@ func buildViewUnionTypeData(u *expr.Union, scope *codegen.NameScope, loc *codege
12081208
}
12091209
}
12101210

1211+
// sortedNamedAttributes returns object fields sorted by attribute name.
1212+
// Union naming uses NameScope uniqueness, so callers that discover unions while
1213+
// traversing objects must use a deterministic field order to avoid oscillating
1214+
// generated identifiers across runs.
1215+
func sortedNamedAttributes(attrs []*expr.NamedAttributeExpr) []*expr.NamedAttributeExpr {
1216+
if len(attrs) < 2 {
1217+
return attrs
1218+
}
1219+
sorted := slices.Clone(attrs)
1220+
sort.Slice(sorted, func(i, j int) bool {
1221+
return sorted[i].Name < sorted[j].Name
1222+
})
1223+
return sorted
1224+
}
1225+
12111226
// primitiveAliasGoType resolves the native Go type for a primitive alias branch.
12121227
// It uses expr.IsPrimitive to enforce the type contract and then unwraps aliases.
12131228
func primitiveAliasGoType(dt expr.DataType) (string, bool) {
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package service
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"goa.design/goa/v3/codegen"
9+
"goa.design/goa/v3/expr"
10+
)
11+
12+
func TestCollectUnionTypesDeterministicAcrossObjectOrder(t *testing.T) {
13+
sourceFromSignals := makeUnionForOrderTest("source",
14+
"physical_point",
15+
"synthetic_series",
16+
)
17+
sourceFromInputs := makeUnionForOrderTest("source",
18+
"time_series",
19+
"energy_rates",
20+
)
21+
22+
forward := &expr.AttributeExpr{
23+
Type: &expr.Object{
24+
{
25+
Name: "alpha",
26+
Attribute: &expr.AttributeExpr{
27+
Type: sourceFromSignals,
28+
},
29+
},
30+
{
31+
Name: "beta",
32+
Attribute: &expr.AttributeExpr{
33+
Type: sourceFromInputs,
34+
},
35+
},
36+
},
37+
}
38+
reverse := &expr.AttributeExpr{
39+
Type: &expr.Object{
40+
{
41+
Name: "beta",
42+
Attribute: &expr.AttributeExpr{
43+
Type: sourceFromInputs,
44+
},
45+
},
46+
{
47+
Name: "alpha",
48+
Attribute: &expr.AttributeExpr{
49+
Type: sourceFromSignals,
50+
},
51+
},
52+
},
53+
}
54+
55+
loc := &codegen.Location{
56+
RelImportPath: "gen/service",
57+
}
58+
forwardNames := collectServiceUnionTypeNames(forward, loc)
59+
reverseNames := collectServiceUnionTypeNames(reverse, loc)
60+
61+
require.Len(t, forwardNames, 2)
62+
require.Equal(t, forwardNames, reverseNames)
63+
}
64+
65+
func collectServiceUnionTypeNames(att *expr.AttributeExpr, loc *codegen.Location) map[string]string {
66+
scope := codegen.NewNameScope()
67+
seen := make(map[string]struct{})
68+
unionByHash := make(map[string]*UnionTypeData)
69+
collectUnionTypes(att, scope, loc, unionByHash, seen)
70+
71+
names := make(map[string]string, len(unionByHash))
72+
for hash, data := range unionByHash {
73+
names[hash] = data.Name
74+
}
75+
return names
76+
}
77+
78+
func makeUnionForOrderTest(typeName string, variants ...string) *expr.Union {
79+
values := make([]*expr.NamedAttributeExpr, len(variants))
80+
for i, variant := range variants {
81+
values[i] = &expr.NamedAttributeExpr{
82+
Name: variant,
83+
Attribute: &expr.AttributeExpr{
84+
Type: expr.String,
85+
},
86+
}
87+
}
88+
return &expr.Union{
89+
TypeName: typeName,
90+
Values: values,
91+
}
92+
}

http/codegen/service_data.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2722,7 +2722,7 @@ func collectHTTPUnionTypes(att *expr.AttributeExpr, scope *codegen.NameScope, un
27222722
seen[dt.ID()] = struct{}{}
27232723
collectHTTPUnionTypes(dt.Attribute(), scope, unions, seen)
27242724
case *expr.Object:
2725-
for _, nat := range *dt {
2725+
for _, nat := range sortedNamedAttributes(*dt) {
27262726
collectHTTPUnionTypes(nat.Attribute, scope, unions, seen)
27272727
}
27282728
case *expr.Array:
@@ -2769,6 +2769,21 @@ func buildHTTPUnionTypeData(u *expr.Union, scope *codegen.NameScope) *service.Un
27692769
}
27702770
}
27712771

2772+
// sortedNamedAttributes returns object fields sorted by attribute name.
2773+
// Union naming uses NameScope uniqueness, so callers that discover unions while
2774+
// traversing objects must use a deterministic field order to avoid oscillating
2775+
// generated identifiers across runs.
2776+
func sortedNamedAttributes(attrs []*expr.NamedAttributeExpr) []*expr.NamedAttributeExpr {
2777+
if len(attrs) < 2 {
2778+
return attrs
2779+
}
2780+
sorted := slices.Clone(attrs)
2781+
sort.Slice(sorted, func(i, j int) bool {
2782+
return sorted[i].Name < sorted[j].Name
2783+
})
2784+
return sorted
2785+
}
2786+
27722787
func (sds *ServicesData) attributeTypeData(ut expr.UserType, req, ptr, server bool, rd *ServiceData) *TypeData {
27732788
if ut == expr.Empty {
27742789
return nil
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package codegen
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
cg "goa.design/goa/v3/codegen"
9+
svc "goa.design/goa/v3/codegen/service"
10+
"goa.design/goa/v3/expr"
11+
)
12+
13+
func TestCollectHTTPUnionTypesDeterministicAcrossObjectOrder(t *testing.T) {
14+
sourceFromSignals := makeUnionForOrderTest("source",
15+
"physical_point",
16+
"synthetic_series",
17+
)
18+
sourceFromInputs := makeUnionForOrderTest("source",
19+
"time_series",
20+
"energy_rates",
21+
)
22+
23+
forward := &expr.AttributeExpr{
24+
Type: &expr.Object{
25+
{
26+
Name: "alpha",
27+
Attribute: &expr.AttributeExpr{
28+
Type: sourceFromSignals,
29+
},
30+
},
31+
{
32+
Name: "beta",
33+
Attribute: &expr.AttributeExpr{
34+
Type: sourceFromInputs,
35+
},
36+
},
37+
},
38+
}
39+
reverse := &expr.AttributeExpr{
40+
Type: &expr.Object{
41+
{
42+
Name: "beta",
43+
Attribute: &expr.AttributeExpr{
44+
Type: sourceFromInputs,
45+
},
46+
},
47+
{
48+
Name: "alpha",
49+
Attribute: &expr.AttributeExpr{
50+
Type: sourceFromSignals,
51+
},
52+
},
53+
},
54+
}
55+
56+
forwardNames := collectHTTPUnionTypeNames(forward)
57+
reverseNames := collectHTTPUnionTypeNames(reverse)
58+
59+
require.Len(t, forwardNames, 2)
60+
require.Equal(t, forwardNames, reverseNames)
61+
}
62+
63+
func collectHTTPUnionTypeNames(att *expr.AttributeExpr) map[string]string {
64+
scope := cg.NewNameScope()
65+
seen := make(map[string]struct{})
66+
unionByHash := make(map[string]*svc.UnionTypeData)
67+
collectHTTPUnionTypes(att, scope, unionByHash, seen)
68+
69+
names := make(map[string]string, len(unionByHash))
70+
for hash, data := range unionByHash {
71+
names[hash] = data.Name
72+
}
73+
return names
74+
}
75+
76+
func makeUnionForOrderTest(typeName string, variants ...string) *expr.Union {
77+
values := make([]*expr.NamedAttributeExpr, len(variants))
78+
for i, variant := range variants {
79+
values[i] = &expr.NamedAttributeExpr{
80+
Name: variant,
81+
Attribute: &expr.AttributeExpr{
82+
Type: expr.String,
83+
},
84+
}
85+
}
86+
return &expr.Union{
87+
TypeName: typeName,
88+
Values: values,
89+
}
90+
}

0 commit comments

Comments
 (0)