diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bf6276..f09017c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.5.1 + +Bugfixes: +* Handling of empty values while decoding responses (#80). +Library will now properly handle empty values for ``, ``, ``, ``, ``, ``, `` and `` (with case of ``). +As `` may not have an empty list of `` elements as per specification. Similarly `` is considered invalid. + ## 0.5.0 Improvements: diff --git a/Dockerfile b/Dockerfile index 281b67f..85d490e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.21-alpine +FROM golang:1.22-alpine # Build dependencies RUN apk --no-cache update diff --git a/README.md b/README.md index 7072cec..0ae3490 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,23 @@ Response is decoded following similar rules to argument encoding. * Structs may contain pointers - they will be initialized if required. * Structs may be parsed as `map[string]any`, in case struct member names are not known at compile time. Map keys are enforced to `string` type. +#### Handling of Empty Values + +If XML-RPC response contains no value for well-known data-types, it will be decoded into the default "empty" values as per table below: + +| XML-RPC Value | Default Value | +|-------------------------|--------------| +| `` | `""` | +| ``, `` | `0` | +| `` | `false` | +| `` | `0.0` | +| `` | `time.Time{}` | +| `` | `nil` | +| `` | `nil` | + +As per XML-RPC specification, `` may not have an empty list of `` elements, thus no default "empty" value is defined for it. +Similarly, `` is considered invalid. + ### Field renaming XML-RPC specification does not necessarily specify any rules for struct's member names. Some services allow struct member names to include characters not compatible with standard Go field naming. diff --git a/client_test.go b/client_test.go index ff54467..68102e0 100644 --- a/client_test.go +++ b/client_test.go @@ -41,7 +41,7 @@ func TestClient_Call(t *testing.T) { require.Equal(t, 2, len(nameParts)) require.Equal(t, "my", nameParts[0], "test server: method should start with 'my.'") require.Equal(t, 1, len(m.Params)) - require.Equal(t, "12345", m.Params[0].Value.Int) + require.Equal(t, "12345", *m.Params[0].Value.Int) file := nameParts[1] _, _ = fmt.Fprintln(w, string(loadTestFile(t, fmt.Sprintf("response_%s.xml", file)))) diff --git a/decode.go b/decode.go index 02695c2..4ad0209 100644 --- a/decode.go +++ b/decode.go @@ -72,13 +72,17 @@ func (d *StdDecoder) decodeFault(fault *ResponseFault) *Fault { for _, m := range fault.Value.Struct { switch m.Name { case "faultCode": - if m.Value.Int != "" { - f.Code, _ = strconv.Atoi(m.Value.Int) + if m.Value.Int != nil { + f.Code, _ = strconv.Atoi(*m.Value.Int) + } else if m.Value.Int4 != nil { + f.Code, _ = strconv.Atoi(*m.Value.Int4) } else { - f.Code, _ = strconv.Atoi(m.Value.Int4) + f.Code = 0 // Unknown fault code } case "faultString": - f.String = m.Value.String + if m.Value.String != nil { + f.String = *m.Value.String + } } } @@ -92,36 +96,41 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field reflect.Value) erro var err error switch { - case value.Int != "": - val, err = strconv.Atoi(value.Int) + case value.Int != nil: + val, err = d.decodeInt(*value.Int) - case value.Int4 != "": - val, err = strconv.Atoi(value.Int4) + case value.Int4 != nil: + val, err = d.decodeInt(*value.Int4) - case value.Double != "": - val, err = strconv.ParseFloat(value.Double, float64BitSize) + case value.Double != nil: + val, err = d.decodeDouble(*value.Double) - case value.Boolean != "": - val, err = d.decodeBoolean(value.Boolean) + case value.Boolean != nil: + val, err = d.decodeBoolean(*value.Boolean) - case value.String != "": - val, err = value.String, nil + case value.String != nil: + val, err = *value.String, nil - case value.Base64 != "": - val, err = d.decodeBase64(value.Base64) + case value.Base64 != nil: + val, err = d.decodeBase64(*value.Base64) - case value.DateTime != "": - val, err = d.decodeDateTime(value.DateTime) + case value.DateTime != nil: + val, err = d.decodeDateTime(*value.DateTime) // Array decoding - case len(value.Array) > 0: - + case value.Array != nil: if field.Kind() != reflect.Slice { return fmt.Errorf(errFormatInvalidFieldType, reflect.Slice.String(), field.Kind().String()) } - slice := reflect.MakeSlice(reflect.TypeOf(field.Interface()), len(value.Array), len(value.Array)) - for i, v := range value.Array { + values := value.Array.Values + if len(values) == 0 { + val, err = nil, nil + break + } + + slice := reflect.MakeSlice(reflect.TypeOf(field.Interface()), len(values), len(values)) + for i, v := range values { item := slice.Index(i) if err := d.decodeValue(v, item); err != nil { return fmt.Errorf("failed decoding array item at index %d: %w", i, err) @@ -132,7 +141,6 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field reflect.Value) erro // Struct decoding case len(value.Struct) != 0: - fieldKind := field.Kind() if fieldKind != reflect.Struct && fieldKind != reflect.Map { return fmt.Errorf(errFormatInvalidFieldTypeOrType, reflect.Struct.String(), reflect.Map.String(), fieldKind.String()) @@ -203,22 +211,44 @@ func (d *StdDecoder) decodeValue(value *ResponseValue, field reflect.Value) erro return nil } +func (d *StdDecoder) decodeInt(value string) (int, error) { + if value == "" { + return 0, nil + } + return strconv.Atoi(value) +} + +func (d *StdDecoder) decodeDouble(value string) (float64, error) { + if value == "" { + return 0.0, nil + } + return strconv.ParseFloat(value, float64BitSize) +} + func (d *StdDecoder) decodeBoolean(value string) (bool, error) { switch value { case "1", "true", "TRUE", "True": return true, nil case "0", "false", "FALSE", "False": return false, nil + case "": + return false, nil default: return false, fmt.Errorf("unrecognized value '%s' for boolean", value) } } func (d *StdDecoder) decodeBase64(value string) ([]byte, error) { + if value == "" { + return nil, nil + } return base64.StdEncoding.DecodeString(value) } func (d *StdDecoder) decodeDateTime(value string) (time.Time, error) { + if value == "" { + return time.Time{}, nil + } return time.Parse(time.RFC3339, value) } diff --git a/decode_response.go b/decode_response.go index b9512c6..d448e7d 100644 --- a/decode_response.go +++ b/decode_response.go @@ -28,15 +28,15 @@ type ResponseParam struct { // ResponseValue encapsulates one of the data types for each parameter. // Only one field should be set. type ResponseValue struct { - Array []*ResponseValue `xml:"array>data>value"` + Array *ResponseArrayData `xml:"array>data"` 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"` + 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"` RawXML string `xml:",innerxml"` } @@ -47,6 +47,11 @@ type ResponseStructMember struct { Value ResponseValue `xml:"value"` } +// ResponseArrayData contains a list of array values +type ResponseArrayData struct { + Values []*ResponseValue `xml:"value"` +} + // ResponseFault wraps around failure type ResponseFault struct { Value ResponseValue `xml:"value"` diff --git a/decode_test.go b/decode_test.go index c618764..4f216ba 100644 --- a/decode_test.go +++ b/decode_test.go @@ -7,21 +7,20 @@ import ( "path/filepath" "reflect" "testing" + "time" "github.com/stretchr/testify/require" ) func TestStdDecoder_DecodeRaw(t *testing.T) { - tests := []struct { - name string + tests := map[string]struct { testFile string skipUnknown bool v interface{} expect interface{} err error }{ - { - name: "simple response", + "simple response": { testFile: "response_simple.xml", v: &struct { Param string @@ -35,8 +34,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { Int: 12345, }, }, - { - name: "array response", + "array response": { testFile: "response_array.xml", v: &struct { Ints []int @@ -49,8 +47,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { }, }, }, - { - name: "array response - mixed content", + "array response - mixed content": { testFile: "response_array_mixed.xml", v: &struct { Mixed []interface{} @@ -63,8 +60,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { }, }, }, - { - name: "array response - bad param", + "array response - bad param": { testFile: "response_array.xml", v: &struct { Ints string // <- This is unexpected type @@ -72,8 +68,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { expect: nil, err: fmt.Errorf(errFormatInvalidFieldType, "slice", "string"), }, - { - name: "struct response", + "struct response": { testFile: "response_struct.xml", v: &struct { Struct struct { @@ -108,8 +103,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { }, }, }, - { - name: "struct response - skip unknown", + "struct response - skip unknown": { testFile: "response_struct.xml", skipUnknown: true, v: &struct { @@ -141,8 +135,7 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { }, }, }, - { - name: "struct response - bad param", + "struct response - bad param": { testFile: "response_struct.xml", v: &struct { Struct string // <- This is unexpected type @@ -150,10 +143,94 @@ func TestStdDecoder_DecodeRaw(t *testing.T) { expect: nil, err: fmt.Errorf(errFormatInvalidFieldTypeOrType, "struct", "map", "string"), }, + "struct response empty values (explicit)": { + testFile: "response_struct_empty_values.xml", + v: &struct { + Struct struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + } + }{}, + expect: &struct { + Struct struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + } + }{ + Struct: struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + }{ + EmptyString: ``, + EmptyInt: 0, + EmptyInt4: 0, + EmptyDouble: 0, + EmptyDate: time.Time{}, + EmptyBase64: nil, + EmptyArray: nil, + }, + }, + }, + "struct response empty values (implicit)": { + testFile: "response_struct_empty_values.xml", + v: &struct { + Struct struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + } + }{}, + expect: &struct { + Struct struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + } + }{ + Struct: struct { + EmptyString string + EmptyInt int + EmptyInt4 int + EmptyDouble int + EmptyBoolean bool + EmptyDate time.Time + EmptyBase64 []byte + EmptyArray []any + }{}, + }, + }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for tName, tt := range tests { + t.Run(tName, func(t *testing.T) { dec := &StdDecoder{} dec.skipUnknownFields = tt.skipUnknown err := dec.DecodeRaw(loadTestFile(t, tt.testFile), tt.v) diff --git a/testdata/response_struct_empty_values.xml b/testdata/response_struct_empty_values.xml new file mode 100644 index 0000000..bf40a73 --- /dev/null +++ b/testdata/response_struct_empty_values.xml @@ -0,0 +1,61 @@ + + + + + + + + EmptyString + + + + + + EmptyInt + + + + + + EmptyInt4 + + + + + + EmptyDouble + + + + + + EmptyBoolean + + + + + + EmptyDate + + + + + + EmptyBase64 + + + + + + EmptyArray + + + + + + + + + + +