Skip to content

Commit bd5810f

Browse files
aldasaldas
committed
separate methods to bind only query params, path params, request body
1 parent 7a90304 commit bd5810f

File tree

2 files changed

+333
-6
lines changed

2 files changed

+333
-6
lines changed

‎bind.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ type (
3030
}
3131
)
3232

33-
// Bind implements the `Binder#Bind` function.
34-
func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
35-
req := c.Request()
36-
33+
// BindPathParams binds path params to bindable object
34+
func (b *DefaultBinder) BindPathParams(c Context, i interface{}) error {
3735
names := c.ParamNames()
3836
values := c.ParamValues()
3937
params := map[string][]string{}
@@ -43,12 +41,28 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
4341
if err := b.bindData(i, params, "param"); err != nil {
4442
return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err)
4543
}
46-
if err = b.bindData(i, c.QueryParams(), "query"); err != nil {
44+
return nil
45+
}
46+
47+
// BindQueryParams binds query params to bindable object
48+
func (b *DefaultBinder) BindQueryParams(c Context, i interface{}) error {
49+
if err := b.bindData(i, c.QueryParams(), "query"); err != nil {
4750
return NewHTTPError(http.StatusBadRequest, err.Error()).SetInternal(err)
4851
}
52+
return nil
53+
}
54+
55+
// BindBody binds request body contents to bindable object
56+
// NB: then binding forms take note that this implementation uses standard library form parsing
57+
// which parses form data from BOTH URL and BODY if content type is not MIMEMultipartForm
58+
// See non-MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseForm
59+
// See MIMEMultipartForm: https://golang.org/pkg/net/http/#Request.ParseMultipartForm
60+
func (b *DefaultBinder) BindBody(c Context, i interface{}) (err error) {
61+
req := c.Request()
4962
if req.ContentLength == 0 {
5063
return
5164
}
65+
5266
ctype := req.Header.Get(HeaderContentType)
5367
switch {
5468
case strings.HasPrefix(ctype, MIMEApplicationJSON):
@@ -80,7 +94,18 @@ func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
8094
default:
8195
return ErrUnsupportedMediaType
8296
}
83-
return
97+
return nil
98+
}
99+
100+
// Bind implements the `Binder#Bind` function.
101+
func (b *DefaultBinder) Bind(i interface{}, c Context) (err error) {
102+
if err := b.BindPathParams(c, i); err != nil {
103+
return err
104+
}
105+
if err = b.BindQueryParams(c, i); err != nil {
106+
return err
107+
}
108+
return b.BindBody(c, i)
84109
}
85110

