A bit of refactoring and more documentation

Author: alexejk

client.go

@@ -7,15 +7,20 @@ import (
 	"net/url"
 )
 
+// Client is responsible for making calls to RPC services with help of underlying rpc.Client.
 type Client struct {
 	*rpc.Client
 }
 
+// NewClient creates a Client with http.DefaultClient.
+// If provided endpoint is not valid, an error is returned.
 func NewClient(endpoint string) (*Client, error) {
 
 	return NewClientWithHttpClient(endpoint, http.DefaultClient)
 }
 
+// NewClientWithHttpClient allows customization of http.Client used to make RPC calls.
+// If provided endpoint is not valid, an error is returned.
 func NewClientWithHttpClient(endpoint string, httpClient *http.Client) (*Client, error) {
 
 	// Parse Endpoint URL

client_test.go

@@ -17,8 +17,8 @@ func TestClient_Call(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 
 		m := &struct {
-			Name   string       `xml:"methodName"`
-			Params []*respParam `xml:"params>param"`
+			Name   string           `xml:"methodName"`
+			Params []*ResponseParam `xml:"params>param"`
 		}{}
 		body, err := ioutil.ReadAll(r.Body)
 		assert.NoError(t, err, "test server: read body")

codec.go

@@ -11,6 +11,8 @@ import (
 	"sync"
 )
 
+// Codec implements methods required by rpc.ClientCodec
+// In this implementation Codec is the one performing actual RPC requests with http.Client.
 type Codec struct {
 	endpoint   *url.URL
 	httpClient *http.Client
@@ -20,7 +22,9 @@ type Codec struct {
 	pending map[uint64]*rpcCall
 
 	// Current in-flight response
-	response *decodableResponse
+	response *Response
+	encoder  Encoder
+	decoder  Decoder
 
 	// presents completed requests by sequence ID
 	ready chan uint64
@@ -32,11 +36,16 @@ type rpcCall struct {
 	httpResponse  *http.Response
 }
 
+// NewCodec creates a new Codec bound to provided endpoint.
+// Provided client will be used to perform RPC requests.
 func NewCodec(endpoint *url.URL, httpClient *http.Client) *Codec {
 	return &Codec{
 		endpoint:   endpoint,
 		httpClient: httpClient,
 
+		encoder: &StdEncoder{},
+		decoder: &StdDecoder{},
+
 		pending:  make(map[uint64]*rpcCall),
 		response: nil,
 		ready:    make(chan uint64),
@@ -46,7 +55,7 @@ func NewCodec(endpoint *url.URL, httpClient *http.Client) *Codec {
 func (c *Codec) WriteRequest(req *rpc.Request, args interface{}) error {
 
 	bodyBuffer := new(bytes.Buffer)
-	err := EncodeMethodCall(bodyBuffer, req.ServiceMethod, args)
+	err := c.encoder.Encode(bodyBuffer, req.ServiceMethod, args)
 	if err != nil {
 		return err
 	}
@@ -105,14 +114,14 @@ func (c *Codec) ReadResponseHeader(resp *rpc.Response) error {
 		return nil
 	}
 
-	decodableResponse, err := newDecodableResponse(body)
+	decodableResponse, err := NewResponse(body)
 	if err != nil {
 		resp.Error = err.Error()
 		return nil
 	}
 
 	// Return response Fault already a this stage
-	if err := decodableResponse.Fault(); err != nil {
+	if err := c.decoder.DecodeFault(decodableResponse); err != nil {
 		resp.Error = err.Error()
 		return nil
 	}
@@ -131,7 +140,7 @@ func (c *Codec) ReadResponseBody(v interface{}) error {
 		return errors.New("no in-flight response found")
 	}
 
-	return c.response.Decode(v)
+	return c.decoder.Decode(c.response, v)
 }
 
 func (c *Codec) Close() error {

codec_response.go

@@ -2,7 +2,6 @@ package xmlrpc
 
 import (
 	"encoding/base64"
-	"encoding/xml"
 	"fmt"
 	"reflect"
 	"strconv"
@@ -14,81 +13,59 @@ const (
 	errFormatInvalidFieldType = "invalid field type: expected '%s', got '%s'"
 )
 
-type respWrapper struct {
-	Params []respParam `xml:"params>param"`
-	Fault  *respFault  `xml:"fault,omitempty"`
+// Decoder implementations provide mechanisms for parsing of XML-RPC responses to native data-types.
+type Decoder interface {
+	DecodeRaw(body []byte, v interface{}) error
+	Decode(response *Response, v interface{}) error
+	DecodeFault(response *Response) *Fault
 }
 
-type respParam struct {
-	Value respValue `xml:"value"`
-}
-
-type respValue struct {
-	Array    []*respValue        `xml:"array>data>value"`
-	Struct   []*respStructMember `xml:"struct>member"`
-	String   string              `xml:"string"`
-	Int      string              `xml:"int"`
-	Int4     string              `xml:"i4"`
-	Double   string              `xml:"double"`
-	Boolean  string              `xml:"boolean"`
-	DateTime string              `xml:"dateTime.iso8601"`
-	Base64   string              `xml:"base64"`
-
-	Raw string `xml:",innerxml"` // the value can be default string
-}
-
-type respStructMember struct {
-	Name  string    `xml:"name"`
-	Value respValue `xml:"value"`
-}
-
-type respFault struct {
-	Value respValue `xml:"value"`
-}
+// StdDecoder is the default implementation of the Decoder interface.
+type StdDecoder struct{}
 
-func DecodeResponse(body []byte, v interface{}) error {
+func (d *StdDecoder) DecodeRaw(body []byte, v interface{}) error {
 
-	wrapper, err := toRespWrapper(body)
+	response, err := NewResponse(body)
 	if err != nil {
 		return err
 	}
 
-	if wrapper.Fault != nil {
-		return decodeFault(wrapper.Fault)
-	}
-
-	return decodeWrapper(wrapper, v)
-}
-
-func toRespWrapper(body []byte) (*respWrapper, error) {
-	wrapper := &respWrapper{}
-	if err := xml.Unmarshal(body, wrapper); err != nil {
-		return nil, err
+	if response.Fault != nil {
+		return d.decodeFault(response.Fault)
 	}
 
-	return wrapper, nil
+	return d.Decode(response, v)
 }
 
-func decodeWrapper(wrapper *respWrapper, v interface{}) error {
+func (d *StdDecoder) Decode(response *Response, v interface{}) error {
 
 	// Validate that v has same number of public fields as response params
-	if err := fieldsMustEqual(v, len(wrapper.Params)); err != nil {
+	if err := fieldsMustEqual(v, len(response.Params)); err != nil {
 		return err
 	}
 
 	vElem := reflect.Indirect(reflect.ValueOf(v))
-	for i, param := range wrapper.Params {
+	for i, param := range response.Params {
 		field := vElem.Field(i)
 
-		if err := decodeValue(&param.Value, &field); err != nil {
+		if err := d.decodeValue(&param.Value, &field); err != nil {
 			return err
 		}
 	}
 
 	return nil
 }
 
-func decodeFault(fault *respFault) *Fault {
+func (d *StdDecoder) DecodeFault(response *Response) *Fault {
+
+	if response.Fault == nil {
+		return nil
+	}
+
+	return d.decodeFault(response.Fault)
+}
+
+func (d *StdDecoder) decodeFault(fault *ResponseFault) *Fault {
 
 	f := &Fault{}
 	for _, m := range fault.Value.Struct {
@@ -107,7 +84,7 @@ func decodeFault(fault *respFault) *Fault {
 	return f
 }
 
-func decodeValue(value *respValue, field *reflect.Value) error {
+func (d *StdDecoder) decodeValue(value *ResponseValue, field *reflect.Value) error {
 
 	var val interface{}
 	var err error
@@ -124,16 +101,16 @@ func decodeValue(value *respValue, field *reflect.Value) error {
 		val, err = strconv.ParseFloat(value.Double, 64)
 
 	case value.Boolean != "":
-		val, err = decodeBoolean(value.Boolean)
+		val, err = d.decodeBoolean(value.Boolean)
 
 	case value.String != "":
 		val, err = value.String, nil
 
 	case value.Base64 != "":
-		val, err = decodeBase64(value.Base64)
+		val, err = d.decodeBase64(value.Base64)
 
 	case value.DateTime != "":
-		val, err = decodeDateTime(value.DateTime)
+		val, err = d.decodeDateTime(value.DateTime)
 
 	// Array decoding
 	case len(value.Array) > 0:
@@ -145,7 +122,7 @@ func decodeValue(value *respValue, field *reflect.Value) error {
 		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 := 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)
 			}
 		}
@@ -168,7 +145,7 @@ func decodeValue(value *respValue, field *reflect.Value) error {
 				return fmt.Errorf("cannot find field '%s' on struct", fName)
 			}
 
-			if err := 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)
 			}
 		}
@@ -188,7 +165,7 @@ func decodeValue(value *respValue, field *reflect.Value) error {
 	return nil
 }
 
-func decodeBoolean(value string) (bool, error) {
+func (d *StdDecoder) decodeBoolean(value string) (bool, error) {
 
 	switch value {
 	case "1", "true", "TRUE", "True":
@@ -199,12 +176,12 @@ func decodeBoolean(value string) (bool, error) {
 	return false, fmt.Errorf("unrecognized value '%s' for boolean", value)
 }
 
-func decodeBase64(value string) ([]byte, error) {
+func (d *StdDecoder) decodeBase64(value string) ([]byte, error) {
 
 	return base64.StdEncoding.DecodeString(value)
 }
 
-func decodeDateTime(value string) (time.Time, error) {
+func (d *StdDecoder) decodeDateTime(value string) (time.Time, error) {
 
 	return time.Parse(time.RFC3339, value)
 }

decode.go

@@ -0,0 +1,47 @@
+package xmlrpc
+
+import "encoding/xml"
+
+// Response is the basic parsed object of the XML-RPC response body.
+// While it's not convenient to use this object directly - it contains all the information needed to unmarshal into other data-types.
+type Response struct {
+	Params []ResponseParam `xml:"params>param"`
+	Fault  *ResponseFault  `xml:"fault,omitempty"`
+}
+
+// NewResponse creates a Response object from XML body.
+// It relies on XML Unmarshaler and if it fails - error is returned.
+func NewResponse(body []byte) (*Response, error) {
+
+	response := &Response{}
+	if err := xml.Unmarshal(body, response); err != nil {
+		return nil, err
+	}
+
+	return response, nil
+}
+
+type ResponseParam struct {
+	Value ResponseValue `xml:"value"`
+}
+
+type ResponseValue struct {
+	Array    []*ResponseValue        `xml:"array>data>value"`
+	Struct   []*ResponseStructMember `xml:"struct>member"`
+	String   string                  `xml:"string"`
+	Int      string                  `xml:"int"`
+	Int4     string                  `xml:"i4"`
+	Double   string                  `xml:"double"`
+	Boolean  string                  `xml:"boolean"`
+	DateTime string                  `xml:"dateTime.iso8601"`
+	Base64   string                  `xml:"base64"`
+}
+
+type ResponseStructMember struct {
+	Name  string        `xml:"name"`
+	Value ResponseValue `xml:"value"`
+}
+
+type ResponseFault struct {
+	Value ResponseValue `xml:"value"`
+}