Skip to content

Commit

Permalink
Supporting pointers when decoding responses.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexejk committed Jan 9, 2020
1 parent 7a73b34 commit 2cb4268
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ lint:
.PHONY: test
test:
@echo "Running tests..."
@go test ./...
@go test -short ./...

#--------------------------------
# Build steps
Expand Down
8 changes: 6 additions & 2 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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)
Expand Down
71 changes: 66 additions & 5 deletions decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(&param.Value, &field); err != nil {
if err := d.decodeValue(&param.Value, field); err != nil {
return err
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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())
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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
}
39 changes: 39 additions & 0 deletions decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 2cb4268

Please sign in to comment.