From 2cb4268b71cdf37e527e097fd56fd5ef21d45efe Mon Sep 17 00:00:00 2001 From: Alexej Kubarev Date: Thu, 9 Jan 2020 14:04:19 +0100 Subject: [PATCH] Supporting pointers when decoding responses. This is similar to how `json.Unmarshal` behaves (logic adapted from `encoding/json`). When value is decoded corresponding field is check for pointers and pointer is then followed and initialized, to allow assignment of value to it. Fixes #2 --- CHANGELOG.md | 2 ++ Makefile | 2 +- client_test.go | 8 ++++-- decode.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++---- decode_test.go | 39 +++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f581c77..a41da5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ ## 0.1.2 Improvements to parsing logic for responses: + * If response struct members are in snake-case - Go struct should have member in camel-case * It is now possible to use type aliases when decoding a response (#1) +* It is now possible to use pointers to fields, without getting an error (#2) ## 0.1.1 diff --git a/Makefile b/Makefile index 1523b93..4d6a8da 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ lint: .PHONY: test test: @echo "Running tests..." - @go test ./... + @go test -short ./... #-------------------------------- # Build steps diff --git a/client_test.go b/client_test.go index 4c24595..a993c5a 100644 --- a/client_test.go +++ b/client_test.go @@ -33,7 +33,7 @@ func TestClient_Call(t *testing.T) { assert.Equal(t, "12345", m.Params[0].Value.Int) file := nameParts[1] - fmt.Fprintln(w, string(loadTestFile(t, fmt.Sprintf("response_%s.xml", file)))) + _, _ = fmt.Fprintln(w, string(loadTestFile(t, fmt.Sprintf("response_%s.xml", file)))) })) defer ts.Close() @@ -60,7 +60,7 @@ func TestClient_Call(t *testing.T) { func TestClient_Fault(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, string(loadTestFile(t, "response_fault.xml"))) + _, _ = fmt.Fprint(w, string(loadTestFile(t, "response_fault.xml"))) })) defer ts.Close() @@ -77,6 +77,10 @@ func TestClient_Fault(t *testing.T) { func TestClient_Bugzilla(t *testing.T) { + if testing.Short() { + t.Skip("skipping making remote call in tests") + } + c, err := NewClient("https://bugzilla.mozilla.org/xmlrpc.cgi") assert.NoError(t, err) assert.NotNil(t, c) diff --git a/decode.go b/decode.go index d5c97b3..cabfaf9 100644 --- a/decode.go +++ b/decode.go @@ -48,7 +48,7 @@ func (d *StdDecoder) Decode(response *Response, v interface{}) error { for i, param := range response.Params { field := vElem.Field(i) - if err := d.decodeValue(¶m.Value, &field); err != nil { + if err := d.decodeValue(¶m.Value, field); err != nil { return err } } @@ -84,7 +84,9 @@ func (d *StdDecoder) decodeFault(fault *ResponseFault) *Fault { return f } -func (d *StdDecoder) decodeValue(value *ResponseValue, field *reflect.Value) error { +func (d *StdDecoder) decodeValue(value *ResponseValue, field reflect.Value) error { + + field = indirect(field) var val interface{} var err error @@ -122,7 +124,7 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field *reflect.Value) err slice := reflect.MakeSlice(reflect.TypeOf(field.Interface()), len(value.Array), len(value.Array)) for i, v := range value.Array { item := slice.Index(i) - if err := d.decodeValue(v, &item); err != nil { + if err := d.decodeValue(v, item); err != nil { return fmt.Errorf("failed decoding array item at index %d: %w", i, err) } } @@ -132,7 +134,6 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field *reflect.Value) err // Struct decoding case len(value.Struct) != 0: - // TODO: Support following *Ptr if field.Kind() != reflect.Struct { return fmt.Errorf(errFormatInvalidFieldType, reflect.Struct.String(), field.Kind().String()) } @@ -147,7 +148,7 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field *reflect.Value) err return fmt.Errorf("cannot find field '%s' on struct", fName) } - if err := d.decodeValue(&m.Value, &f); err != nil { + if err := d.decodeValue(&m.Value, f); err != nil { return fmt.Errorf("failed decoding struct member '%s': %w", m.Name, err) } } @@ -248,3 +249,63 @@ func structMemberToFieldName(structName string) string { return b.String() } + +// indirect walks down v allocating pointers as needed, +// until it gets to a non-pointer. +// +// Adapted from encoding/json indirect() function +// https://golang.org/src/encoding/json/decode.go?#L480 +func indirect(v reflect.Value) reflect.Value { + + // After the first round-trip, we set v back to the original value to + // preserve the original RW flags contained in reflect.Value. + v0 := v + haveAddr := false + + // If v is a named type and is addressable, + // start with its address, so that if the type has pointer methods, + // we find them. + if v.Kind() != reflect.Ptr && v.Type().Name() != "" && v.CanAddr() { + haveAddr = true + v = v.Addr() + } + + for { + + // Load value from interface, but only if the result will be + // usefully addressable. + if v.Kind() == reflect.Interface && !v.IsNil() { + e := v.Elem() + if e.Kind() == reflect.Ptr && !e.IsNil() { + haveAddr = false + v = e + continue + } + } + + if v.Kind() != reflect.Ptr { + break + } + + // Prevent infinite loop if v is an interface pointing to its own address: + // var v interface{} + // v = &v + if v.Elem().Kind() == reflect.Interface && v.Elem().Elem() == v { + v = v.Elem() + break + } + + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + + if haveAddr { + v = v0 // restore original value after round-trip Value.Addr().Elem() + haveAddr = false + } else { + v = v.Elem() + } + } + + return v +} diff --git a/decode_test.go b/decode_test.go index b4a000b..5dbef87 100644 --- a/decode_test.go +++ b/decode_test.go @@ -135,6 +135,13 @@ func TestStdDecoder_DecodeRaw_StructFields(t *testing.T) { type StrAlias string type IntAlias int + sPtr := func(v string) *string { + return &v + } + iPtr := func(v int) *int { + return &v + } + tests := []struct { name string testFile string @@ -174,6 +181,38 @@ func TestStdDecoder_DecodeRaw_StructFields(t *testing.T) { }, }, }, + { + name: "struct pointer", + testFile: "response_struct.xml", + v: &struct { + Struct *struct { + Foo *string + Baz int + WoBleBobble bool + WoBleBobble2 *int + } + }{}, + expect: &struct { + Struct *struct { + Foo *string + Baz int + WoBleBobble bool + WoBleBobble2 *int + } + }{ + Struct: &struct { + Foo *string + Baz int + WoBleBobble bool + WoBleBobble2 *int + }{ + Foo: sPtr("bar"), + Baz: 2, + WoBleBobble: true, + WoBleBobble2: iPtr(34), + }, + }, + }, { name: "struct non-convertible type", testFile: "response_struct.xml",