Skip to content

Commit

Permalink
Fix cookies (goadesign#2579)
Browse files Browse the repository at this point in the history
* Add test to HTTP cookie expressions

Fix related issues.

* Fix comment of Cookie DSL
  • Loading branch information
raphael committed Jun 7, 2020
1 parent e52a990 commit cb1d437
Show file tree
Hide file tree
Showing 8 changed files with 296 additions and 14 deletions.
13 changes: 6 additions & 7 deletions dsl/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,14 +423,13 @@ func Header(name string, args ...interface{}) {
// // attributes. When reading the cookie value client
// // side validate that's it is a GUID.
// Cookie("session:SID", String, func() {
// CookieMaxAge(3600) // Cookie attributes
// CookieDomain("goa.design")
// CookiePath("/session")
// CookieSecure()
// CookieHTTPOnly()
//
// Format(FormatGUID) // Cookie value validations
// })
// CookieMaxAge(3600) // Cookie attributes
// CookieDomain("goa.design")
// CookiePath("/session")
// CookieSecure()
// CookieHTTPOnly()
// })
// })
// })
Expand Down Expand Up @@ -589,7 +588,7 @@ func CookieSecure() {
// })
//
func CookieHTTPOnly() {
_, ok := eval.Current().(*expr.MappedAttributeExpr)
_, ok := eval.Current().(*expr.HTTPResponseExpr)
if !ok {
eval.IncompatibleDSL()
return
Expand Down
48 changes: 48 additions & 0 deletions expr/http_cookie_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package expr_test

import (
"fmt"
"testing"

"goa.design/goa/v3/expr"
"goa.design/goa/v3/expr/testdata"
)

func TestHTTPResponseCookie(t *testing.T) {
type Props map[string]interface{}

cases := []struct {
Name string
DSL func()
Props Props
}{
{"cookie", testdata.CookieObjectResultDSL, nil},
{"cookie", testdata.CookieStringResultDSL, nil},
{"max-age", testdata.CookieMaxAgeDSL, Props{"cookie:max-age": testdata.CookieMaxAgeValue}},
{"domain", testdata.CookieDomainDSL, Props{"cookie:domain": testdata.CookieDomainValue}},
{"path", testdata.CookiePathDSL, Props{"cookie:path": testdata.CookiePathValue}},
{"secure", testdata.CookieSecureDSL, Props{"cookie:secure": "Secure"}},
{"http-only", testdata.CookieHTTPOnlyDSL, Props{"cookie:http-only": "HttpOnly"}},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
root := expr.RunDSL(t, c.DSL)
e := root.API.HTTP.Services[len(root.API.HTTP.Services)-1].HTTPEndpoints[0]
cookies := e.Responses[0].Cookies.AttributeExpr
if len(*expr.AsObject(cookies.Type)) != 1 {
t.Errorf("got %d cookie(s), expected exactly one", len(*expr.AsObject(cookies.Type)))
} else {
m := cookies.Meta
for n, v := range c.Props {
if len(m) != 1 {
t.Errorf("got cookies metadata with length %d, expected 1", len(m))
} else if len(m[n]) != 1 {
t.Errorf("got cookies metadata %q with length %d, expected 1", n, len(m[n]))
} else if m[n][0] != fmt.Sprintf("%v", v) {
t.Errorf("got value %q for cookies metadata %q, expected %q", m[n][0], n, fmt.Sprintf("%v", v))
}
}
}
})
}
}
3 changes: 3 additions & 0 deletions expr/http_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ func (e *HTTPEndpointExpr) Prepare() {
c.Prepare()
if !e.HasAbsoluteRoutes() {
headers.Merge(c.Headers)
cookies.Merge(c.Cookies)
cpp := c.PathParams()
params.Merge(cpp)

Expand All @@ -234,9 +235,11 @@ func (e *HTTPEndpointExpr) Prepare() {
}
}
headers.Merge(e.Headers)
cookies.Merge(e.Cookies)
params.Merge(e.Params)

e.Headers = headers
e.Cookies = cookies
e.Params = params

// Initialize path params that are not defined explicitly in
Expand Down
14 changes: 14 additions & 0 deletions expr/http_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestHTTPEndpointPrepare(t *testing.T) {
DSL func()
Headers []string
Params []string
Cookies []string
Error string
}{
"valid": {
Expand All @@ -46,11 +47,13 @@ func TestHTTPEndpointPrepare(t *testing.T) {
DSL: testdata.EndpointWithParentDSL,
Headers: []string{"pheader", "header"},
Params: []string{"pparam", "param"},
Cookies: []string{"pcookie", "cookie"},
},
"with parent revert": {
DSL: testdata.EndpointWithParentRevertDSL,
Headers: []string{"pheader", "header"},
Params: []string{"pparam", "param"},
Cookies: []string{"pcookie", "cookie"},
},
"error": {
DSL: testdata.EndpointRecursiveParentDSL,
Expand All @@ -74,6 +77,17 @@ func TestHTTPEndpointPrepare(t *testing.T) {
}
}

ct := expr.AsObject(e.Cookies.AttributeExpr.Type)
if len(*ct) != len(c.Cookies) {
t.Errorf("got %d cookies, expected %d", len(*ct), len(c.Cookies))
} else {
for _, n := range c.Cookies {
if ct.Attribute(n) == nil {
t.Errorf("cookie %q is missing", n)
}
}
}

pt := expr.AsObject(e.Params.AttributeExpr.Type)
if len(*pt) != len(c.Params) {
t.Errorf("got %d params, expected %d", len(*pt), len(c.Params))
Expand Down
94 changes: 89 additions & 5 deletions expr/http_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,23 @@ func TestHTTPResponseValidation(t *testing.T) {
{"empty", emptyResultEmptyResponseDSL, ""},
{"non empty result", nonEmptyResultEmptyResponseDSL, ""},
{"non empty response", emptyResultNonEmptyResponseDSL, ""},
{"string result", stringResultResponseWithHeadersDSL, ""},
{"string result", stringResultResponseWithTextContentTypeDSL, ""},
{"object result", objectResultResponseWithHeadersDSL, ""},
{"array result", arrayResultResponseWithHeadersDSL, ""},
{"map result", mapResultResponseWithHeadersDSL, `service "MapResultResponseWithHeaders" HTTP endpoint "Method": attribute "foo" used in HTTP headers must be a primitive type or an array of primitive types.`},
{"header string result", stringResultResponseWithHeadersDSL, ""},
{"cookie string result", stringResultResponseWithCookiesDSL, ""},
{"string result text encoding", stringResultResponseWithTextContentTypeDSL, ""},
{"header object result", objectResultResponseWithHeadersDSL, ""},
{"cookie object result", objectResultResponseWithCookiesDSL, ""},
{"header array result", arrayResultResponseWithHeadersDSL, ""},
{"cookie array result", arrayResultResponseWithCookiesDSL, `service "ArrayResultResponseWithCookies" HTTP endpoint "Method": attribute "foo" used in HTTP cookies must be a primitive type.`},
{"header map result", mapResultResponseWithHeadersDSL, `service "MapResultResponseWithHeaders" HTTP endpoint "Method": attribute "foo" used in HTTP headers must be a primitive type or an array of primitive types.`},
{"cookie map result", mapResultResponseWithCookiesDSL, `service "MapResultResponseWithCookies" HTTP endpoint "Method": attribute "foo" used in HTTP cookies must be a primitive type.`},
{"invalid", emptyResultResponseWithHeadersDSL, `HTTP response of service "EmptyResultResponseWithHeaders" HTTP endpoint "Method": response defines headers but result is empty`},
{"implicit object in header", implicitObjectResultResponseWithHeadersDSL, `service "ArrayObjectResultResponseWithHeaders" HTTP endpoint "Method": attribute "foo" used in HTTP headers must be a primitive type or an array of primitive types.`},
{"array of object in header", arrayObjectResultResponseWithHeadersDSL, `service "ArrayObjectResultResponseWithHeaders" HTTP endpoint "Method": Array result is mapped to an HTTP header but is not an array of primitive types.`},
{"not string or []byte", intResultResponseWithTextContentTypeDSL, `HTTP response of service "StringResultResponseWithHeaders" HTTP endpoint "Method": Result type must be String or Bytes when ContentType is 'text/plain'`},
{"missing header result attribute", missingHeaderResultAttributeDSL, `HTTP response of service "MissingHeaderResultAttribute" HTTP endpoint "Method": header "bar" has no equivalent attribute in result type, use notation 'attribute_name:header_name' to identify corresponding result type attribute.
service "MissingHeaderResultAttribute" HTTP endpoint "Method": attribute "bar" used in HTTP headers must be a primitive type or an array of primitive types.`},
{"missing cookie result attribute", missingCookieResultAttributeDSL, `HTTP response of service "MissingCookieResultAttribute" HTTP endpoint "Method": cookie "bar" has no equivalent attribute in result type, use notation 'attribute_name:cookie_name' to identify corresponding result type attribute.
service "MissingCookieResultAttribute" HTTP endpoint "Method": attribute "bar" used in HTTP cookies must be a primitive type.`},
{"skip encode and gRPC", skipEncodeAndGRPCDSL, `service "SkipEncodeAndGRPC" HTTP endpoint "Method": Endpoint response cannot use SkipResponseBodyEncodeDecode and define a gRPC transport.`},
}
for _, c := range cases {
Expand Down Expand Up @@ -89,6 +95,20 @@ var stringResultResponseWithHeadersDSL = func() {
})
}

var stringResultResponseWithCookiesDSL = func() {
Service("StringResultResponseWithCookies", func() {
Method("Method", func() {
Result(String)
HTTP(func() {
POST("/")
Response(func() {
Cookie("Location")
})
})
})
})
}

var stringResultResponseWithTextContentTypeDSL = func() {
Service("StringResultResponseWithHeaders", func() {
Method("Method", func() {
Expand Down Expand Up @@ -119,6 +139,22 @@ var objectResultResponseWithHeadersDSL = func() {
})
}

var objectResultResponseWithCookiesDSL = func() {
Service("ObjectResultResponseWithCookies", func() {
Method("Method", func() {
Result(func() {
Attribute("foo", String)
})
HTTP(func() {
POST("/")
Response(func() {
Cookie("foo:Location")
})
})
})
})
}

var implicitObjectResultResponseWithHeadersDSL = func() {
Service("ArrayObjectResultResponseWithHeaders", func() {
Method("Method", func() {
Expand Down Expand Up @@ -171,6 +207,22 @@ var arrayResultResponseWithHeadersDSL = func() {
})
}

var arrayResultResponseWithCookiesDSL = func() {
Service("ArrayResultResponseWithCookies", func() {
Method("Method", func() {
Result(func() {
Attribute("foo", ArrayOf(String))
})
HTTP(func() {
POST("/")
Response(func() {
Cookie("foo:Location")
})
})
})
})
}

var mapResultResponseWithHeadersDSL = func() {
Service("MapResultResponseWithHeaders", func() {
Method("Method", func() {
Expand All @@ -187,6 +239,22 @@ var mapResultResponseWithHeadersDSL = func() {
})
}

var mapResultResponseWithCookiesDSL = func() {
Service("MapResultResponseWithCookies", func() {
Method("Method", func() {
Result(func() {
Attribute("foo", MapOf(String, String))
})
HTTP(func() {
POST("/")
Response(func() {
Cookie("foo:Location")
})
})
})
})
}

var emptyResultResponseWithHeadersDSL = func() {
Service("EmptyResultResponseWithHeaders", func() {
Method("Method", func() {
Expand Down Expand Up @@ -230,6 +298,22 @@ var missingHeaderResultAttributeDSL = func() {
})
}

var missingCookieResultAttributeDSL = func() {
Service("MissingCookieResultAttribute", func() {
Method("Method", func() {
Result(func() {
Attribute("foo")
})
HTTP(func() {
POST("/")
Response(func() {
Cookie("bar")
})
})
})
})
}

var skipEncodeAndGRPCDSL = func() {
Service("SkipEncodeAndGRPC", func() {
Method("Method", func() {
Expand Down
Loading

0 comments on commit cb1d437

Please sign in to comment.