From 93281db678e5839dddca989cd58ae72849062351 Mon Sep 17 00:00:00 2001 From: Alexej Kubarev Date: Sat, 17 Feb 2024 20:11:57 +0100 Subject: [PATCH 1/4] decoder: Handling of empty value types --- client_test.go | 2 +- decode.go | 76 ++++++++++----- decode_response.go | 21 ++-- decode_test.go | 113 ++++++++++++++++++---- testdata/response_struct_empty_values.xml | 61 ++++++++++++ 5 files changed, 223 insertions(+), 50 deletions(-) create mode 100644 testdata/response_struct_empty_values.xml 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 + + + + + + + + + + + From fadb80df5d315743ab9e0b2f959a44c347adbed3 Mon Sep 17 00:00:00 2001 From: Alexej Kubarev Date: Sat, 17 Feb 2024 20:12:40 +0100 Subject: [PATCH 2/4] readme: Update to reflect handling of empty data types --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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. From 332b961a613fdae266457e56a3e2c885e3bb95ef Mon Sep 17 00:00:00 2001 From: Alexej Kubarev Date: Sat, 17 Feb 2024 20:14:55 +0100 Subject: [PATCH 3/4] changelog: Upcomming version changelog update --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) 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: From 6fd64e7bb0a074ad460c0de5a5fa1ee89c2eaa28 Mon Sep 17 00:00:00 2001 From: Alexej Kubarev Date: Sat, 17 Feb 2024 20:23:24 +0100 Subject: [PATCH 4/4] build(docker): Updating builder image to go1.22 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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