From edfaa480cfbe3190f338acdb1b30c3f6ec57cab0 Mon Sep 17 00:00:00 2001 From: kargakis Date: Wed, 2 Sep 2015 18:24:47 +0200 Subject: [PATCH 1/3] queryparams: Handle pointer fields in structs --- pkg/conversion/queryparams/convert.go | 20 ++++++++++++++---- pkg/conversion/queryparams/convert_test.go | 24 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/pkg/conversion/queryparams/convert.go b/pkg/conversion/queryparams/convert.go index a9ef5b9796a..ffa50170de0 100644 --- a/pkg/conversion/queryparams/convert.go +++ b/pkg/conversion/queryparams/convert.go @@ -50,6 +50,10 @@ func formatValue(value interface{}) string { return fmt.Sprintf("%v", value) } +func isPointerKind(kind reflect.Kind) bool { + return kind == reflect.Ptr +} + func isValueKind(kind reflect.Kind) bool { switch kind { case reflect.String, reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, @@ -92,11 +96,11 @@ func Convert(obj runtime.Object) (url.Values, error) { case reflect.Ptr, reflect.Interface: sv = reflect.ValueOf(obj).Elem() default: - return nil, fmt.Errorf("Expecting a pointer or interface") + return nil, fmt.Errorf("expecting a pointer or interface") } st := sv.Type() if st.Kind() != reflect.Struct { - return nil, fmt.Errorf("Expecting a pointer to a struct") + return nil, fmt.Errorf("expecting a pointer to a struct") } for i := 0; i < st.NumField(); i++ { field := sv.Field(i) @@ -105,10 +109,18 @@ func Convert(obj runtime.Object) (url.Values, error) { continue } ft := field.Type() + + kind := ft.Kind() + if isPointerKind(kind) { + kind = ft.Elem().Kind() + if !field.IsNil() { + field = reflect.Indirect(field) + } + } switch { - case isValueKind(ft.Kind()): + case isValueKind(kind): addParam(result, tag, omitempty, field) - case ft.Kind() == reflect.Array || ft.Kind() == reflect.Slice: + case kind == reflect.Array || kind == reflect.Slice: if isValueKind(ft.Elem().Kind()) { addListOfParams(result, tag, omitempty, field) } diff --git a/pkg/conversion/queryparams/convert_test.go b/pkg/conversion/queryparams/convert_test.go index 88390a5dcd8..664e128046b 100644 --- a/pkg/conversion/queryparams/convert_test.go +++ b/pkg/conversion/queryparams/convert_test.go @@ -53,6 +53,13 @@ type foo struct { func (*foo) IsAnAPIObject() {} +type baz struct { + Ptr *int `json:"ptr"` + Bptr *bool `json:"bptr,omitempty"` +} + +func (*baz) IsAnAPIObject() {} + func validateResult(t *testing.T, input interface{}, actual, expected url.Values) { local := url.Values{} for k, v := range expected { @@ -131,6 +138,19 @@ func TestConvert(t *testing.T) { }, expected: url.Values{"str": {""}, "namedStr": {"named str"}}, }, + { + input: &baz{ + Ptr: intp(5), + Bptr: boolp(true), + }, + expected: url.Values{"ptr": {"5"}, "bptr": {"true"}}, + }, + { + input: &baz{ + Bptr: boolp(true), + }, + expected: url.Values{"ptr": {""}, "bptr": {"true"}}, + }, } for _, test := range tests { @@ -141,3 +161,7 @@ func TestConvert(t *testing.T) { validateResult(t, test.input, result, test.expected) } } + +func intp(n int) *int { return &n } + +func boolp(b bool) *bool { return &b } From 6c32e071f4ab1bcc6f63c20d8c72ceabae664cdd Mon Sep 17 00:00:00 2001 From: kargakis Date: Thu, 3 Sep 2015 15:32:19 +0200 Subject: [PATCH 2/3] Dont output nil; test nil & omitempty --- pkg/conversion/queryparams/convert.go | 8 +++++++- pkg/conversion/queryparams/convert_test.go | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/conversion/queryparams/convert.go b/pkg/conversion/queryparams/convert.go index ffa50170de0..9be5f581e61 100644 --- a/pkg/conversion/queryparams/convert.go +++ b/pkg/conversion/queryparams/convert.go @@ -74,7 +74,13 @@ func addParam(values url.Values, tag string, omitempty bool, value reflect.Value if omitempty && zeroValue(value) { return } - values.Add(tag, fmt.Sprintf("%v", value.Interface())) + val := "" + iValue := fmt.Sprintf("%v", value.Interface()) + + if iValue != "" { + val = iValue + } + values.Add(tag, val) } func addListOfParams(values url.Values, tag string, omitempty bool, list reflect.Value) { diff --git a/pkg/conversion/queryparams/convert_test.go b/pkg/conversion/queryparams/convert_test.go index 664e128046b..e068f0d9b08 100644 --- a/pkg/conversion/queryparams/convert_test.go +++ b/pkg/conversion/queryparams/convert_test.go @@ -151,6 +151,12 @@ func TestConvert(t *testing.T) { }, expected: url.Values{"ptr": {""}, "bptr": {"true"}}, }, + { + input: &baz{ + Ptr: intp(5), + }, + expected: url.Values{"ptr": {"5"}}, + }, } for _, test := range tests { From f9bca7bc7df013d3ec134008a5f06fce6947707d Mon Sep 17 00:00:00 2001 From: kargakis Date: Thu, 3 Sep 2015 15:44:03 +0200 Subject: [PATCH 3/3] handle structs --- pkg/conversion/queryparams/convert.go | 18 ++++++++++++++++-- pkg/conversion/queryparams/convert_test.go | 6 +++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/pkg/conversion/queryparams/convert.go b/pkg/conversion/queryparams/convert.go index 9be5f581e61..450a43001a2 100644 --- a/pkg/conversion/queryparams/convert.go +++ b/pkg/conversion/queryparams/convert.go @@ -54,6 +54,10 @@ func isPointerKind(kind reflect.Kind) bool { return kind == reflect.Ptr } +func isStructKind(kind reflect.Kind) bool { + return kind == reflect.Struct +} + func isValueKind(kind reflect.Kind) bool { switch kind { case reflect.String, reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, @@ -105,9 +109,17 @@ func Convert(obj runtime.Object) (url.Values, error) { return nil, fmt.Errorf("expecting a pointer or interface") } st := sv.Type() - if st.Kind() != reflect.Struct { + if !isStructKind(st.Kind()) { return nil, fmt.Errorf("expecting a pointer to a struct") } + + // Check all object fields + convertStruct(result, st, sv) + + return result, nil +} + +func convertStruct(result url.Values, st reflect.Type, sv reflect.Value) { for i := 0; i < st.NumField(); i++ { field := sv.Field(i) tag, omitempty := jsonTag(st.Field(i)) @@ -123,6 +135,7 @@ func Convert(obj runtime.Object) (url.Values, error) { field = reflect.Indirect(field) } } + switch { case isValueKind(kind): addParam(result, tag, omitempty, field) @@ -130,7 +143,8 @@ func Convert(obj runtime.Object) (url.Values, error) { if isValueKind(ft.Elem().Kind()) { addListOfParams(result, tag, omitempty, field) } + case isStructKind(kind) && !(zeroValue(field) && omitempty): + convertStruct(result, ft, field) } } - return result, nil } diff --git a/pkg/conversion/queryparams/convert_test.go b/pkg/conversion/queryparams/convert_test.go index e068f0d9b08..c409756e44a 100644 --- a/pkg/conversion/queryparams/convert_test.go +++ b/pkg/conversion/queryparams/convert_test.go @@ -111,12 +111,12 @@ func TestConvert(t *testing.T) { }, { input: &foo{ - Str: "ignore embedded struct", + Str: "don't ignore embedded struct", Foobar: bar{ Float1: 5.0, }, }, - expected: url.Values{"str": {"ignore embedded struct"}}, + expected: url.Values{"str": {"don't ignore embedded struct"}, "float1": {"5"}, "float2": {"0"}}, }, { // Ignore untagged fields @@ -149,7 +149,7 @@ func TestConvert(t *testing.T) { input: &baz{ Bptr: boolp(true), }, - expected: url.Values{"ptr": {""}, "bptr": {"true"}}, + expected: url.Values{"ptr": {""}, "bptr": {"true"}}, }, { input: &baz{