From 5c0347e3874f883d95e1fea7475159527a3a0219 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Thu, 7 Jun 2018 21:45:22 -0700 Subject: [PATCH 1/2] service/dynamodb/dynamodbattribute: Retain backwards compatibility with ConvertTo data --- service/dynamodb/dynamodbattribute/decode.go | 11 +++++++++ .../dynamodb/dynamodbattribute/decode_test.go | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/service/dynamodb/dynamodbattribute/decode.go b/service/dynamodb/dynamodbattribute/decode.go index e02497568e9..f6cbbbc63d1 100644 --- a/service/dynamodb/dynamodbattribute/decode.go +++ b/service/dynamodb/dynamodbattribute/decode.go @@ -1,6 +1,7 @@ package dynamodbattribute import ( + "encoding/base64" "fmt" "reflect" "strconv" @@ -538,6 +539,16 @@ func (d *Decoder) decodeString(s *string, v reflect.Value, fieldTag tag) error { switch v.Kind() { case reflect.String: v.SetString(*s) + case reflect.Slice: + // To maintain backwards compatibility with the ConvertFrom family of methods + // which converted []byte into base64-encoded strings if the input was typed + if v.Type() == byteSliceType { + decoded, err := base64.StdEncoding.DecodeString(*s) + if err != nil { + return &UnmarshalTypeError{Value: "string", Type: v.Type()} + } + v.SetBytes(decoded) + } case reflect.Interface: // Ensure type aliasing is handled properly v.Set(reflect.ValueOf(*s).Convert(v.Type())) diff --git a/service/dynamodb/dynamodbattribute/decode_test.go b/service/dynamodb/dynamodbattribute/decode_test.go index 1313064af6f..0f73a68b07c 100644 --- a/service/dynamodb/dynamodbattribute/decode_test.go +++ b/service/dynamodb/dynamodbattribute/decode_test.go @@ -234,6 +234,30 @@ func TestUnmarshalListError(t *testing.T) { } } +func TestUnmarshalConvertToData(t *testing.T) { + type T struct { + Int int + Str string + ByteSlice []byte + StrSlice []string + } + + exp := T{ + Int: 42, + Str: "foo", + ByteSlice: []byte{42, 97, 83}, + StrSlice: []string{"cat", "dog"}, + } + av, err := ConvertToMap(exp) + if err != nil { + t.Fatalf("expect no error, got %v", err) + } + + var act T + err = UnmarshalMap(av, &act) + assertConvertTest(t, 0, act, exp, err, nil) +} + func TestUnmarshalMapShared(t *testing.T) { for i, c := range sharedMapTestCases { err := UnmarshalMap(c.in, c.actual) From fcd5a4094bd9267d5328f925cb1490c172498d3f Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Fri, 8 Jun 2018 13:06:14 -0700 Subject: [PATCH 2/2] Switch to UnmarshalError and update docs --- service/dynamodb/dynamodbattribute/decode.go | 2 +- service/dynamodb/dynamodbattribute/doc.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/service/dynamodb/dynamodbattribute/decode.go b/service/dynamodb/dynamodbattribute/decode.go index f6cbbbc63d1..41b67b3ae02 100644 --- a/service/dynamodb/dynamodbattribute/decode.go +++ b/service/dynamodb/dynamodbattribute/decode.go @@ -545,7 +545,7 @@ func (d *Decoder) decodeString(s *string, v reflect.Value, fieldTag tag) error { if v.Type() == byteSliceType { decoded, err := base64.StdEncoding.DecodeString(*s) if err != nil { - return &UnmarshalTypeError{Value: "string", Type: v.Type()} + return &UnmarshalError{Err: err, Value: "string", Type: v.Type()} } v.SetBytes(decoded) } diff --git a/service/dynamodb/dynamodbattribute/doc.go b/service/dynamodb/dynamodbattribute/doc.go index 7a51ac07616..b83a29c951f 100644 --- a/service/dynamodb/dynamodbattribute/doc.go +++ b/service/dynamodb/dynamodbattribute/doc.go @@ -81,7 +81,7 @@ // The ConvertTo, ConvertToList, ConvertToMap, ConvertFrom, ConvertFromMap // and ConvertFromList methods have been deprecated. The Marshal and Unmarshal // functions should be used instead. The ConvertTo|From marshallers do not -// support BinarySet, NumberSet, nor StringSets, and will incorrect marshal +// support BinarySet, NumberSet, nor StringSets, and will incorrectly marshal // binary data fields in structs as base64 strings. // // The Marshal and Unmarshal functions correct this behavior, and removes @@ -91,5 +91,11 @@ // replaced with have been replaced with dynamodbattribute.Marshaler and // dynamodbattribute.Unmarshaler interfaces. // +// The Unmarshal functions are backwards compatible with data marshalled by +// ConvertTo*, but the reverse is not true: objects marshalled using Marshal +// are not necessarily usable by ConvertFrom*. This backward compatibility is +// intended to assist with incremental upgrading of data following a switch +// away from the Convert* family of functions. +// // `time.Time` is marshaled as RFC3339 format. package dynamodbattribute