From 011acb4732fe4bdca1cb6d8ea9c29735e0b941f7 Mon Sep 17 00:00:00 2001 From: Martti T Date: Wed, 13 Mar 2024 22:07:08 +0200 Subject: [PATCH] default binder can bind pointer to slice as struct field. For example `*[]string` (#2608) --- bind.go | 28 +++++--- bind_test.go | 192 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 149 insertions(+), 71 deletions(-) diff --git a/bind.go b/bind.go index b6146e8ca..507def3ea 100644 --- a/bind.go +++ b/bind.go @@ -188,14 +188,14 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri } structFieldKind := structField.Kind() inputFieldName := typeField.Tag.Get(tag) - if typeField.Anonymous && structField.Kind() == reflect.Struct && inputFieldName != "" { + if typeField.Anonymous && structFieldKind == reflect.Struct && inputFieldName != "" { // if anonymous struct with query/param/form tags, report an error return errors.New("query/param/form tags are not allowed with anonymous struct field") } if inputFieldName == "" { // If tag is nil, we inspect if the field is a not BindUnmarshaler struct and try to bind data into it (might contains fields with tags). - // structs that implement BindUnmarshaler are binded only when they have explicit tag + // structs that implement BindUnmarshaler are bound only when they have explicit tag if _, ok := structField.Addr().Interface().(BindUnmarshaler); !ok && structFieldKind == reflect.Struct { if err := b.bindData(structField.Addr().Interface(), data, tag); err != nil { return err @@ -224,6 +224,10 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri continue } + // NOTE: algorithm here is not particularly sophisticated. It probably does not work with absurd types like `**[]*int` + // but it is smart enough to handle niche cases like `*int`,`*[]string`,`[]*int` . + + // try unmarshalling first, in case we're dealing with an alias to an array type if ok, err := unmarshalInputsToField(typeField.Type.Kind(), inputValue, structField); ok { if err != nil { return err @@ -231,7 +235,6 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri continue } - // Call this first, in case we're dealing with an alias to an array type if ok, err := unmarshalInputToField(typeField.Type.Kind(), inputValue[0], structField); ok { if err != nil { return err @@ -239,19 +242,28 @@ func (b *DefaultBinder) bindData(destination interface{}, data map[string][]stri continue } - numElems := len(inputValue) - if structFieldKind == reflect.Slice && numElems > 0 { + // we could be dealing with pointer to slice `*[]string` so dereference it. There are wierd OpenAPI generators + // that could create struct fields like that. + if structFieldKind == reflect.Pointer { + structFieldKind = structField.Elem().Kind() + structField = structField.Elem() + } + + if structFieldKind == reflect.Slice { sliceOf := structField.Type().Elem().Kind() + numElems := len(inputValue) slice := reflect.MakeSlice(structField.Type(), numElems, numElems) for j := 0; j < numElems; j++ { if err := setWithProperType(sliceOf, inputValue[j], slice.Index(j)); err != nil { return err } } - val.Field(i).Set(slice) - } else if err := setWithProperType(typeField.Type.Kind(), inputValue[0], structField); err != nil { - return err + structField.Set(slice) + continue + } + if err := setWithProperType(structFieldKind, inputValue[0], structField); err != nil { + return err } } return nil diff --git a/bind_test.go b/bind_test.go index 9647d6566..db7693cb9 100644 --- a/bind_test.go +++ b/bind_test.go @@ -168,6 +168,11 @@ var values = map[string][]string{ "ST": {"bar"}, } +// ptr return pointer to value. This is useful as `v := []*int8{&int8(1)}` will not compile +func ptr[T any](value T) *T { + return &value +} + func TestToMultipleFields(t *testing.T) { e := New() req := httptest.NewRequest(http.MethodGet, "/?id=1&ID=2", nil) @@ -747,7 +752,7 @@ func testBindError(t *testing.T, r io.Reader, ctype string, expectedInternal err } func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) { - // tests to check binding behaviour when multiple sources path params, query params and request body are in use + // 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 @@ -917,7 +922,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) { } func TestDefaultBinder_BindBody(t *testing.T) { - // tests to check binding behaviour when multiple sources path params, query params and request body are in use + // 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 @@ -1097,6 +1102,14 @@ func TestDefaultBinder_BindBody(t *testing.T) { } } +func testBindURL(queryString string, target any) error { + e := New() + req := httptest.NewRequest(http.MethodGet, queryString, nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + return c.Bind(target) +} + type unixTimestamp struct { Time time.Time } @@ -1136,27 +1149,19 @@ func TestBindUnmarshalParamExtras(t *testing.T) { // NOTE: BindUnmarshaler chooses first input value to be bound. t.Run("nok, unmarshalling fails", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?t=xxxx", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V unixTimestamp `query:"t"` }{} - err := c.Bind(&result) + err := testBindURL("/?t=xxxx", &result) assert.EqualError(t, err, "code=400, message='xxxx' is not an integer, internal='xxxx' is not an integer") }) t.Run("ok, target is struct", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?t=1710095540&t=1710095541", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V unixTimestamp `query:"t"` }{} - err := c.Bind(&result) + err := testBindURL("/?t=1710095540&t=1710095541", &result) assert.NoError(t, err) expect := unixTimestamp{ @@ -1166,42 +1171,30 @@ func TestBindUnmarshalParamExtras(t *testing.T) { }) t.Run("ok, target is an alias to slice and is nil, append only values from first", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1,2,3&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V IntArrayA `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1,2,3&a=4,5,6", &result) assert.NoError(t, err) assert.Equal(t, IntArrayA([]int{1, 2, 3}), result.V) }) t.Run("ok, target is an alias to slice and is nil, single input", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1,2", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V IntArrayA `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1,2", &result) assert.NoError(t, err) assert.Equal(t, IntArrayA([]int{1, 2}), result.V) }) t.Run("ok, target is pointer an alias to slice and is nil", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V *IntArrayA `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1&a=4,5,6", &result) assert.NoError(t, err) var expected = IntArrayA([]int{1}) @@ -1209,16 +1202,12 @@ func TestBindUnmarshalParamExtras(t *testing.T) { }) t.Run("ok, target is pointer an alias to slice and is NOT nil", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V *IntArrayA `query:"a"` }{} result.V = new(IntArrayA) // NOT nil - err := c.Bind(&result) + err := testBindURL("/?a=1&a=4,5,6", &result) assert.NoError(t, err) var expected = IntArrayA([]int{1}) @@ -1265,27 +1254,19 @@ func TestBindUnmarshalParams(t *testing.T) { // this test documents how bind handles `bindMultipleUnmarshaler` interface: t.Run("nok, unmarshalling fails", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?t=xxxx", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V unixTimestampLast `query:"t"` }{} - err := c.Bind(&result) + err := testBindURL("/?t=xxxx", &result) assert.EqualError(t, err, "code=400, message='xxxx' is not an integer, internal='xxxx' is not an integer") }) t.Run("ok, target is struct", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?t=1710095540&t=1710095541", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V unixTimestampLast `query:"t"` }{} - err := c.Bind(&result) + err := testBindURL("/?t=1710095540&t=1710095541", &result) assert.NoError(t, err) expect := unixTimestampLast{ @@ -1295,42 +1276,30 @@ func TestBindUnmarshalParams(t *testing.T) { }) t.Run("ok, target is an alias to slice and is nil, append multiple inputs", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1,2,3&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V IntArrayB `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1,2,3&a=4,5,6", &result) assert.NoError(t, err) assert.Equal(t, IntArrayB([]int{1, 2, 3, 4, 5, 6}), result.V) }) t.Run("ok, target is an alias to slice and is nil, single input", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1,2", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V IntArrayB `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1,2", &result) assert.NoError(t, err) assert.Equal(t, IntArrayB([]int{1, 2}), result.V) }) t.Run("ok, target is pointer an alias to slice and is nil", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V *IntArrayB `query:"a"` }{} - err := c.Bind(&result) + err := testBindURL("/?a=1&a=4,5,6", &result) assert.NoError(t, err) var expected = IntArrayB([]int{1, 4, 5, 6}) @@ -1338,19 +1307,116 @@ func TestBindUnmarshalParams(t *testing.T) { }) t.Run("ok, target is pointer an alias to slice and is NOT nil", func(t *testing.T) { - e := New() - req := httptest.NewRequest(http.MethodGet, "/?a=1&a=4,5,6", nil) - rec := httptest.NewRecorder() - c := e.NewContext(req, rec) result := struct { V *IntArrayB `query:"a"` }{} result.V = new(IntArrayB) // NOT nil - err := c.Bind(&result) - + err := testBindURL("/?a=1&a=4,5,6", &result) assert.NoError(t, err) var expected = IntArrayB([]int{1, 4, 5, 6}) assert.Equal(t, &expected, result.V) }) } + +func TestBindInt8(t *testing.T) { + t.Run("nok, binding fails", func(t *testing.T) { + type target struct { + V int8 `query:"v"` + } + p := target{} + err := testBindURL("/?v=x&v=2", &p) + assert.EqualError(t, err, "code=400, message=strconv.ParseInt: parsing \"x\": invalid syntax, internal=strconv.ParseInt: parsing \"x\": invalid syntax") + }) + + t.Run("nok, int8 embedded in struct", func(t *testing.T) { + type target struct { + int8 `query:"v"` // embedded field is `Anonymous`. We can only set public fields + } + p := target{} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{0}, p) + }) + + t.Run("nok, pointer to int8 embedded in struct", func(t *testing.T) { + type target struct { + *int8 `query:"v"` // embedded field is `Anonymous`. We can only set public fields + } + p := target{} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + + assert.Equal(t, target{int8: nil}, p) + }) + + t.Run("ok, bind int8 as struct field", func(t *testing.T) { + type target struct { + V int8 `query:"v"` + } + p := target{V: 127} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: 1}, p) + }) + + t.Run("ok, bind pointer to int8 as struct field, value is nil", func(t *testing.T) { + type target struct { + V *int8 `query:"v"` + } + p := target{} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: ptr(int8(1))}, p) + }) + + t.Run("ok, bind pointer to int8 as struct field, value is set", func(t *testing.T) { + type target struct { + V *int8 `query:"v"` + } + p := target{V: ptr(int8(127))} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: ptr(int8(1))}, p) + }) + + t.Run("ok, bind int8 slice as struct field, value is nil", func(t *testing.T) { + type target struct { + V []int8 `query:"v"` + } + p := target{} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: []int8{1, 2}}, p) + }) + + t.Run("ok, bind slice of int8 as struct field, value is set", func(t *testing.T) { + type target struct { + V []int8 `query:"v"` + } + p := target{V: []int8{111}} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: []int8{1, 2}}, p) + }) + + t.Run("ok, bind slice of pointer to int8 as struct field, value is set", func(t *testing.T) { + type target struct { + V []*int8 `query:"v"` + } + p := target{V: []*int8{ptr(int8(127))}} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: []*int8{ptr(int8(1)), ptr(int8(2))}}, p) + }) + + t.Run("ok, bind pointer to slice of int8 as struct field, value is set", func(t *testing.T) { + type target struct { + V *[]int8 `query:"v"` + } + p := target{V: &[]int8{111}} + err := testBindURL("/?v=1&v=2", &p) + assert.NoError(t, err) + assert.Equal(t, target{V: &[]int8{1, 2}}, p) + }) +}