Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

separate methods to bind only: query params, route params, request body #1681

Merged
merged 1 commit into from
Dec 16, 2020
Merged

separate methods to bind only: query params, route params, request body #1681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 31 additions & 6 deletions bind.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,8 @@ type (
}
)

// Bind implements the `Binder#Bind` function.
func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
req := c.Request()

// BindPathParams binds path params to bindable object
func (b *DefaultBinder) BindPathParams(c Context, i interface{}) error {
names := c.ParamNames()
values := c.ParamValues()
params := map[string][]string{}
Expand All @@ -43,12 +41,28 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
if err := b.bindData(i, params, "param"); err != nil {
return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err)
}
if err = b.bindData(i, c.QueryParams(), "query"); err != nil {
return nil
}

// BindQueryParams binds query params to bindable object
func (b *DefaultBinder) BindQueryParams(c Context, i interface{}) error {
if err := b.bindData(i, c.QueryParams(), "query"); err != nil {
return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err)
}
return nil
}

// BindBody binds request body contents to bindable object
// NB: then binding forms take note that this implementation uses standard library form parsing
// which parses form data from BOTH URL and BODY if content type is not MIMEMultipartForm
// See non-MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseForm
// See MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseMultipartForm
func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) {
req := c.Request()
if req.ContentLength == 0 {
return
}

ctype := req.Header.Get(HeaderContentType)
switch {
case strings.HasPrefix(ctype, MIMEApplicationJSON):
Expand Down Expand Up @@ -80,7 +94,18 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
default:
return ErrUnsupportedMediaType
}
return
return nil
}

// Bind implements the `Binder#Bind` function.
func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
if err := b.BindPathParams(c, i); err != nil {
return err
}
if err = b.BindQueryParams(c, i); err != nil {
return err
}
return b.BindBody(c, i)
}

func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag string) error {
Expand Down
302 changes: 302 additions & 0 deletions bind_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,3 +553,305 @@ func testBindError(assert *assert.Assertions, r io.Reader, ctype string, expecte
}
}
}

func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
// tests to check binding behaviour when multiple sources path params, query params and request body are in use
// binding is done in steps and one source could overwrite previous source binded data
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed

type Node struct {
ID int `json:"id"`
Node string `json:"node"`
}

var testCases = []struct {
name string
givenURL string
givenContent io.Reader
givenMethod string
whenBindTarget interface{}
whenNoPathParams bool
expect interface{}
expectError string
}{
{
name: "ok, POST bind to struct with: path param + query param + empty body",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: "xxx"}, // in current implementation query params has higher priority than path params
},
{
name: "ok, POST bind to struct with: path param + empty body",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint",
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: "real_node"},
},
{
name: "ok, POST bind to struct with path + query + body = body has priority",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
},
{
name: "nok, POST body bind failure",
givenMethod: http.MethodPost,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`{`),
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
expectError: "code=400, message=unexpected EOF, internal=unexpected EOF",
},
{
name: "nok, GET body bind failure - trying to bind json array to struct",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
expectError: "code=400, message=Unmarshal type error: expected=echo.Node, got=array, field=, offset=1, internal=json: cannot unmarshal array into Go value of type echo.Node",
},
{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
name: "nok, GET query params bind failure - trying to bind json array to slice",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Node{},
expect: &[]Node{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
},
{ // binding path params interferes with body. b.BindBody() should be used to bind only body to slice
name: "nok, GET path params bind failure - trying to bind json array to slice",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint?node=xxx",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenBindTarget: &[]Node{},
expect: &[]Node{},
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
},
{
name: "ok, GET body bind json array to slice",
givenMethod: http.MethodGet,
givenURL: "/api/real_node/endpoint",
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Node{},
expect: &[]Node{{ID: 1, Node: ""}},
expectError: "",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
e := New()
// assume route we are testing is "/api/:node/endpoint?some_query_params=here"
req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent)
req.Header.Set(HeaderContentType, MIMEApplicationJSON)
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)

if !tc.whenNoPathParams {
c.SetParamNames("node")
c.SetParamValues("real_node")
}

var bindTarget interface{}
if tc.whenBindTarget != nil {
bindTarget = tc.whenBindTarget
} else {
bindTarget = &Node{}
}
b := new(DefaultBinder)

err := b.Bind(bindTarget, c)
if tc.expectError != "" {
assert.EqualError(t, err, tc.expectError)
} else {
assert.NoError(t, err)
}
assert.Equal(t, tc.expect, bindTarget)
})
}
}

