From 06793e42d6fdaaf05ad98d4fdbd0062c5640fd62 Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Thu, 30 Apr 2020 21:11:04 +0200 Subject: [PATCH 1/6] feature: multicall support, with fault handling --- client.go | 46 +++++++-- encoder.go | 2 - fixtures/multicall_error.xml | 194 +++++++++++++++++++++++++++++++++++ fixtures/multicall_ok.xml | 45 ++++++++ is_zero.go | 2 +- multicall.go | 110 ++++++++++++++++++++ multicall_test.go | 74 +++++++++++++ response.go | 37 +++++++ 8 files changed, 498 insertions(+), 12 deletions(-) create mode 100644 fixtures/multicall_error.xml create mode 100644 fixtures/multicall_ok.xml create mode 100644 multicall.go create mode 100644 multicall_test.go diff --git a/client.go b/client.go index 643dc1c..409846d 100644 --- a/client.go +++ b/client.go @@ -11,10 +11,31 @@ import ( "sync" ) +const multicallMethod = "system.multicall" + type Client struct { *rpc.Client } +// Multicall performs a multicall request. +// `args` should be constructed with `NewMulticallArg` +// and `outs` must be an array/slice of pointers. +// If the response contains at least one fault, +// the first is returned. +func (c Client) Multicall(calls []MulticallArg, outs ...interface{}) error { + if len(calls) != len(outs) { + return errors.New("lengths of calls and responses are not matching") + } + return c.Call(multicallMethod, calls, outs) +} + +// store the method name as well +// used to special-case multicall +type responseMethod struct { + response *http.Response + method string +} + // clientCodec is rpc.ClientCodec interface implementation. type clientCodec struct { // url presents url of xmlrpc service @@ -28,10 +49,13 @@ type clientCodec struct { // responses presents map of active requests. It is required to return request id, that // rpc.Client can mark them as done. - responses map[uint64]*http.Response + responses map[uint64]responseMethod mutex sync.Mutex - response Response + response interface { + Err() error + Unmarshal(v interface{}) error + } // ready presents channel, that is used to link request and it`s response. ready chan uint64 @@ -65,7 +89,7 @@ func (codec *clientCodec) WriteRequest(request *rpc.Request, args interface{}) ( } codec.mutex.Lock() - codec.responses[request.Seq] = httpResponse + codec.responses[request.Seq] = responseMethod{response: httpResponse, method: request.ServiceMethod} codec.mutex.Unlock() codec.ready <- request.Seq @@ -83,7 +107,8 @@ func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) response.Seq = seq codec.mutex.Lock() - httpResponse := codec.responses[seq] + resp := codec.responses[seq] + httpResponse, method := resp.response, resp.method delete(codec.responses, seq) codec.mutex.Unlock() @@ -100,14 +125,17 @@ func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) return nil } - resp := Response(body) - if err := resp.Err(); err != nil { + if method == multicallMethod { + codec.response = ResponseMulticall(body) + } else { + codec.response = Response(body) + } + + if err := codec.response.Err(); err != nil { response.Error = err.Error() return nil } - codec.response = resp - return nil } @@ -153,7 +181,7 @@ func NewClient(requrl string, transport http.RoundTripper) (*Client, error) { httpClient: httpClient, close: make(chan uint64), ready: make(chan uint64), - responses: make(map[uint64]*http.Response), + responses: make(map[uint64]responseMethod), cookies: jar, } diff --git a/encoder.go b/encoder.go index 7ab271a..6a1cf95 100644 --- a/encoder.go +++ b/encoder.go @@ -14,8 +14,6 @@ import ( // Base64 represents value in base64 encoding type Base64 string -type encodeFunc func(reflect.Value) ([]byte, error) - func marshal(v interface{}) ([]byte, error) { if v == nil { return []byte{}, nil diff --git a/fixtures/multicall_error.xml b/fixtures/multicall_error.xml new file mode 100644 index 0000000..6bf13dd --- /dev/null +++ b/fixtures/multicall_error.xml @@ -0,0 +1,194 @@ + + + + + + + + + + + + + + + + + label + + Test + + + + orderby + + date + + + + ordertype + + ascending + + + + date + + 1565526014000 + + + + createddate + + 1565526019000 + + + + lastvisitdate + + 1565526459000 + + + + lastfileaddeddate + + 1565526368000 + + + + public + + 0 + + + + allowdownload + + 1 + + + + allowupload + + 0 + + + + allowprintorder + + 1 + + + + allowsendcomments + + 1 + + + + + + + + label + + test2 + + + + orderby + + date + + + + ordertype + + ascending + + + + date + + 1565526625000 + + + + createddate + + 1565526629000 + + + + lastvisitdate + + 1581931485000 + + + + lastfileaddeddate + + 1565526652000 + + + + public + + 0 + + + + allowdownload + + 1 + + + + allowupload + + 0 + + + + allowprintorder + + 1 + + + + allowsendcomments + + 1 + + + + + + + + + + + + + + faultCode + + 205 + + + + faultString + + Error (205) : Can't get properties for the session xxxx + + + + + + + + + + + \ No newline at end of file diff --git a/fixtures/multicall_ok.xml b/fixtures/multicall_ok.xml new file mode 100644 index 0000000..3262ad3 --- /dev/null +++ b/fixtures/multicall_ok.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + nbfiles + + 4 + + + + + + + + + + + + + + nbfiles + + 1 + + + + + + + + + + + + + \ No newline at end of file diff --git a/is_zero.go b/is_zero.go index 65276d0..b3aae8f 100644 --- a/is_zero.go +++ b/is_zero.go @@ -39,6 +39,6 @@ func isZero(v Value) bool { default: // This should never happens, but will act as a safeguard for // later, as a default value doesn't makes sense here. - panic(&ValueError{"reflect.Value.IsZero", v.Kind()}) + panic(&ValueError{Method: "reflect.Value.IsZero", Kind: v.Kind()}) } } diff --git a/multicall.go b/multicall.go new file mode 100644 index 0000000..bea2ff7 --- /dev/null +++ b/multicall.go @@ -0,0 +1,110 @@ +package xmlrpc + +import ( + "bytes" + "encoding/xml" + "fmt" + "strconv" +) + +// Types used for unmarshalling + +// main fault should be already checked +type response struct { + Name xml.Name `xml:"methodResponse"` + Params []param `xml:"params>param"` +} + +type param struct { + Value value `xml:"value"` +} + +type member struct { + Name string `xml:"name"` + Value value `xml:"value"` +} + +type value struct { + Array []value `xml:"array>data>value"` // used for returns + Struct []member `xml:"struct>member"` // used for fault + String string `xml:"string"` // used for faults + Int string `xml:"int"` // used for faults + Raw []byte `xml:",innerxml"` +} + +// getFaultResponse converts faultValue to Fault. +func getFaultResponse(fault []member) FaultError { + var ( + code int + str string + ) + + for _, field := range fault { + if field.Name == "faultCode" { + code, _ = strconv.Atoi(field.Value.Int) + } else if field.Name == "faultString" { + str = field.Value.String + if str == "" { + str = string(field.Value.Raw) + } + } + } + + return FaultError{Code: code, String: str} +} + +// MulticallFault track the position of the fault. +type MulticallFault struct { + FaultError + Index int // 0 based +} + +func (m MulticallFault) Error() string { + return fmt.Sprintf("fault for call %d : %s", m.Index, m.FaultError.Error()) +} + +// returns xml encoded chunks, one for each multicall response +// if there is (at least) one fault, returns the first one +// as error +func splitMulticall(xmlraw []byte) ([][]byte, error) { + // Unmarshal raw XML into the temporal structure + var ret response + + dec := xml.NewDecoder(bytes.NewReader(xmlraw)) + if CharsetReader != nil { + dec.CharsetReader = CharsetReader + } + + if err := dec.Decode(&ret); err != nil { + return nil, err + } + if L := len(ret.Params); L != 1 { + return nil, fmt.Errorf("unexpected number of arguments : got %d", L) + } + // multicall returns one array of values + returns := ret.Params[0].Value.Array + + out := make([][]byte, len(returns)) + for i, oneReturn := range returns { + // multicall return are always wrapped in one-sized array + // otherwise, it's a fault + if len(oneReturn.Array) != 1 { + fault := getFaultResponse(oneReturn.Struct) + return nil, MulticallFault{Index: i, FaultError: fault} + } + // unwrap the value and store raw xml + // to further process + out[i] = oneReturn.Array[0].Raw + } + return out, nil +} + +// MulticallArg stores one call +type MulticallArg struct { + MethodName string `xmlrpc:"methodName"` + Params []interface{} `xmlrpc:"params"` // 1-sized list containing the real arguments +} + +func NewMulticallArg(method string, args interface{}) MulticallArg { + return MulticallArg{MethodName: method, Params: []interface{}{args}} +} diff --git a/multicall_test.go b/multicall_test.go new file mode 100644 index 0000000..2a337c8 --- /dev/null +++ b/multicall_test.go @@ -0,0 +1,74 @@ +package xmlrpc + +import ( + "io/ioutil" + "testing" +) + +func Test_splitMulticall(t *testing.T) { + b, err := ioutil.ReadFile("fixtures/test_error.xml") + if err != nil { + t.Fatal(err) + } + _, err = splitMulticall(b) + mf, ok := err.(MulticallFault) + if !ok { + t.Errorf("expected multicall fault, got %s", err) + } + if mf.Index != 1 { + t.Errorf("wrong position for fault %d", mf.Index) + } + + b, err = ioutil.ReadFile("fixtures/test_ok.xml") + if err != nil { + t.Fatal(err) + } + out, err := splitMulticall(b) + if err != nil { + t.Error(err) + } + if L := len(out); L != 2 { + t.Errorf("expected 2 answers, got %d", L) + } +} + +func TestUnmarshal(t *testing.T) { + b, err := ioutil.ReadFile("fixtures/test_ok.xml") + if err != nil { + t.Fatal(err) + } + type data struct { + NbFiles int `xmlrpc:"nbfiles"` + } + var d1, d2 data + out := []interface{}{&d1, &d2} + err = ResponseMulticall(b).Unmarshal(out) + if err != nil { + t.Error(err) + } + if nb1 := d1.NbFiles; nb1 != 4 { + t.Errorf("expected 4, got %d", nb1) + } + if nb2 := d2.NbFiles; nb2 != 1 { + t.Errorf("expected 4, got %d", nb2) + } + + outArray := [2]interface{}{&d1, &d2} + err = ResponseMulticall(b).Unmarshal(outArray) + if err != nil { + t.Error(err) + } + if nb1 := d1.NbFiles; nb1 != 4 { + t.Errorf("expected 4, got %d", nb1) + } + if nb2 := d2.NbFiles; nb2 != 1 { + t.Errorf("expected 4, got %d", nb2) + } + + var outWrong string + err = ResponseMulticall(b).Unmarshal(&outWrong) + if err == nil { + t.Error("expected error") + } + +} diff --git a/response.go b/response.go index 18e6d36..e42685c 100644 --- a/response.go +++ b/response.go @@ -2,6 +2,7 @@ package xmlrpc import ( "fmt" + "reflect" "regexp" ) @@ -37,6 +38,42 @@ func (r Response) Unmarshal(v interface{}) error { if err := unmarshal(r, v); err != nil { return err } + return nil +} + +type ResponseMulticall []byte + +func (r ResponseMulticall) Err() error { + return Response(r).Err() +} + +// Unmarshal decodes a multicall anwser +// `v` must be a slice/array of pointers +func (r ResponseMulticall) Unmarshal(v interface{}) error { + switch ki := reflect.TypeOf(v).Kind(); ki { + case reflect.Array, reflect.Slice: // OK + default: + return fmt.Errorf("destination for multicall must be Array or Slice, got %s", ki) + } + outSlice := reflect.ValueOf(v) + parts, err := splitMulticall(r) + if err != nil { + return err + } + + if outSlice.Len() != len(parts) { + return fmt.Errorf("invalid number of return destinations : response needs %d, got %d", len(parts), outSlice.Len()) + } + for i, xmlReturn := range parts { + // pointer to one call's destination + elem := outSlice.Index(i).Interface() + + // unmarshal expect a wrapping tag + xmlReturn = append(append([]byte(""), xmlReturn...), ""...) + if err := unmarshal(xmlReturn, elem); err != nil { + return fmt.Errorf("unmarshall number %d failed : %s", i, err.Error()) + } + } return nil } From a395e5c37469d6f18e1f85c34fc0a217578e58a6 Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Thu, 30 Apr 2020 21:12:50 +0200 Subject: [PATCH 2/6] fix tests --- multicall_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/multicall_test.go b/multicall_test.go index 2a337c8..6591a25 100644 --- a/multicall_test.go +++ b/multicall_test.go @@ -6,7 +6,7 @@ import ( ) func Test_splitMulticall(t *testing.T) { - b, err := ioutil.ReadFile("fixtures/test_error.xml") + b, err := ioutil.ReadFile("fixtures/multicall_error.xml") if err != nil { t.Fatal(err) } @@ -19,7 +19,7 @@ func Test_splitMulticall(t *testing.T) { t.Errorf("wrong position for fault %d", mf.Index) } - b, err = ioutil.ReadFile("fixtures/test_ok.xml") + b, err = ioutil.ReadFile("fixtures/multicall_ok.xml") if err != nil { t.Fatal(err) } @@ -33,7 +33,7 @@ func Test_splitMulticall(t *testing.T) { } func TestUnmarshal(t *testing.T) { - b, err := ioutil.ReadFile("fixtures/test_ok.xml") + b, err := ioutil.ReadFile("fixtures/multicall_ok.xml") if err != nil { t.Fatal(err) } From 865f7327a5088a9a46e2a12385cdebeb8327d761 Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Thu, 30 Apr 2020 22:18:50 +0200 Subject: [PATCH 3/6] better errors report --- client.go | 17 +++++++++++++---- multicall.go | 7 ++++--- multicall_test.go | 6 +++--- response.go | 28 ++++++++++++++++++++-------- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/client.go b/client.go index 409846d..0e20265 100644 --- a/client.go +++ b/client.go @@ -19,14 +19,23 @@ type Client struct { // Multicall performs a multicall request. // `args` should be constructed with `NewMulticallArg` -// and `outs` must be an array/slice of pointers. +// and `outs` must be a slice of pointers. // If the response contains at least one fault, -// the first is returned. +// the first is returned as `MulticallFault`. func (c Client) Multicall(calls []MulticallArg, outs ...interface{}) error { if len(calls) != len(outs) { return errors.New("lengths of calls and responses are not matching") } - return c.Call(multicallMethod, calls, outs) + tmp := responsesError{datas: outs} + if err := c.Call(multicallMethod, calls, &tmp); err != nil { + return err + } + if tmp.err != nil { + err := *tmp.err + err.methodName = calls[err.Index].MethodName + return err + } + return nil } // store the method name as well @@ -126,7 +135,7 @@ func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) } if method == multicallMethod { - codec.response = ResponseMulticall(body) + codec.response = responseMulticall(body) } else { codec.response = Response(body) } diff --git a/multicall.go b/multicall.go index bea2ff7..ad7a821 100644 --- a/multicall.go +++ b/multicall.go @@ -53,14 +53,15 @@ func getFaultResponse(fault []member) FaultError { return FaultError{Code: code, String: str} } -// MulticallFault track the position of the fault. +// MulticallFault tracks the position of the fault. type MulticallFault struct { FaultError - Index int // 0 based + Index int // 0 based + methodName string // for better message } func (m MulticallFault) Error() string { - return fmt.Sprintf("fault for call %d : %s", m.Index, m.FaultError.Error()) + return fmt.Sprintf("fault in call %d (%s) : %s", m.Index, m.methodName, m.FaultError.Error()) } // returns xml encoded chunks, one for each multicall response diff --git a/multicall_test.go b/multicall_test.go index 6591a25..4a9070d 100644 --- a/multicall_test.go +++ b/multicall_test.go @@ -42,7 +42,7 @@ func TestUnmarshal(t *testing.T) { } var d1, d2 data out := []interface{}{&d1, &d2} - err = ResponseMulticall(b).Unmarshal(out) + err = responseMulticall(b).Unmarshal(&responsesError{datas: out}) if err != nil { t.Error(err) } @@ -54,7 +54,7 @@ func TestUnmarshal(t *testing.T) { } outArray := [2]interface{}{&d1, &d2} - err = ResponseMulticall(b).Unmarshal(outArray) + err = responseMulticall(b).Unmarshal(&responsesError{datas: outArray}) if err != nil { t.Error(err) } @@ -66,7 +66,7 @@ func TestUnmarshal(t *testing.T) { } var outWrong string - err = ResponseMulticall(b).Unmarshal(&outWrong) + err = responseMulticall(b).Unmarshal(&outWrong) if err == nil { t.Error("expected error") } diff --git a/response.go b/response.go index e42685c..7ec84ae 100644 --- a/response.go +++ b/response.go @@ -41,24 +41,36 @@ func (r Response) Unmarshal(v interface{}) error { return nil } -type ResponseMulticall []byte +type responseMulticall []byte -func (r ResponseMulticall) Err() error { +func (r responseMulticall) Err() error { return Response(r).Err() } -// Unmarshal decodes a multicall anwser -// `v` must be a slice/array of pointers -func (r ResponseMulticall) Unmarshal(v interface{}) error { - switch ki := reflect.TypeOf(v).Kind(); ki { +// tmp storage for multicall responses +type responsesError struct { + err *MulticallFault + datas interface{} // slice/array of pointers +} + +func (r responseMulticall) Unmarshal(v interface{}) error { + out, ok := v.(*responsesError) + if !ok { + return fmt.Errorf("wrong type for destination") + } + + switch ki := reflect.TypeOf(out.datas).Kind(); ki { case reflect.Array, reflect.Slice: // OK default: return fmt.Errorf("destination for multicall must be Array or Slice, got %s", ki) } - outSlice := reflect.ValueOf(v) + outSlice := reflect.ValueOf(out.datas) parts, err := splitMulticall(r) - if err != nil { + if multicallErr, ok := err.(MulticallFault); ok { + out.err = &multicallErr + return nil + } else if err != nil { return err } From a80fea6a84f22d58222fec6429146cf510a3db0d Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Thu, 30 Apr 2020 22:23:06 +0200 Subject: [PATCH 4/6] doc: small fix --- client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 0e20265..42f8fc4 100644 --- a/client.go +++ b/client.go @@ -18,10 +18,10 @@ type Client struct { } // Multicall performs a multicall request. -// `args` should be constructed with `NewMulticallArg` +// `calls` should be constructed with `NewMulticallArg` // and `outs` must be a slice of pointers. // If the response contains at least one fault, -// the first is returned as `MulticallFault`. +// the first is returned as a `MulticallFault` error. func (c Client) Multicall(calls []MulticallArg, outs ...interface{}) error { if len(calls) != len(outs) { return errors.New("lengths of calls and responses are not matching") From 911c5496a0a313f3fc91e80ca758225094db9098 Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Thu, 30 Apr 2020 23:53:36 +0200 Subject: [PATCH 5/6] backward compatibility --- client.go | 46 ++++++++++------------------------------ multicall.go | 33 +++++++++++++++++++++++++++++ multicall_test.go | 7 +++--- response.go | 54 ++++++----------------------------------------- 4 files changed, 55 insertions(+), 85 deletions(-) diff --git a/client.go b/client.go index 42f8fc4..84b8c13 100644 --- a/client.go +++ b/client.go @@ -11,8 +11,6 @@ import ( "sync" ) -const multicallMethod = "system.multicall" - type Client struct { *rpc.Client } @@ -26,23 +24,8 @@ func (c Client) Multicall(calls []MulticallArg, outs ...interface{}) error { if len(calls) != len(outs) { return errors.New("lengths of calls and responses are not matching") } - tmp := responsesError{datas: outs} - if err := c.Call(multicallMethod, calls, &tmp); err != nil { - return err - } - if tmp.err != nil { - err := *tmp.err - err.methodName = calls[err.Index].MethodName - return err - } - return nil -} - -// store the method name as well -// used to special-case multicall -type responseMethod struct { - response *http.Response - method string + tmp := multicallOut{args: calls, datas: outs} + return c.Call("system.multicall", calls, tmp) } // clientCodec is rpc.ClientCodec interface implementation. @@ -58,13 +41,10 @@ type clientCodec struct { // responses presents map of active requests. It is required to return request id, that // rpc.Client can mark them as done. - responses map[uint64]responseMethod + responses map[uint64]*http.Response mutex sync.Mutex - response interface { - Err() error - Unmarshal(v interface{}) error - } + response Response // ready presents channel, that is used to link request and it`s response. ready chan uint64 @@ -98,7 +78,7 @@ func (codec *clientCodec) WriteRequest(request *rpc.Request, args interface{}) ( } codec.mutex.Lock() - codec.responses[request.Seq] = responseMethod{response: httpResponse, method: request.ServiceMethod} + codec.responses[request.Seq] = httpResponse codec.mutex.Unlock() codec.ready <- request.Seq @@ -116,8 +96,7 @@ func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) response.Seq = seq codec.mutex.Lock() - resp := codec.responses[seq] - httpResponse, method := resp.response, resp.method + httpResponse := codec.responses[seq] delete(codec.responses, seq) codec.mutex.Unlock() @@ -134,17 +113,14 @@ func (codec *clientCodec) ReadResponseHeader(response *rpc.Response) (err error) return nil } - if method == multicallMethod { - codec.response = responseMulticall(body) - } else { - codec.response = Response(body) - } - - if err := codec.response.Err(); err != nil { + resp := Response(body) + if err := resp.Err(); err != nil { response.Error = err.Error() return nil } + codec.response = resp + return nil } @@ -190,7 +166,7 @@ func NewClient(requrl string, transport http.RoundTripper) (*Client, error) { httpClient: httpClient, close: make(chan uint64), ready: make(chan uint64), - responses: make(map[uint64]responseMethod), + responses: make(map[uint64]*http.Response), cookies: jar, } diff --git a/multicall.go b/multicall.go index ad7a821..37053af 100644 --- a/multicall.go +++ b/multicall.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/xml" "fmt" + "reflect" "strconv" ) @@ -64,6 +65,38 @@ func (m MulticallFault) Error() string { return fmt.Sprintf("fault in call %d (%s) : %s", m.Index, m.methodName, m.FaultError.Error()) } +func (r Response) unmarshalMulticall(out multicallOut) error { + switch ki := reflect.TypeOf(out.datas).Kind(); ki { + case reflect.Array, reflect.Slice: // OK + default: + return fmt.Errorf("destination for multicall must be Array or Slice, got %s", ki) + } + outSlice := reflect.ValueOf(out.datas) + + parts, err := splitMulticall(r) + if multicallErr, ok := err.(MulticallFault); ok { + multicallErr.methodName = out.args[multicallErr.Index].MethodName + return multicallErr + } else if err != nil { + return err + } + + if outSlice.Len() != len(parts) { + return fmt.Errorf("invalid number of return destinations : response needs %d, got %d", len(parts), outSlice.Len()) + } + for i, xmlReturn := range parts { + // pointer to one call's destination + elem := outSlice.Index(i).Interface() + + // unmarshal expect a wrapping tag + xmlReturn = append(append([]byte(""), xmlReturn...), ""...) + if err := unmarshal(xmlReturn, elem); err != nil { + return fmt.Errorf("unmarshall number %d failed : %s", i, err.Error()) + } + } + return nil +} + // returns xml encoded chunks, one for each multicall response // if there is (at least) one fault, returns the first one // as error diff --git a/multicall_test.go b/multicall_test.go index 4a9070d..7abf0d6 100644 --- a/multicall_test.go +++ b/multicall_test.go @@ -37,12 +37,13 @@ func TestUnmarshal(t *testing.T) { if err != nil { t.Fatal(err) } + calls := make([]MulticallArg, 2) type data struct { NbFiles int `xmlrpc:"nbfiles"` } var d1, d2 data out := []interface{}{&d1, &d2} - err = responseMulticall(b).Unmarshal(&responsesError{datas: out}) + err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: out}) if err != nil { t.Error(err) } @@ -54,7 +55,7 @@ func TestUnmarshal(t *testing.T) { } outArray := [2]interface{}{&d1, &d2} - err = responseMulticall(b).Unmarshal(&responsesError{datas: outArray}) + err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: outArray}) if err != nil { t.Error(err) } @@ -66,7 +67,7 @@ func TestUnmarshal(t *testing.T) { } var outWrong string - err = responseMulticall(b).Unmarshal(&outWrong) + err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: &outWrong}) if err == nil { t.Error("expected error") } diff --git a/response.go b/response.go index 7ec84ae..5fa1bca 100644 --- a/response.go +++ b/response.go @@ -2,7 +2,6 @@ package xmlrpc import ( "fmt" - "reflect" "regexp" ) @@ -34,58 +33,19 @@ func (r Response) Err() error { return fault } -func (r Response) Unmarshal(v interface{}) error { - if err := unmarshal(r, v); err != nil { - return err - } - return nil -} - -type responseMulticall []byte - -func (r responseMulticall) Err() error { - return Response(r).Err() -} - // tmp storage for multicall responses -type responsesError struct { - err *MulticallFault - datas interface{} // slice/array of pointers +type multicallOut struct { + args []MulticallArg // for error messages + datas interface{} // slice/array of pointers } -func (r responseMulticall) Unmarshal(v interface{}) error { - out, ok := v.(*responsesError) - if !ok { - return fmt.Errorf("wrong type for destination") - } - - switch ki := reflect.TypeOf(out.datas).Kind(); ki { - case reflect.Array, reflect.Slice: // OK - default: - return fmt.Errorf("destination for multicall must be Array or Slice, got %s", ki) +func (r Response) Unmarshal(v interface{}) error { + if mc, isMulticall := v.(multicallOut); isMulticall { + return r.unmarshalMulticall(mc) } - outSlice := reflect.ValueOf(out.datas) - parts, err := splitMulticall(r) - if multicallErr, ok := err.(MulticallFault); ok { - out.err = &multicallErr - return nil - } else if err != nil { + if err := unmarshal(r, v); err != nil { return err } - - if outSlice.Len() != len(parts) { - return fmt.Errorf("invalid number of return destinations : response needs %d, got %d", len(parts), outSlice.Len()) - } - for i, xmlReturn := range parts { - // pointer to one call's destination - elem := outSlice.Index(i).Interface() - - // unmarshal expect a wrapping tag - xmlReturn = append(append([]byte(""), xmlReturn...), ""...) - if err := unmarshal(xmlReturn, elem); err != nil { - return fmt.Errorf("unmarshall number %d failed : %s", i, err.Error()) - } - } return nil } From 929d71fa10d4885cd972eae99f6a5dc2174e6a09 Mon Sep 17 00:00:00 2001 From: Benoit KUGLER Date: Fri, 1 May 2020 00:01:07 +0200 Subject: [PATCH 6/6] ref: rename for consistency --- client.go | 2 +- multicall.go | 2 +- multicall_test.go | 6 +++--- response.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client.go b/client.go index 84b8c13..11e2e83 100644 --- a/client.go +++ b/client.go @@ -24,7 +24,7 @@ func (c Client) Multicall(calls []MulticallArg, outs ...interface{}) error { if len(calls) != len(outs) { return errors.New("lengths of calls and responses are not matching") } - tmp := multicallOut{args: calls, datas: outs} + tmp := multicallOut{calls: calls, datas: outs} return c.Call("system.multicall", calls, tmp) } diff --git a/multicall.go b/multicall.go index 37053af..4bfd1c8 100644 --- a/multicall.go +++ b/multicall.go @@ -75,7 +75,7 @@ func (r Response) unmarshalMulticall(out multicallOut) error { parts, err := splitMulticall(r) if multicallErr, ok := err.(MulticallFault); ok { - multicallErr.methodName = out.args[multicallErr.Index].MethodName + multicallErr.methodName = out.calls[multicallErr.Index].MethodName return multicallErr } else if err != nil { return err diff --git a/multicall_test.go b/multicall_test.go index 7abf0d6..45f0f24 100644 --- a/multicall_test.go +++ b/multicall_test.go @@ -43,7 +43,7 @@ func TestUnmarshal(t *testing.T) { } var d1, d2 data out := []interface{}{&d1, &d2} - err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: out}) + err = Response(b).unmarshalMulticall(multicallOut{calls: calls, datas: out}) if err != nil { t.Error(err) } @@ -55,7 +55,7 @@ func TestUnmarshal(t *testing.T) { } outArray := [2]interface{}{&d1, &d2} - err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: outArray}) + err = Response(b).unmarshalMulticall(multicallOut{calls: calls, datas: outArray}) if err != nil { t.Error(err) } @@ -67,7 +67,7 @@ func TestUnmarshal(t *testing.T) { } var outWrong string - err = Response(b).unmarshalMulticall(multicallOut{args: calls, datas: &outWrong}) + err = Response(b).unmarshalMulticall(multicallOut{calls: calls, datas: &outWrong}) if err == nil { t.Error("expected error") } diff --git a/response.go b/response.go index 5fa1bca..64a3f7d 100644 --- a/response.go +++ b/response.go @@ -35,7 +35,7 @@ func (r Response) Err() error { // tmp storage for multicall responses type multicallOut struct { - args []MulticallArg // for error messages + calls []MulticallArg // for error messages datas interface{} // slice/array of pointers }