Skip to content

Commit

Permalink
Map unmapped attrs ErrorResult (goadesign#2857)
Browse files Browse the repository at this point in the history 
* Finalize body types before removing headers/params/cookies

* Map unmapped attrs only for ErrorResult

* Extend body type before removing headers/params/cookies
  • Loading branch information
@nitinmohan87
nitinmohan87 committed May 27, 2021
1 parent 43cde50 commit 2f3b482
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 1,003 deletions.
90 changes: 20 additions & 70 deletions expr/http_body_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func httpRequestBody(a *HTTPEndpointExpr) *AttributeExpr {

// 2. Remove header, param and cookies attributes
body := NewMappedAttributeExpr(payload)
extendBodyAttribute(body)
removeAttributes(body, headers)
removeAttributes(body, cookies)
removeAttributes(body, params)
Expand Down Expand Up @@ -209,6 +210,7 @@ func buildHTTPResponseBody(name string, attr *AttributeExpr, resp *HTTPResponseE
return &AttributeExpr{Type: Empty}
}
body := NewMappedAttributeExpr(attr)
extendBodyAttribute(body)

// 2. Remove header and cookie attributes
removeAttributes(body, resp.Headers)
Expand Down Expand Up @@ -386,85 +388,33 @@ func removeAttribute(attr *MappedAttributeExpr, name string) {
}
}

// extendedHTTPRequestBody returns an attribute describing the HTTP request body.
// This is used only in the validation phase to figure out the request body when
// method Payload extends or references other types.
func extendedHTTPRequestBody(a *HTTPEndpointExpr) *AttributeExpr {
const suffix = "RequestBody"
var (
name = concat(a.Name(), "Request", "Body")
)
if a.Body != nil {
a.Body = DupAtt(a.Body)
renameType(a.Body, name, suffix)
return a.Body
}

var (
payload = a.MethodExpr.Payload
headers = a.Headers
params = a.Params
bodyOnly = headers.IsEmpty() && params.IsEmpty() && a.MapQueryParams == nil
)

// 1. If Payload is not an object then check whether there are params or
// headers defined and if so return empty type (payload encoded in
// request params or headers) otherwise return payload type (payload
// encoded in request body).
if !IsObject(payload.Type) {
if bodyOnly {
payload = DupAtt(payload)
renameType(payload, name, suffix)
return payload
}
return &AttributeExpr{Type: Empty}
// extendedBodyAttribute returns an attribute describing the HTTP
// request/response body type by merging any Bases and References to the parent
// attribute. This must be invoked during validation or to determine the actual
// body type by removing any headers/params/cookies.
func extendBodyAttribute(body *MappedAttributeExpr) {
att := body.AttributeExpr
if isEmpty(att) {
return
}

// Merge extended and referenced types
payload = DupAtt(payload)
for _, ref := range payload.References {
for _, ref := range att.References {
ru, ok := ref.(UserType)
if !ok {
continue
}
payload.Inherit(ru.Attribute())
att.Inherit(ru.Attribute())
}
for _, base := range payload.Bases {
// unset references so that they don't get added back to the body type during
// finalize
att.References = nil
for _, base := range att.Bases {
ru, ok := base.(UserType)
if !ok {
continue
}
payload.Merge(ru.Attribute())
}

// 2. Remove header and param attributes
body := NewMappedAttributeExpr(payload)
removeAttributes(body, headers)
removeAttributes(body, params)
if a.MapQueryParams != nil && *a.MapQueryParams != "" {
removeAttribute(body, *a.MapQueryParams)
}
for att := range defaultRequestHeaderAttributes(a) {
removeAttribute(body, att)
}

// 3. Return empty type if no attribute left
if len(*AsObject(body.Type)) == 0 {
return &AttributeExpr{Type: Empty}
}

// 4. Build computed user type
att := body.Attribute()
ut := &UserTypeExpr{
AttributeExpr: att,
TypeName: name,
UID: a.Service.Name() + "#" + a.Name(),
}
appendSuffix(ut.Attribute().Type, suffix)

return &AttributeExpr{
Type: ut,
Validation: att.Validation,
UserExamples: att.UserExamples,
att.Merge(ru.Attribute())
}
// unset bases so that they don't get added back to the body type during
// finalize
att.Bases = nil
}
21 changes: 6 additions & 15 deletions expr/http_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ func (e *HTTPEndpointExpr) Validate() error {
}
}

body := extendedHTTPRequestBody(e)
body := httpRequestBody(e)
if e.SkipRequestBodyEncodeDecode && body.Type != Empty {
verr.Add(e, "HTTP endpoint request body must be empty when using SkipRequestBodyEncodeDecode but not all method payload attributes are mapped to headers and params. Make sure to define Headers and Params as needed.")
}
Expand Down Expand Up @@ -674,28 +674,19 @@ func (e *HTTPEndpointExpr) Finalize() {
initAttr(e.Headers, e.MethodExpr.Payload)
initAttr(e.Cookies, e.MethodExpr.Payload)

if e.Body != nil {
// rename type to add RequestBody suffix so that we don't end with
// duplicate type definitions - https://github.com/goadesign/goa/issues/1969
e.Body = httpRequestBody(e)
e.Body.Finalize()
} else {
// Compute body from the payload expression
e.Body = httpRequestBody(e)
// Don't call e.Body.Finalize() after computing the body because the
// payload expression might define bases and references which will be
// added to the body even when design explicitly maps them to headers or
// params.
}
e.Body = httpRequestBody(e)
e.Body.Finalize()

e.StreamingBody = httpStreamingBody(e)
if e.StreamingBody != nil {
e.StreamingBody.Finalize()
}

// Initialize responses parent, headers and body
for _, r := range e.Responses {
r.Finalize(e, e.MethodExpr.Result)
r.Body = httpResponseBody(e, r)
r.Body.Finalize()
r.mapUnmappedAttrs(e.MethodExpr.Result)
}

// Make sure all error types are user types and have a body.
Expand Down
2 changes: 2 additions & 0 deletions expr/http_error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ func (e *HTTPErrorExpr) Finalize(a *HTTPEndpointExpr) {
e.Response.Finalize(a, e.AttributeExpr)
if e.Response.Body == nil {
e.Response.Body = httpErrorResponseBody(a, e)
e.Response.Body.Finalize()
}
// map any unmapped attributes in ErrorResult type to response headers
e.Response.mapUnmappedAttrs(e.AttributeExpr)

// Initialize response content type if result is media type.
Expand Down
96 changes: 44 additions & 52 deletions expr/http_response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (r *HTTPResponseExpr) Validate(e *HTTPEndpointExpr) *eval.ValidationErrors

if !r.Headers.IsEmpty() {
verr.Merge(r.Headers.Validate("HTTP response headers", r))
if e.MethodExpr.Result.Type == Empty {
if isEmpty(e.MethodExpr.Result) {
verr.Add(r, "response defines headers but result is empty")
} else if IsObject(e.MethodExpr.Result.Type) {
mobj := AsObject(r.Headers.Type)
Expand All @@ -206,7 +206,7 @@ func (r *HTTPResponseExpr) Validate(e *HTTPEndpointExpr) *eval.ValidationErrors
}
if !r.Cookies.IsEmpty() {
verr.Merge(r.Cookies.Validate("HTTP response cookies", r))
if e.MethodExpr.Result.Type == Empty {
if isEmpty(e.MethodExpr.Result) {
verr.Add(r, "response defines cookies but result is empty")
} else if IsObject(e.MethodExpr.Result.Type) {
mobj := AsObject(r.Cookies.Type)
Expand Down Expand Up @@ -255,47 +255,45 @@ func (r *HTTPResponseExpr) Validate(e *HTTPEndpointExpr) *eval.ValidationErrors
func (r *HTTPResponseExpr) Finalize(a *HTTPEndpointExpr, svcAtt *AttributeExpr) {
r.Parent = a

if r.Body != nil {
if r.Body.Type != Empty {
bodyAtt := svcAtt
if o, ok := r.Body.Meta["origin:attribute"]; ok {
bodyAtt = svcAtt.Find(o[0])
}
bodyObj := AsObject(bodyAtt.Type)
if body := AsObject(r.Body.Type); body != nil {
for _, nat := range *body {
n := nat.Name
n = strings.Split(n, ":")[0]
var att, patt *AttributeExpr
var required bool
if bodyObj != nil {
att = bodyObj.Attribute(n)
required = bodyAtt.IsRequired(n)
} else {
att = bodyAtt
required = bodyAtt.Type != Empty
}
initAttrFromDesign(att, patt)
if required {
if r.Body.Validation == nil {
r.Body.Validation = &ValidationExpr{}
}
r.Body.Validation.AddRequired(n)
}
}
// Remember original name for example to generate friendlier OpenAPI specs.
if t, ok := r.Body.Type.(UserType); ok {
t.Attribute().AddMeta("name:original", t.Name())
if r.Body != nil && r.Body.Type != Empty {
bodyAtt := svcAtt
if o, ok := r.Body.Meta["origin:attribute"]; ok {
bodyAtt = svcAtt.Find(o[0])
}
bodyObj := AsObject(bodyAtt.Type)
if body := AsObject(r.Body.Type); body != nil {
for _, nat := range *body {
n := nat.Name
n = strings.Split(n, ":")[0]
var att, patt *AttributeExpr
var required bool
if bodyObj != nil {
att = bodyObj.Attribute(n)
required = bodyAtt.IsRequired(n)
} else {
att = bodyAtt
required = bodyAtt.Type != Empty
}
// Wrap object with user type to simplify response rendering code.
r.Body.Type = &UserTypeExpr{
AttributeExpr: DupAtt(r.Body),
TypeName: fmt.Sprintf("%s%sResponseBody", a.Service.Name(), a.Name()),
initAttrFromDesign(att, patt)
if required {
if r.Body.Validation == nil {
r.Body.Validation = &ValidationExpr{}
}
r.Body.Validation.AddRequired(n)
}
}
if r.Body.Meta == nil {
r.Body.Meta = bodyAtt.Meta
// Remember original name for example to generate friendlier OpenAPI specs.
if t, ok := r.Body.Type.(UserType); ok {
t.Attribute().AddMeta("name:original", t.Name())
}
// Wrap object with user type to simplify response rendering code.
r.Body.Type = &UserTypeExpr{
AttributeExpr: DupAtt(r.Body),
TypeName: fmt.Sprintf("%s%sResponseBody", a.Service.Name(), a.Name()),
}
}
if r.Body.Meta == nil {
r.Body.Meta = bodyAtt.Meta
}
}

Expand Down Expand Up @@ -331,13 +329,13 @@ func (r *HTTPResponseExpr) Dup() *HTTPResponseExpr {
return &res
}

// mapUnmappedAttrs maps any unmapped attributes to the response headers.
// Unmapped attributes refer to the attributes in the given service type
// (Result or Error) that are not mapped to response body, headers, cookies, or
// tags. Such unmapped attributes are mapped to special goa headers in the form
// of "Goa-Attribute(-<Attribute Name>)".
// mapUnmappedAttrs maps any unmapped attributes in ErrorResult type to the
// response headers. Unmapped attributes refer to the attributes in ErrorResult
// type that are not mapped to response body or headers. Such unmapped
// attributes are mapped to special goa headers in the form of
// "Goa-Attribute(-<Attribute Name>)".
func (r *HTTPResponseExpr) mapUnmappedAttrs(svcAtt *AttributeExpr) {
if svcAtt.Type == Empty {
if svcAtt.Type != ErrorResult {
return
}

Expand Down Expand Up @@ -365,12 +363,6 @@ func (r *HTTPResponseExpr) mapUnmappedAttrs(svcAtt *AttributeExpr) {
if _, ok := r.Headers.FindKey(nat.Name); ok {
continue
}
if _, ok := r.Cookies.FindKey(nat.Name); ok {
continue
}
if r.Tag[0] == nat.Name {
continue
}
r.Headers.Type.(*Object).Set(nat.Name, nat.Attribute)
r.Headers.Map("goa-attribute-"+nat.Name, nat.Name)
if svcAtt.IsRequired(nat.Name) {
Expand All @@ -382,7 +374,7 @@ func (r *HTTPResponseExpr) mapUnmappedAttrs(svcAtt *AttributeExpr) {
}
}
default:
if r.Headers.IsEmpty() && r.Cookies.IsEmpty() && (r.Body == nil || r.Body.Type == Empty) {
if r.Headers.IsEmpty() && (r.Body == nil || r.Body.Type == Empty) {
r.Headers.Type.(*Object).Set("goa-attribute", svcAtt)
}
}
Expand Down
6 changes: 2 additions & 4 deletions http/codegen/client_body_types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,12 @@ const ExplicitBodyPrimitiveResultMultipleViewsInitCode = `// NewMethodExplicitBo
// builds a "ServiceExplicitBodyPrimitiveResultMultipleView" service
// "MethodExplicitBodyPrimitiveResultMultipleView" endpoint result from a HTTP
// "OK" response.
func NewMethodExplicitBodyPrimitiveResultMultipleViewResulttypemultipleviewsOK(body string, c *string, b *string) *serviceexplicitbodyprimitiveresultmultipleviewviews.ResulttypemultipleviewsView {
func NewMethodExplicitBodyPrimitiveResultMultipleViewResulttypemultipleviewsOK(body string, c *string) *serviceexplicitbodyprimitiveresultmultipleviewviews.ResulttypemultipleviewsView {
v := body
res := &serviceexplicitbodyprimitiveresultmultipleviewviews.ResulttypemultipleviewsView{
A: &v,
}
res.C = c
res.B = b
return res
}
Expand All @@ -238,7 +237,7 @@ const ExplicitBodyUserResultMultipleViewsInitCode = `// NewMethodExplicitBodyUse
// a "ServiceExplicitBodyUserResultMultipleView" service
// "MethodExplicitBodyUserResultMultipleView" endpoint result from a HTTP "OK"
// response.
func NewMethodExplicitBodyUserResultMultipleViewResulttypemultipleviewsOK(body *MethodExplicitBodyUserResultMultipleViewResponseBody, c *string, b *string) *serviceexplicitbodyuserresultmultipleviewviews.ResulttypemultipleviewsView {
func NewMethodExplicitBodyUserResultMultipleViewResulttypemultipleviewsOK(body *MethodExplicitBodyUserResultMultipleViewResponseBody, c *string) *serviceexplicitbodyuserresultmultipleviewviews.ResulttypemultipleviewsView {
v := &serviceexplicitbodyuserresultmultipleviewviews.UserTypeView{
X: body.X,
Y: body.Y,
Expand All @@ -247,7 +246,6 @@ func NewMethodExplicitBodyUserResultMultipleViewResulttypemultipleviewsOK(body *
A: v,
}
res.C = c
res.B = b
return res
}
Expand Down
8 changes: 0 additions & 8 deletions http/codegen/client_decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ func TestClientDecode(t *testing.T) {
{"with-headers-dsl-viewed-result", testdata.WithHeadersBlockViewedResultDSL, testdata.WithHeadersBlockViewedResultResponseDecodeCode},
{"validate-error-response-type", testdata.ValidateErrorResponseTypeDSL, testdata.ValidateErrorResponseTypeDecodeCode},
{"empty-error-response-body", testdata.EmptyErrorResponseBodyDSL, testdata.EmptyErrorResponseBodyDecodeCode},

{"unmapped-result-object", testdata.UnmappedResultObjectDSL, testdata.UnmappedResultObjectDecodeCode},
{"unmapped-result-object-validate", testdata.UnmappedResultObjectValidateDSL, testdata.UnmappedResultObjectValidateDecodeCode},
{"unmapped-result-object-with-headers", testdata.UnmappedResultObjectWithHeadersDSL, testdata.UnmappedResultObjectWithHeadersDecodeCode},
{"unmapped-result-primitive", testdata.UnmappedResultPrimitiveDSL, testdata.UnmappedResultPrimitiveDecodeCode},
{"unmapped-result-primitive-validate", testdata.UnmappedResultPrimitiveValidateDSL, testdata.UnmappedResultPrimitiveValidateDecodeCode},
{"unmapped-result-array", testdata.UnmappedResultArrayDSL, testdata.UnmappedResultArrayDecodeCode},
{"unmapped-result-array-validate", testdata.UnmappedResultArrayValidateDSL, testdata.UnmappedResultArrayValidateDecodeCode},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down
9 changes: 0 additions & 9 deletions http/codegen/server_encode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,6 @@ func TestEncode(t *testing.T) {

{"empty-server-response", testdata.EmptyServerResponseDSL, testdata.EmptyServerResponseEncodeCode},
{"empty-server-response-with-tags", testdata.EmptyServerResponseWithTagsDSL, testdata.EmptyServerResponseWithTagsEncodeCode},

{"unmapped-result-object", testdata.UnmappedResultObjectDSL, testdata.UnmappedResultObjectEncodeCode},
{"unmapped-result-object-with-body", testdata.UnmappedResultObjectWithBodyDSL, testdata.UnmappedResultObjectWithBodyEncodeCode},
{"unmapped-result-object-validate", testdata.UnmappedResultObjectValidateDSL, testdata.UnmappedResultObjectValidateEncodeCode},
{"unmapped-result-object-with-headers", testdata.UnmappedResultObjectWithHeadersDSL, testdata.UnmappedResultObjectWithHeadersEncodeCode},
{"unmapped-result-primitive", testdata.UnmappedResultPrimitiveDSL, testdata.UnmappedResultPrimitiveEncodeCode},
{"unmapped-result-primitive-validate", testdata.UnmappedResultPrimitiveValidateDSL, testdata.UnmappedResultPrimitiveValidateEncodeCode},
{"unmapped-result-array", testdata.UnmappedResultArrayDSL, testdata.UnmappedResultArrayEncodeCode},
{"unmapped-result-array-validate", testdata.UnmappedResultArrayValidateDSL, testdata.UnmappedResultArrayValidateEncodeCode},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions http/codegen/server_error_encoder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func TestEncodeError(t *testing.T) {
{"api-no-body-error-response", testdata.APINoBodyErrorResponseDSL, testdata.NoBodyErrorResponseEncoderCode},
{"api-no-body-error-response-with-content-type", testdata.APINoBodyErrorResponseWithContentTypeDSL, testdata.NoBodyErrorResponseWithContentTypeEncoderCode},
{"empty-error-response-body", testdata.EmptyErrorResponseBodyDSL, testdata.EmptyErrorResponseBodyEncoderCode},
{"empty-custom-error-response-body", testdata.EmptyCustomErrorResponseBodyDSL, testdata.EmptyCustomErrorResponseBodyEncoderCode},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down
Loading

0 comments on commit 2f3b482

Please sign in to comment.