func TestDefaultBinder_BindBody(t *testing.T) {
// tests to check binding behaviour when multiple sources path params, query params and request body are in use
// generally when binding from request body - URL and path params are ignored - unless form is being binded.
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed

type Node struct {
ID int `json:"id" xml:"id"`
Node string `json:"node" xml:"node"`
}
type Nodes struct {
Nodes []Node `xml:"node" form:"node"`
}

var testCases = []struct {
name string
givenURL string
givenContent io.Reader
givenMethod string
givenContentType string
whenNoPathParams bool
whenBindTarget interface{}
expect interface{}
expectError string
}{
{
name: "ok, JSON POST bind to struct with: path + query + empty field in body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body
},
{
name: "ok, JSON POST bind to struct with: path + query + body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
},
{
name: "ok, JSON POST body bind json array to slice (has matching path/query params)",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`[{"id": 1}]`),
whenNoPathParams: true,
whenBindTarget: &[]Node{},
expect: &[]Node{{ID: 1, Node: ""}},
expectError: "",
},
{ // rare case as GET is not usually used to send request body
name: "ok, JSON GET bind to struct with: path + query + empty field in body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodGet,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`{"id": 1}`),
expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body
},
{ // rare case as GET is not usually used to send request body
name: "ok, JSON GET bind to struct with: path + query + body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodGet,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
},
{
name: "nok, JSON POST body bind failure",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationJSON,
givenContent: strings.NewReader(`{`),
expect: &Node{ID: 0, Node: ""},
expectError: "code=400, message=unexpected EOF, internal=unexpected EOF",
},
{
name: "ok, XML POST bind to struct with: path + query + empty body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationXML,
givenContent: strings.NewReader(`<node><id>1</id><node>yyy</node></node>`),
expect: &Node{ID: 1, Node: "yyy"},
},
{
name: "ok, XML POST bind array to slice with: path + query + body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationXML,
givenContent: strings.NewReader(`<nodes><node><id>1</id><node>yyy</node></node></nodes>`),
whenBindTarget: &Nodes{},
expect: &Nodes{Nodes: []Node{{ID: 1, Node: "yyy"}}},
},
{
name: "nok, XML POST bind failure",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationXML,
givenContent: strings.NewReader(`<node><`),
expect: &Node{ID: 0, Node: ""},
expectError: "code=400, message=Syntax error: line=1, error=XML syntax error on line 1: unexpected EOF, internal=XML syntax error on line 1: unexpected EOF",
},
{
name: "ok, FORM POST bind to struct with: path + query + empty body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationForm,
givenContent: strings.NewReader(`id=1&node=yyy`),
expect: &Node{ID: 1, Node: "yyy"},
},
{
// NB: form values are taken from BOTH body and query for POST/PUT/PATCH by standard library implementation
// See: https://golang.org/pkg/net/http/#Request.ParseForm
name: "ok, FORM POST bind to struct with: path + query + empty field in body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMEApplicationForm,
givenContent: strings.NewReader(`id=1`),
expect: &Node{ID: 1, Node: "xxx"},
},
{
// NB: form values are taken from query by standard library implementation
// See: https://golang.org/pkg/net/http/#Request.ParseForm
name: "ok, FORM GET bind to struct with: path + query + empty field in body",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodGet,
givenContentType: MIMEApplicationForm,
givenContent: strings.NewReader(`id=1`),
expect: &Node{ID: 0, Node: "xxx"}, // 'xxx' is taken from URL and body is not used with GET by implementation
},
{
name: "nok, unsupported content type",
givenURL: "/api/real_node/endpoint?node=xxx",
givenMethod: http.MethodPost,
givenContentType: MIMETextPlain,
givenContent: strings.NewReader(`<html></html>`),
expect: &Node{ID: 0, Node: ""},
expectError: "code=415, message=Unsupported Media Type",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
e := New()
// assume route we are testing is "/api/:node/endpoint?some_query_params=here"
req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent)
switch tc.givenContentType {
case MIMEApplicationXML:
req.Header.Set(HeaderContentType, MIMEApplicationXML)
case MIMEApplicationForm:
req.Header.Set(HeaderContentType, MIMEApplicationForm)
case MIMEApplicationJSON:
req.Header.Set(HeaderContentType, MIMEApplicationJSON)
}
rec := httptest.NewRecorder()
c := e.NewContext(req, rec)

if !tc.whenNoPathParams {
c.SetParamNames("node")
c.SetParamValues("real_node")
}

var bindTarget interface{}
if tc.whenBindTarget != nil {
bindTarget = tc.whenBindTarget
} else {
bindTarget = &Node{}
}
b := new(DefaultBinder)

err := b.BindBody(c, bindTarget)
if tc.expectError != "" {
assert.EqualError(t, err, tc.expectError)
} else {
assert.NoError(t, err)
}
assert.Equal(t, tc.expect, bindTarget)
})
}
}