Skip to content

Commit 3bc54e5

Browse files
authored
codegen: fix HTTP oneof ResultType projections (#3917)
Project client response bodies to the effective view before collecting unions, emitting body types, and generating decode transforms so HTTP codegen uses one transport shape consistently. Align union validation with value-shaped request bodies and cover the regressions in HTTP and validation tests.
1 parent 430e0d8 commit 3bc54e5

5 files changed

Lines changed: 315 additions & 22 deletions

File tree

codegen/validation.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,11 @@ func recurseValidationCode(att *expr.AttributeExpr, put expr.UserType, attCtx *A
192192
for _, v := range u.Values {
193193
// Sum-type unions (struct-based, with Kind/AsX accessors) store each
194194
// branch as either a value (primitives, arrays, maps) or a pointer
195-
// (object user types). The union validation template binds the branch
196-
// value to a local named "actual"; the pointer semantics must match
197-
// that local, not the enclosing field context.
195+
// (object user types). Request-body validation may already use value
196+
// semantics for nested objects, so preserve the enclosing context and
197+
// only keep pointer semantics when both layers use pointers.
198198
unionCtx := attCtx.Dup()
199-
unionCtx.Pointer = expr.IsObject(v.Attribute.Type)
199+
unionCtx.Pointer = unionCtx.Pointer && expr.IsObject(v.Attribute.Type)
200200
val := validateAttribute(unionCtx, v.Attribute, put, "actual", context+".value", true, view, seen)
201201
if val == "" {
202202
continue
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// These tests regress union validation code generation when sum-type object
2+
// branches are validated in value contexts such as HTTP request bodies.
3+
package codegen
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
dsl "goa.design/goa/v3/dsl"
11+
"goa.design/goa/v3/expr"
12+
)
13+
14+
func TestUnionValidationPreservesValueContextForRequiredOnlyObjectBranches(t *testing.T) {
15+
root := RunDSL(t, requiredObjectUnionDSL)
16+
scope := NewNameScope()
17+
unionT := root.UserType("UnionUserValidate")
18+
att := &expr.AttributeExpr{Type: unionT}
19+
20+
valueCode := ValidationCode(att, nil, NewAttributeContext(false, false, false, "", scope), true, false, false, "target")
21+
require.NotContains(t, valueCode, "ValidateSomeType(actual)")
22+
require.NotContains(t, valueCode, "ValidateSomeOtherType(actual)")
23+
require.NotContains(t, valueCode, "if actual != nil")
24+
25+
pointerCode := ValidationCode(att, nil, NewAttributeContext(true, false, false, "", scope), true, false, false, "target")
26+
require.Contains(t, pointerCode, "ValidateSomeType(actual)")
27+
require.Contains(t, pointerCode, "ValidateSomeOtherType(actual)")
28+
}
29+
30+
// requiredObjectUnionDSL defines a OneOf with required-only object branches so
31+
// validation generation can distinguish pointer and value contexts.
32+
func requiredObjectUnionDSL() {
33+
var someType = dsl.Type("SomeType", func() {
34+
dsl.Attribute("a", dsl.String)
35+
dsl.Required("a")
36+
})
37+
var someOtherType = dsl.Type("SomeOtherType", func() {
38+
dsl.Attribute("b", dsl.String)
39+
dsl.Required("b")
40+
})
41+
42+
_ = dsl.Type("UnionUserValidate", func() {
43+
dsl.OneOf("values", func() {
44+
dsl.Attribute("SomeType", someType)
45+
dsl.Attribute("SomeOtherType", someOtherType)
46+
})
47+
})
48+
}
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
// These tests regress HTTP code generation around OneOf request bodies and
2+
// single-view ResultType responses. Client validation, union collection, and
3+
// decode/init must all derive from the same effective transport body shape.
4+
package codegen
5+
6+
import (
7+
"bytes"
8+
"testing"
9+
10+
"github.com/stretchr/testify/require"
11+
12+
"goa.design/goa/v3/codegen"
13+
. "goa.design/goa/v3/dsl"
14+
"goa.design/goa/v3/expr"
15+
"goa.design/goa/v3/http/codegen/testdata"
16+
)
17+
18+
func TestClientCLIInlinesOneOfRequestValidation(t *testing.T) {
19+
code := renderClientCLISectionCode(t, testdata.PayloadBodyUnionUserValidateDSL, 1, 1)
20+
21+
require.Contains(t, code, "BuildMethodBodyUnionUserValidatePayload")
22+
require.Contains(t, code, "if body.A == nil")
23+
require.Contains(t, code, "marshalUnionUserValidateRequestBodyTo")
24+
require.NotContains(t, code, "ValidateMethodBodyUnionUserValidateRequestBody")
25+
}
26+
27+
func TestClientResponseCodeProjectsSingleViewOneOfResults(t *testing.T) {
28+
cases := []struct {
29+
name string
30+
dsl func()
31+
}{
32+
{
33+
name: "single-result",
34+
dsl: oneOfResultSingleViewDSL,
35+
},
36+
{
37+
name: "collection-result",
38+
dsl: oneOfResultCollectionSingleViewDSL,
39+
},
40+
}
41+
42+
for _, c := range cases {
43+
t.Run(c.name, func(t *testing.T) {
44+
typeCode := renderClientTypesCode(t, c.dsl)
45+
decodeCode := renderClientDecodeCode(t, c.dsl)
46+
47+
require.Contains(t, typeCode, "AnimalView")
48+
require.NotContains(t, typeCode, "CatDetailsResponseBody")
49+
require.NotContains(t, typeCode, "DogDetailsResponseBody")
50+
require.NotContains(t, typeCode, "BirdDetailsResponseBody")
51+
require.NotContains(t, typeCode, "FishDetailsResponseBody")
52+
require.NotContains(t, typeCode, "json:\"details,omitempty\"")
53+
54+
require.NotContains(t, decodeCode, "body.Details")
55+
require.NotContains(t, decodeCode, "CatDetailsResponseBody")
56+
require.NotContains(t, decodeCode, "DogDetailsResponseBody")
57+
require.NotContains(t, decodeCode, "BirdDetailsResponseBody")
58+
require.NotContains(t, decodeCode, "FishDetailsResponseBody")
59+
})
60+
}
61+
}
62+
63+
// renderClientCLISectionCode renders the requested client CLI section for a
64+
// DSL and returns the generated code.
65+
func renderClientCLISectionCode(t *testing.T, dsl func(), fileIndex, sectionIndex int) string {
66+
t.Helper()
67+
68+
root := RunHTTPDSL(t, dsl)
69+
services := CreateHTTPServices(root)
70+
fs := ClientCLIFiles("", services)
71+
72+
return codegen.SectionCode(t, fs[fileIndex].SectionTemplates[sectionIndex])
73+
}
74+
75+
// renderClientTypesCode renders the client type file for a single-service DSL.
76+
func renderClientTypesCode(t *testing.T, dsl func()) string {
77+
t.Helper()
78+
79+
const genpkg = "gen"
80+
81+
root := RunHTTPDSL(t, dsl)
82+
services := CreateHTTPServices(root)
83+
fs := clientType(genpkg, root.API.HTTP.Services[0], make(map[string]struct{}), services)
84+
85+
var buf bytes.Buffer
86+
for _, s := range fs.SectionTemplates[1:] {
87+
require.NoError(t, s.Write(&buf))
88+
}
89+
90+
return codegen.FormatTestCode(t, "package foo\n"+buf.String())
91+
}
92+
93+
// renderClientDecodeCode renders the client decode section for a single-service
94+
// DSL and returns the generated code.
95+
func renderClientDecodeCode(t *testing.T, dsl func()) string {
96+
t.Helper()
97+
98+
root := RunHTTPDSL(t, dsl)
99+
services := CreateHTTPServices(root)
100+
fs := ClientFiles("", services)
101+
require.Len(t, fs, 2)
102+
103+
sections := fs[1].SectionTemplates
104+
require.Greater(t, len(sections), 2)
105+
106+
return codegen.SectionCode(t, sections[2])
107+
}
108+
109+
// oneOfResultSingleViewDSL defines a ResultType whose only view drops the OneOf
110+
// field. Client response code must therefore treat the transport body as the
111+
// projected view, not the raw ResultType.
112+
func oneOfResultSingleViewDSL() {
113+
animal := oneOfAnimalResultType("application/vnd.oneof-http-single-view.animal")
114+
115+
Service("ServiceOneOfSingleView", func() {
116+
Method("MethodShowAnimal", func() {
117+
Payload(func() {
118+
Attribute("id", String)
119+
Required("id")
120+
})
121+
Result(animal)
122+
HTTP(func() {
123+
GET("/animals/{id}")
124+
Response(StatusOK)
125+
})
126+
})
127+
})
128+
}
129+
130+
// oneOfResultCollectionSingleViewDSL defines a collection result whose
131+
// transport body is fixed to a single view. The generated client collection
132+
// code must project before collecting unions and body types.
133+
func oneOfResultCollectionSingleViewDSL() {
134+
animal := oneOfAnimalResultType("application/vnd.oneof-http-collection-view.animal")
135+
136+
Service("ServiceOneOfCollectionSingleView", func() {
137+
Method("MethodListAnimals", func() {
138+
Result(CollectionOf(animal), func() {
139+
View("default")
140+
})
141+
HTTP(func() {
142+
GET("/animals")
143+
Response(StatusOK)
144+
})
145+
})
146+
})
147+
}
148+
149+
// oneOfAnimalResultType returns a ResultType whose default view excludes the
150+
// OneOf details attribute. The omitted union is the regression target.
151+
func oneOfAnimalResultType(mediaType string) *expr.ResultTypeExpr {
152+
var catDetails = Type("CatDetails", func() {
153+
Attribute("favorite_spot", String)
154+
Attribute("lives_left", Int)
155+
Required("favorite_spot", "lives_left")
156+
})
157+
var dogDetails = Type("DogDetails", func() {
158+
Attribute("favorite_park", String)
159+
Attribute("plays_fetch", Boolean)
160+
Required("favorite_park", "plays_fetch")
161+
})
162+
var birdDetails = Type("BirdDetails", func() {
163+
Attribute("can_fly", Boolean)
164+
Attribute("vocabulary_size", Int)
165+
Required("can_fly", "vocabulary_size")
166+
})
167+
var fishDetails = Type("FishDetails", func() {
168+
Attribute("water_type", String)
169+
Required("water_type")
170+
})
171+
172+
return ResultType(mediaType, func() {
173+
TypeName("Animal")
174+
Attributes(func() {
175+
Attribute("name", String)
176+
OneOf("details", func() {
177+
Attribute("cat", catDetails)
178+
Attribute("dog", dogDetails)
179+
Attribute("bird", birdDetails)
180+
Attribute("fish", fishDetails)
181+
})
182+
Attribute("id", String)
183+
})
184+
View("default", func() {
185+
Attribute("name")
186+
Attribute("id")
187+
})
188+
Required("name", "id")
189+
})
190+
}

0 commit comments

Comments
 (0)