Reworking datacodec package (#428)

* Fixed data codec. Now it accepts only []byte which are already decoded from base64
* Improved documentation to explain the behaviour of SetData() and Data()
* Improved tests for base64 encoding case
* Settled the xml test
* Renamed DataBinary to DataBase64

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

v2/event/datacodec/codec.go

@@ -9,10 +9,10 @@ import (
 	"github.com/cloudevents/sdk-go/v2/event/datacodec/xml"
 )
 
-// Decoder is the expected function signature for decoding `in` to `out`. What
-// `in` is could be decoder dependent. For example, `in` could be bytes, or a
-// base64 string.
-type Decoder func(ctx context.Context, in, out interface{}) error
+// Decoder is the expected function signature for decoding `in` to `out`.
+// If Event sent the payload as base64, Decoder assumes that `in` is the
+// decoded base64 byte array.
+type Decoder func(ctx context.Context, in []byte, out interface{}) error
 
 // Encoder is the expected function signature for encoding `in` to bytes.
 // Returns an error if the encoder has an issue encoding `in`.
@@ -55,7 +55,7 @@ func AddEncoder(contentType string, fn Encoder) {
 // Decode looks up and invokes the decoder registered for the given content
 // type. An error is returned if no decoder is registered for the given
 // content type.
-func Decode(ctx context.Context, contentType string, in, out interface{}) error {
+func Decode(ctx context.Context, contentType string, in []byte, out interface{}) error {
 	if fn, ok := decoder[contentType]; ok {
 		return fn(ctx, in, out)
 	}

v2/event/datacodec/codec_observed.go

@@ -26,7 +26,7 @@ func SetObservedCodecs() {
 }
 
 // DecodeObserved calls Decode and records the result.
-func DecodeObserved(ctx context.Context, contentType string, in, out interface{}) error {
+func DecodeObserved(ctx context.Context, contentType string, in []byte, out interface{}) error {
 	_, r := observability.NewReporter(ctx, reportDecode)
 	err := Decode(ctx, contentType, in, out)
 	if err != nil {

v2/event/datacodec/codec_test.go

@@ -22,7 +22,7 @@ func TestCodecDecode(t *testing.T) {
 	testCases := map[string]struct {
 		contentType string
 		decoder     datacodec.Decoder
-		in          interface{}
+		in          []byte
 		want        interface{}
 		wantErr     string
 	}{
@@ -34,7 +34,7 @@ func TestCodecDecode(t *testing.T) {
 
 		"text/plain": {
 			contentType: "text/plain",
-			in:          "hello😀",
+			in:          []byte("hello😀"),
 			want:        strptr("hello😀"), // Test  unicode outiside UTF-8
 		},
 		"application/json": {
@@ -54,15 +54,13 @@ func TestCodecDecode(t *testing.T) {
 		"custom content type": {
 			contentType: "unit/testing",
 			in:          []byte("Hello, Testing"),
-			decoder: func(ctx context.Context, in, out interface{}) error {
-				if b, ok := in.([]byte); ok {
-					if s, k := out.(*map[string]string); k {
-						if (*s) == nil {
-							(*s) = make(map[string]string)
-						}
-						(*s)["upper"] = strings.ToUpper(string(b))
-						(*s)["lower"] = strings.ToLower(string(b))
+			decoder: func(ctx context.Context, in []byte, out interface{}) error {
+				if s, k := out.(*map[string]string); k {
+					if (*s) == nil {
+						(*s) = make(map[string]string)
 					}
+					(*s)["upper"] = strings.ToUpper(string(in))
+					(*s)["lower"] = strings.ToLower(string(in))
 				}
 				return nil
 			},
@@ -74,7 +72,7 @@ func TestCodecDecode(t *testing.T) {
 		"custom content type error": {
 			contentType: "unit/testing",
 			in:          []byte("Hello, Testing"),
-			decoder: func(ctx context.Context, in, out interface{}) error {
+			decoder: func(ctx context.Context, in []byte, out interface{}) error {
 				return fmt.Errorf("expecting unit test error")
 			},
 			wantErr: "expecting unit test error",
@@ -221,44 +219,3 @@ func TestCodecEncode(t *testing.T) {
 func TestSetObservedCodecs(t *testing.T) {
 	datacodec.SetObservedCodecs()
 }
-
-//
-//func TestCodecRoundTrip(t *testing.T) {
-//	testCases := map[string]struct {
-//		contentType string
-//		decoder     datacodec.Decoder
-//		encoder     datacodec.Encoder
-//		in          interface{}
-//		want        interface{}
-//		wantErr     string
-//	}{
-//		"empty": {},
-//	}
-//	for n, tc := range testCases {
-//		t.Run(n, func(t *testing.T) {
-//
-//			if tc.decoder != nil {
-//				datacodec.AddDecoder(tc.contentType, tc.decoder)
-//			}
-//
-//
-//			// TODO
-//			got, _ := types.Allocate(tc.want)
-//
-//			err := datacodec.Decode(tc.contentType, tc.in, got)
-//
-//			if tc.wantErr != "" || err != nil {
-//				if diff := cmp.Diff(tc.wantErr, err.Error()); diff != "" {
-//					t.Errorf("unexpected error (-want, +got) = %v", diff)
-//				}
-//				return
-//			}
-//
-//			if tc.want != nil {
-//				if diff := cmp.Diff(tc.want, got); diff != "" {
-//					t.Errorf("unexpected data (-want, +got) = %v", diff)
-//				}
-//			}
-//		})
-//	}
-//}

v2/event/datacodec/json/data.go

@@ -5,43 +5,21 @@ import (
 	"encoding/json"
 	"fmt"
 	"reflect"
-	"strconv"
 )
 
-// Decode takes `in` as []byte, or base64 string, normalizes in to unquoted and
-// base64 decoded []byte if required, and then attempts to use json.Unmarshal
-// to convert those bytes to `out`. Returns and error if this process fails.
-func Decode(ctx context.Context, in, out interface{}) error {
+// Decode takes `in` as []byte.
+// If Event sent the payload as base64, Decoder assumes that `in` is the
+// decoded base64 byte array.
+func Decode(ctx context.Context, in []byte, out interface{}) error {
 	if in == nil {
 		return nil
 	}
 	if out == nil {
 		return fmt.Errorf("out is nil")
 	}
 
-	b, ok := in.([]byte) // TODO: I think there is fancy marshaling happening here. Fix with reflection?
-	if !ok {
-		var err error
-		b, err = json.Marshal(in)
-		if err != nil {
-			return fmt.Errorf("[json] failed to marshal in: %s", err.Error())
-		}
-	}
-
-	// TODO: the spec says json could be just data... At the moment we expect wrapped.
-	if len(b) > 1 && (b[0] == byte('"') || (b[0] == byte('\\') && b[1] == byte('"'))) {
-		s, err := strconv.Unquote(string(b))
-		if err != nil {
-			return fmt.Errorf("[json] failed to unquote in: %s", err.Error())
-		}
-		if len(s) > 0 && (s[0] == '{' || s[0] == '[') {
-			// looks like json, use it
-			b = []byte(s)
-		}
-	}
-
-	if err := json.Unmarshal(b, out); err != nil {
-		return fmt.Errorf("[json] found bytes \"%s\", but failed to unmarshal: %s", string(b), err.Error())
+	if err := json.Unmarshal(in, out); err != nil {
+		return fmt.Errorf("[json] found bytes \"%s\", but failed to unmarshal: %s", string(in), err.Error())
 	}
 	return nil
 }

v2/event/datacodec/json/data_observed.go

@@ -6,7 +6,7 @@ import (
 )
 
 // DecodeObserved calls Decode and records the results.
-func DecodeObserved(ctx context.Context, in, out interface{}) error {
+func DecodeObserved(ctx context.Context, in []byte, out interface{}) error {
 	_, r := observability.NewReporter(ctx, reportDecode)
 	err := Decode(ctx, in, out)
 	if err != nil {

v2/event/datacodec/json/data_test.go

@@ -30,33 +30,33 @@ func TestCodecDecode(t *testing.T) {
 	now := time.Now()
 
 	testCases := map[string]struct {
-		in      interface{}
+		in      []byte
 		want    interface{}
 		wantErr string
 	}{
 		"empty": {},
 		"out nil": {
-			in:      "not nil",
+			in:      []byte{},
 			wantErr: "out is nil",
 		},
-		"not a []byte": {
-			in: "something that is not a map",
+		"wrong unmarshalling receiver": {
+			in: []byte("\"something that is not a map\""),
 			want: &map[string]string{
 				"an": "error",
 			},
 			wantErr: `[json] found bytes ""something that is not a map"", but failed to unmarshal: json: cannot unmarshal string into Go value of type map[string]string`,
 		},
-		"BadMarshal": {
-			in:      BadMarshal{},
-			want:    &BadMarshal{},
-			wantErr: "[json] failed to marshal in: json: error calling MarshalJSON for type json_test.BadMarshal: BadMashal Error",
+		"wrong string": {
+			in:      []byte("a non json string"),
+			want:    "a non json string",
+			wantErr: `[json] found bytes "a non json string", but failed to unmarshal: invalid character 'a' looking for beginning of value`,
 		},
 		"Bad Quotes": {
-			in: []byte{'\\', '"'},
+			in: []byte("\""),
 			want: &map[string]string{
 				"an": "error",
 			},
-			wantErr: "[json] failed to unquote in: invalid syntax",
+			wantErr: `[json] found bytes """, but failed to unmarshal: unexpected end of JSON input`,
 		},
 		"simple": {
 			in: []byte(`{"a":"apple","b":"banana"}`),
@@ -76,13 +76,6 @@ func TestCodecDecode(t *testing.T) {
 				"banana",
 			},
 		},
-		"simple quoted array": {
-			in: []byte(`"[\"apple\",\"banana\"]"`),
-			want: &[]string{
-				"apple",
-				"banana",
-			},
-		},
 		"complex filled": {
 			in: func() []byte {
 				data := &DataExample{
@@ -113,14 +106,6 @@ func TestCodecDecode(t *testing.T) {
 				AnArray: []string{"Anne", "Bob", "Chad"},
 			},
 		},
-		"object in": {
-			in: &DataExample{
-				AnInt: 42,
-			},
-			want: &DataExample{
-				AnInt: 42,
-			},
-		},
 	}
 	for n, tc := range testCases {
 		t.Run(n, func(t *testing.T) {

v2/event/datacodec/text/data.go

@@ -6,21 +6,12 @@ import (
 	"fmt"
 )
 
-func Decode(_ context.Context, in, out interface{}) error {
+func Decode(_ context.Context, in []byte, out interface{}) error {
 	p, _ := out.(*string)
 	if p == nil {
 		return fmt.Errorf("text.Decode out: want *string, got %T", out)
 	}
-	switch s := in.(type) {
-	case string:
-		*p = s
-	case []byte:
-		*p = string(s)
-	case nil: // treat nil like []byte{}
-		*p = ""
-	default:
-		return fmt.Errorf("text.Decode in: want []byte or string, got %T", in)
-	}
+	*p = string(in)
 	return nil
 }
 

v2/event/datacodec/text/data_observed.go

@@ -6,7 +6,7 @@ import (
 )
 
 // DecodeObserved calls Decode and records the results.
-func DecodeObserved(ctx context.Context, in, out interface{}) error {
+func DecodeObserved(ctx context.Context, in []byte, out interface{}) error {
 	_, r := observability.NewReporter(ctx, reportDecode)
 	err := Decode(ctx, in, out)
 	if err != nil {

v2/event/datacodec/text/data_observed_test.go

@@ -27,7 +27,7 @@ func TestEncodeObserved(t *testing.T) {
 func TestDecodeObserved(t *testing.T) {
 	assert := assert.New(t)
 	var s string
-	assert.NoError(text.DecodeObserved(ctx, "hello", &s))
+	assert.NoError(text.DecodeObserved(ctx, []byte("hello"), &s))
 	assert.Equal("hello", s)
 	assert.NoError(text.DecodeObserved(ctx, []byte("bye"), &s))
 	assert.Equal("bye", s)
@@ -36,8 +36,4 @@ func TestDecodeObserved(t *testing.T) {
 	s = "xxx"
 	assert.NoError(text.DecodeObserved(ctx, nil, &s))
 	assert.Equal("", s)
-
-	assert.EqualError(text.DecodeObserved(ctx, 123, &s), "text.Decode in: want []byte or string, got int")
-	assert.EqualError(text.DecodeObserved(ctx, "", nil), "text.Decode out: want *string, got <nil>")
-	assert.EqualError(text.DecodeObserved(ctx, "", 1), "text.Decode out: want *string, got int")
 }

v2/event/datacodec/text/data_test.go

@@ -30,17 +30,11 @@ func TestEncode(t *testing.T) {
 func TestDecode(t *testing.T) {
 	assert := assert.New(t)
 	var s string
-	assert.NoError(text.Decode(ctx, "hello", &s))
-	assert.Equal("hello", s)
 	assert.NoError(text.Decode(ctx, []byte("bye"), &s))
 	assert.Equal("bye", s)
 	assert.NoError(text.Decode(ctx, []byte{}, &s))
 	assert.Equal("", s)
 	s = "xxx"
 	assert.NoError(text.Decode(ctx, nil, &s))
 	assert.Equal("", s)
-
-	assert.EqualError(text.Decode(ctx, 123, &s), "text.Decode in: want []byte or string, got int")
-	assert.EqualError(text.Decode(ctx, "", nil), "text.Decode out: want *string, got <nil>")
-	assert.EqualError(text.Decode(ctx, "", 1), "text.Decode out: want *string, got int")
 }

v2/event/datacodec/xml/data.go

@@ -2,51 +2,20 @@ package xml
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/xml"
 	"fmt"
-	"strconv"
 )
 
-// Decode takes `in` as []byte, or base64 string, normalizes in to unquoted and
-// base64 decoded []byte if required, and then attempts to use xml.Unmarshal
-// to convert those bytes to `out`. Returns and error if this process fails.
-func Decode(ctx context.Context, in, out interface{}) error {
+// Decode takes `in` as []byte.
+// If Event sent the payload as base64, Decoder assumes that `in` is the
+// decoded base64 byte array.
+func Decode(ctx context.Context, in []byte, out interface{}) error {
 	if in == nil {
 		return nil
 	}
 
-	b, ok := in.([]byte)
-	if !ok {
-		var err error
-		b, err = xml.Marshal(in)
-		if err != nil {
-			return fmt.Errorf("[xml] failed to marshal in: %s", err.Error())
-		}
-	}
-
-	// If the message is encoded as a base64 block as a string, we need to
-	// decode that first before trying to unmarshal the bytes
-	if len(b) > 1 && (b[0] == byte('"') || (b[0] == byte('\\') && b[1] == byte('"'))) {
-		s, err := strconv.Unquote(string(b))
-		if err != nil {
-			return fmt.Errorf("[xml] failed to unquote quoted data: %s", err.Error())
-		}
-		if len(s) > 0 && s[0] == '<' {
-			// looks like xml, use it
-			b = []byte(s)
-		} else if len(s) > 0 {
-			// looks like base64, decode
-			bs, err := base64.StdEncoding.DecodeString(s)
-			if err != nil {
-				return fmt.Errorf("[xml] failed to decode base64 encoded string: %s", err.Error())
-			}
-			b = bs
-		}
-	}
-
-	if err := xml.Unmarshal(b, out); err != nil {
-		return fmt.Errorf("[xml] found bytes, but failed to unmarshal: %s %s", err.Error(), string(b))
+	if err := xml.Unmarshal(in, out); err != nil {
+		return fmt.Errorf("[xml] found bytes, but failed to unmarshal: %s %s", err.Error(), string(in))
 	}
 	return nil
 }

v2/event/datacodec/xml/data_observed.go

@@ -6,7 +6,7 @@ import (
 )
 
 // DecodeObserved calls Decode and records the result.
-func DecodeObserved(ctx context.Context, in, out interface{}) error {
+func DecodeObserved(ctx context.Context, in []byte, out interface{}) error {
 	_, r := observability.NewReporter(ctx, reportDecode)
 	err := Decode(ctx, in, out)
 	if err != nil {