86111
func (b *DefaultBinder) bindData(ptr interface{}, data map[string][]string, tag string) error {

‎bind_test.go

Lines changed: 302 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,3 +553,305 @@ func testBindError(assert *assert.Assertions, r io.Reader, ctype string, expecte
553553
}
554554
}
555555
}
556+
557+
func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) {
558+
// tests to check binding behaviour when multiple sources path params, query params and request body are in use
559+
// binding is done in steps and one source could overwrite previous source binded data
560+
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
561+
562+
type Node struct {
563+
ID int `json:"id"`
564+
Node string `json:"node"`
565+
}
566+
567+
var testCases = []struct {
568+
name string
569+
givenURL string
570+
givenContent io.Reader
571+
givenMethod string
572+
whenBindTarget interface{}
573+
whenNoPathParams bool
574+
expect interface{}
575+
expectError string
576+
}{
577+
{
578+
name: "ok, POST bind to struct with: path param + query param + empty body",
579+
givenMethod: http.MethodPost,
580+
givenURL: "/api/real_node/endpoint?node=xxx",
581+
givenContent: strings.NewReader(`{"id": 1}`),
582+
expect: &Node{ID: 1, Node: "xxx"}, // in current implementation query params has higher priority than path params
583+
},
584+
{
585+
name: "ok, POST bind to struct with: path param + empty body",
586+
givenMethod: http.MethodPost,
587+
givenURL: "/api/real_node/endpoint",
588+
givenContent: strings.NewReader(`{"id": 1}`),
589+
expect: &Node{ID: 1, Node: "real_node"},
590+
},
591+
{
592+
name: "ok, POST bind to struct with path + query + body = body has priority",
593+
givenMethod: http.MethodPost,
594+
givenURL: "/api/real_node/endpoint?node=xxx",
595+
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
596+
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
597+
},
598+
{
599+
name: "nok, POST body bind failure",
600+
givenMethod: http.MethodPost,
601+
givenURL: "/api/real_node/endpoint?node=xxx",
602+
givenContent: strings.NewReader(`{`),
603+
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
604+
expectError: "code=400, message=unexpected EOF, internal=unexpected EOF",
605+
},
606+
{
607+
name: "nok, GET body bind failure - trying to bind json array to struct",
608+
givenMethod: http.MethodGet,
609+
givenURL: "/api/real_node/endpoint?node=xxx",
610+
givenContent: strings.NewReader(`[{"id": 1}]`),
611+
expect: &Node{ID: 0, Node: "xxx"}, // query binding has already modified bind target
612+
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",
613+
},
614+
{ // binding query params interferes with body. b.BindBody() should be used to bind only body to slice
615+
name: "nok, GET query params bind failure - trying to bind json array to slice",
616+
givenMethod: http.MethodGet,
617+
givenURL: "/api/real_node/endpoint?node=xxx",
618+
givenContent: strings.NewReader(`[{"id": 1}]`),
619+
whenNoPathParams: true,
620+
whenBindTarget: &[]Node{},
621+
expect: &[]Node{},
622+
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
623+
},
624+
{ // binding path params interferes with body. b.BindBody() should be used to bind only body to slice
625+
name: "nok, GET path params bind failure - trying to bind json array to slice",
626+
givenMethod: http.MethodGet,
627+
givenURL: "/api/real_node/endpoint?node=xxx",
628+
givenContent: strings.NewReader(`[{"id": 1}]`),
629+
whenBindTarget: &[]Node{},
630+
expect: &[]Node{},
631+
expectError: "code=400, message=binding element must be a struct, internal=binding element must be a struct",
632+
},
633+
{
634+
name: "ok, GET body bind json array to slice",
635+
givenMethod: http.MethodGet,
636+
givenURL: "/api/real_node/endpoint",
637+
givenContent: strings.NewReader(`[{"id": 1}]`),
638+
whenNoPathParams: true,
639+
whenBindTarget: &[]Node{},
640+
expect: &[]Node{{ID: 1, Node: ""}},
641+
expectError: "",
642+
},
643+
}
644+
645+
for _, tc := range testCases {
646+
t.Run(tc.name, func(t *testing.T) {
647+
e := New()
648+
// assume route we are testing is "/api/:node/endpoint?some_query_params=here"
649+
req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent)
650+
req.Header.Set(HeaderContentType, MIMEApplicationJSON)
651+
rec := httptest.NewRecorder()
652+
c := e.NewContext(req, rec)
653+
654+
if !tc.whenNoPathParams {
655+
c.SetParamNames("node")
656+
c.SetParamValues("real_node")
657+
}
658+
659+
var bindTarget interface{}
660+
if tc.whenBindTarget != nil {
661+
bindTarget = tc.whenBindTarget
662+
} else {
663+
bindTarget = &Node{}
664+
}
665+
b := new(DefaultBinder)
666+
667+
err := b.Bind(bindTarget, c)
668+
if tc.expectError != "" {
669+
assert.EqualError(t, err, tc.expectError)
670+
} else {
671+
assert.NoError(t, err)
672+
}
673+
assert.Equal(t, tc.expect, bindTarget)
674+
})
675+
}
676+
}
677+
678+
func TestDefaultBinder_BindBody(t *testing.T) {
679+
// tests to check binding behaviour when multiple sources path params, query params and request body are in use
680+
// generally when binding from request body - URL and path params are ignored - unless form is being binded.
681+
// these tests are to document this behaviour and detect further possible regressions when bind implementation is changed
682+
683+
type Node struct {
684+
ID int `json:"id" xml:"id"`
685+
Node string `json:"node" xml:"node"`
686+
}
687+
type Nodes struct {
688+
Nodes []Node `xml:"node" form:"node"`
689+
}
690+
691+
var testCases = []struct {
692+
name string
693+
givenURL string
694+
givenContent io.Reader
695+
givenMethod string
696+
givenContentType string
697+
whenNoPathParams bool
698+
whenBindTarget interface{}
699+
expect interface{}
700+
expectError string
701+
}{
702+
{
703+
name: "ok, JSON POST bind to struct with: path + query + empty field in body",
704+
givenURL: "/api/real_node/endpoint?node=xxx",
705+
givenMethod: http.MethodPost,
706+
givenContentType: MIMEApplicationJSON,
707+
givenContent: strings.NewReader(`{"id": 1}`),
708+
expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body
709+
},
710+
{
711+
name: "ok, JSON POST bind to struct with: path + query + body",
712+
givenURL: "/api/real_node/endpoint?node=xxx",
713+
givenMethod: http.MethodPost,
714+
givenContentType: MIMEApplicationJSON,
715+
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
716+
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
717+
},
718+
{
719+
name: "ok, JSON POST body bind json array to slice (has matching path/query params)",
720+
givenURL: "/api/real_node/endpoint?node=xxx",
721+
givenMethod: http.MethodPost,
722+
givenContentType: MIMEApplicationJSON,
723+
givenContent: strings.NewReader(`[{"id": 1}]`),
724+
whenNoPathParams: true,
725+
whenBindTarget: &[]Node{},
726+
expect: &[]Node{{ID: 1, Node: ""}},
727+
expectError: "",
728+
},
729+
{ // rare case as GET is not usually used to send request body
730+
name: "ok, JSON GET bind to struct with: path + query + empty field in body",
731+
givenURL: "/api/real_node/endpoint?node=xxx",
732+
givenMethod: http.MethodGet,
733+
givenContentType: MIMEApplicationJSON,
734+
givenContent: strings.NewReader(`{"id": 1}`),
735+
expect: &Node{ID: 1, Node: ""}, // path params or query params should not interfere with body
736+
},
737+
{ // rare case as GET is not usually used to send request body
738+
name: "ok, JSON GET bind to struct with: path + query + body",
739+
givenURL: "/api/real_node/endpoint?node=xxx",
740+
givenMethod: http.MethodGet,
741+
givenContentType: MIMEApplicationJSON,
742+
givenContent: strings.NewReader(`{"id": 1, "node": "zzz"}`),
743+
expect: &Node{ID: 1, Node: "zzz"}, // field value from content has higher priority
744+
},
745+
{
746+
name: "nok, JSON POST body bind failure",
747+
givenURL: "/api/real_node/endpoint?node=xxx",
748+
givenMethod: http.MethodPost,
749+
givenContentType: MIMEApplicationJSON,
750+
givenContent: strings.NewReader(`{`),
751+
expect: &Node{ID: 0, Node: ""},
752+
expectError: "code=400, message=unexpected EOF, internal=unexpected EOF",
753+
},
754+
{
755+
name: "ok, XML POST bind to struct with: path + query + empty body",
756+
givenURL: "/api/real_node/endpoint?node=xxx",
757+
givenMethod: http.MethodPost,
758+
givenContentType: MIMEApplicationXML,
759+
givenContent: strings.NewReader(`<node><id>1</id><node>yyy</node></node>`),
760+
expect: &Node{ID: 1, Node: "yyy"},
761+
},
762+
{
763+
name: "ok, XML POST bind array to slice with: path + query + body",
764+
givenURL: "/api/real_node/endpoint?node=xxx",
765+
givenMethod: http.MethodPost,
766+
givenContentType: MIMEApplicationXML,
767+
givenContent: strings.NewReader(`<nodes><node><id>1</id><node>yyy</node></node></nodes>`),
768+
whenBindTarget: &Nodes{},
769+
expect: &Nodes{Nodes: []Node{{ID: 1, Node: "yyy"}}},
770+
},
771+
{
772+
name: "nok, XML POST bind failure",
773+
givenURL: "/api/real_node/endpoint?node=xxx",
774+
givenMethod: http.MethodPost,
775+
givenContentType: MIMEApplicationXML,
776+
givenContent: strings.NewReader(`<node><`),
777+
expect: &Node{ID: 0, Node: ""},
778+
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",
779+
},
780+
{
781+
name: "ok, FORM POST bind to struct with: path + query + empty body",
782+
givenURL: "/api/real_node/endpoint?node=xxx",
783+
givenMethod: http.MethodPost,
784+
givenContentType: MIMEApplicationForm,
785+
givenContent: strings.NewReader(`id=1&node=yyy`),
786+
expect: &Node{ID: 1, Node: "yyy"},
787+
},
788+
{
789+
// NB: form values are taken from BOTH body and query for POST/PUT/PATCH by standard library implementation
790+
// See: https://golang.org/pkg/net/http/#Request.ParseForm
791+
name: "ok, FORM POST bind to struct with: path + query + empty field in body",
792+
givenURL: "/api/real_node/endpoint?node=xxx",
793+
givenMethod: http.MethodPost,
794+
givenContentType: MIMEApplicationForm,
795+
givenContent: strings.NewReader(`id=1`),
796+
expect: &Node{ID: 1, Node: "xxx"},
797+
},
798+
{
799+
// NB: form values are taken from query by standard library implementation
800+
// See: https://golang.org/pkg/net/http/#Request.ParseForm
801+
name: "ok, FORM GET bind to struct with: path + query + empty field in body",
802+
givenURL: "/api/real_node/endpoint?node=xxx",
803+
givenMethod: http.MethodGet,
804+
givenContentType: MIMEApplicationForm,
805+
givenContent: strings.NewReader(`id=1`),
806+
expect: &Node{ID: 0, Node: "xxx"}, // 'xxx' is taken from URL and body is not used with GET by implementation
807+
},
808+
{
809+
name: "nok, unsupported content type",
810+
givenURL: "/api/real_node/endpoint?node=xxx",
811+
givenMethod: http.MethodPost,
812+
givenContentType: MIMETextPlain,
813+
givenContent: strings.NewReader(`<html></html>`),
814+
expect: &Node{ID: 0, Node: ""},
815+
expectError: "code=415, message=Unsupported Media Type",
816+
},
817+
}
818+
819+
for _, tc := range testCases {
820+
t.Run(tc.name, func(t *testing.T) {
821+
e := New()
822+
// assume route we are testing is "/api/:node/endpoint?some_query_params=here"
823+
req := httptest.NewRequest(tc.givenMethod, tc.givenURL, tc.givenContent)
824+
switch tc.givenContentType {
825+
case MIMEApplicationXML:
826+
req.Header.Set(HeaderContentType, MIMEApplicationXML)
827+
case MIMEApplicationForm:
828+
req.Header.Set(HeaderContentType, MIMEApplicationForm)
829+
case MIMEApplicationJSON:
830+
req.Header.Set(HeaderContentType, MIMEApplicationJSON)
831+
}
832+
rec := httptest.NewRecorder()
833+
c := e.NewContext(req, rec)
834+
835+
if !tc.whenNoPathParams {
836+
c.SetParamNames("node")
837+
c.SetParamValues("real_node")
838+
}
839+
840+
var bindTarget interface{}
841+
if tc.whenBindTarget != nil {
842+
bindTarget = tc.whenBindTarget
843+
} else {
844+
bindTarget = &Node{}
845+
}
846+
b := new(DefaultBinder)
847+
848+
err := b.BindBody(c, bindTarget)
849+
if tc.expectError != "" {
850+
assert.EqualError(t, err, tc.expectError)
851+
} else {
852+
assert.NoError(t, err)
853+
}
854+
assert.Equal(t, tc.expect, bindTarget)
855+
})
856+
}
857+
}

0 commit comments

Comments
 (0)