Improved proto conversion quality and tests

observIQ · Dec 16, 2021 · 57c40d7 · 57c40d7
1 parent 3140581
commit 57c40d7
Show file tree

Hide file tree

Showing 3 changed files with 187 additions and 73 deletions.
diff --git a/operator/builtin/output/googlecloud/entry.go b/operator/builtin/output/googlecloud/entry.go
@@ -167,7 +167,7 @@ func (g *GoogleEntryBuilder) setPayload(entry *entry.Entry, logEntry *logging.Lo
 		logEntry.Payload = &logging.LogEntry_TextPayload{TextPayload: string(value)}
 		return nil
 	case map[string]interface{}:
-		structValue, err := toProto(value)
+		structValue, err := convertToProto(value)
 		if err != nil {
 			return fmt.Errorf("failed to convert record of type map[string]interface: %w", err)
 		}

diff --git a/operator/builtin/output/googlecloud/proto.go b/operator/builtin/output/googlecloud/proto.go
@@ -8,8 +8,8 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
-// toProto converts a value to a protobuf equivalent
-func toProto(v interface{}) (*structpb.Value, error) {
+// convertToProto converts a value to a protobuf equivalent
+func convertToProto(v interface{}) (*structpb.Value, error) {
 	switch v := v.(type) {
 	case nil:
 		return structpb.NewNullValue(), nil
@@ -40,91 +40,131 @@ func toProto(v interface{}) (*structpb.Value, error) {
 	case float64:
 		return structpb.NewNumberValue(v), nil
 	case string:
-		return toProtoString(v)
+		return convertStringToProto(v)
 	case []byte:
-		s := base64.StdEncoding.EncodeToString(v)
-		return structpb.NewStringValue(s), nil
+		return convertBytesToProto(v)
 	case map[string]interface{}:
-		v2, err := toProtoStruct(v)
-		if err != nil {
-			return nil, err
-		}
-		return structpb.NewStructValue(v2), nil
+		return convertMapToProto(v)
 	case map[string]string:
-		fields := map[string]*structpb.Value{}
-		for key, value := range v {
-			fields[key] = structpb.NewStringValue(value)
-		}
-		return structpb.NewStructValue(&structpb.Struct{
-			Fields: fields,
-		}), nil
+		return convertStringMapToProto(v)
 	case map[string]map[string]string:
-		fields := map[string]*structpb.Value{}
-		for key, value := range v {
-			proto, err := toProto(value)
-			if err != nil {
-				return nil, err
-			}
-
-			fields[key] = proto
+		return convertEmbeddedMapToProto(v)
+	case []interface{}:
+		return convertListToProto(v)
+	case []string:
+		return convertStringListToProto(v)
+	default:
+		return nil, fmt.Errorf("invalid type for proto conversion: %T", v)
+	}
+}
+
+// convertStringToProto converts a string to a proto string
+func convertStringToProto(value string) (*structpb.Value, error) {
+	if !utf8.ValidString(value) {
+		return nil, fmt.Errorf("invalid UTF-8 in string: %q", value)
+	}
+	return structpb.NewStringValue(value), nil
+}
+
+// convertBytesToProto converts a byte array to a protobuf equivalent
+func convertBytesToProto(bytes []byte) (*structpb.Value, error) {
+	s := base64.StdEncoding.EncodeToString(bytes)
+	return structpb.NewStringValue(s), nil
+}
+
+// convertMapToProto converts a map to a protobuf equivalent
+func convertMapToProto(mapValue map[string]interface{}) (*structpb.Value, error) {
+	fields := make(map[string]*structpb.Value, len(mapValue))
+
+	for key, value := range mapValue {
+		if !utf8.ValidString(key) {
+			return nil, fmt.Errorf("invalid UTF-8 in key: %q", key)
 		}
 
-		return structpb.NewStructValue(&structpb.Struct{
-			Fields: fields,
-		}), nil
-	case []interface{}:
-		v2, err := toProtoList(v)
+		protoValue, err := convertToProto(value)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to convert value for key %s to proto: %w", key, err)
 		}
-		return structpb.NewListValue(v2), nil
-	case []string:
-		values := []*structpb.Value{}
-		for _, str := range v {
-			values = append(values, structpb.NewStringValue(str))
+
+		fields[key] = protoValue
+	}
+
+	protoStruct := &structpb.Struct{Fields: fields}
+	return structpb.NewStructValue(protoStruct), nil
+}
+
+// convertStringMapToProto converts a string map to a protobuf equivalent
+func convertStringMapToProto(mapValue map[string]string) (*structpb.Value, error) {
+	fields := make(map[string]*structpb.Value, len(mapValue))
+
+	for key, value := range mapValue {
+		if !utf8.ValidString(key) {
+			return nil, fmt.Errorf("invalid UTF-8 in key: %q", key)
 		}
 
-		return structpb.NewListValue(&structpb.ListValue{
-			Values: values,
-		}), nil
-	default:
-		return nil, fmt.Errorf("invalid type: %T", v)
+		protoValue, err := convertStringToProto(value)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert value for key %s to proto: %w", key, err)
+		}
+
+		fields[key] = protoValue
 	}
+
+	protoStruct := &structpb.Struct{Fields: fields}
+	return structpb.NewStructValue(protoStruct), nil
 }
 
-// toProtoStruct converts a map to a protobuf equivalent
-func toProtoStruct(v map[string]interface{}) (*structpb.Struct, error) {
-	x := &structpb.Struct{Fields: make(map[string]*structpb.Value, len(v))}
-	for k, v := range v {
-		if !utf8.ValidString(k) {
-			return nil, fmt.Errorf("invalid UTF-8 in string: %q", k)
+// convertEmbeddedMapToProto converts an embedded string map to a protobuf equivalent
+func convertEmbeddedMapToProto(mapValue map[string]map[string]string) (*structpb.Value, error) {
+	fields := make(map[string]*structpb.Value, len(mapValue))
+
+	for key, value := range mapValue {
+		if !utf8.ValidString(key) {
+			return nil, fmt.Errorf("invalid UTF-8 in key: %q", key)
 		}
-		var err error
-		x.Fields[k], err = toProto(v)
+
+		protoValue, err := convertToProto(value)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to convert embedded value for key %s to proto: %w", key, err)
 		}
+
+		fields[key] = protoValue
 	}
-	return x, nil
+
+	protoStruct := &structpb.Struct{Fields: fields}
+	return structpb.NewStructValue(protoStruct), nil
 }
 
-// toProtoList converts a slice of interface a protobuf equivalent
-func toProtoList(v []interface{}) (*structpb.ListValue, error) {
-	x := &structpb.ListValue{Values: make([]*structpb.Value, len(v))}
-	for i, v := range v {
-		var err error
-		x.Values[i], err = toProto(v)
+// convertListToProto converts a generic list to a protobuf equivalent
+func convertListToProto(list []interface{}) (*structpb.Value, error) {
+	values := make([]*structpb.Value, len(list))
+
+	for i, value := range list {
+		protoValue, err := convertToProto(value)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to convert index %d of list to proto: %w", i, err)
 		}
+
+		values[i] = protoValue
 	}
-	return x, nil
+
+	protoList := &structpb.ListValue{Values: values}
+	return structpb.NewListValue(protoList), nil
 }
 
-// toProtoString converts a string to a proto string
-func toProtoString(v string) (*structpb.Value, error) {
-	if !utf8.ValidString(v) {
-		return nil, fmt.Errorf("invalid UTF-8 in string: %q", v)
+// convertStringListToProto converts a string list to a protobuf equivalent
+func convertStringListToProto(list []string) (*structpb.Value, error) {
+	values := make([]*structpb.Value, len(list))
+
+	for i, value := range list {
+		protoValue, err := convertStringToProto(value)
+		if err != nil {
+			return nil, fmt.Errorf("failed to convert index %d to proto: %w", i, err)
+		}
+
+		values[i] = protoValue
 	}
-	return structpb.NewStringValue(v), nil
+
+	protoList := &structpb.ListValue{Values: values}
+	return structpb.NewListValue(protoList), nil
 }
diff --git a/operator/builtin/output/googlecloud/proto_test.go b/operator/builtin/output/googlecloud/proto_test.go
@@ -7,6 +7,8 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
+const invalidUTF8 = "\xa0\xa1"
+
 func TestToProto(t *testing.T) {
 	testCases := []struct {
 		name          string
@@ -15,12 +17,12 @@ func TestToProto(t *testing.T) {
 		expectedErr   string
 	}{
 		{
-			name:          "nil",
+			name:          "nil value",
 			value:         nil,
 			expectedValue: structpb.NewNullValue(),
 		},
 		{
-			name: "numbers",
+			name: "valid numbers",
 			value: map[string]interface{}{
 				"int":     int(1),
 				"int8":    int8(1),
@@ -53,7 +55,32 @@ func TestToProto(t *testing.T) {
 			}),
 		},
 		{
-			name: "map[string]string",
+			name: "valid map[string]interface{}",
+			value: map[string]interface{}{
+				"test": "value",
+			},
+			expectedValue: structpb.NewStructValue(&structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					"test": structpb.NewStringValue("value"),
+				},
+			}),
+		},
+		{
+			name: "map[string]interface with invalid key",
+			value: map[string]interface{}{
+				invalidUTF8: "value",
+			},
+			expectedErr: "invalid UTF-8 in key",
+		},
+		{
+			name: "map[string]interface with invalid value",
+			value: map[string]interface{}{
+				"test_key": make(chan int),
+			},
+			expectedErr: "failed to convert value for key test_key to proto",
+		},
+		{
+			name: "valid map[string]string",
 			value: map[string]string{
 				"test": "value",
 			},
@@ -64,7 +91,21 @@ func TestToProto(t *testing.T) {
 			}),
 		},
 		{
-			name: "map[string]map[string]string",
+			name: "map[string]string with invalid key",
+			value: map[string]string{
+				invalidUTF8: "value",
+			},
+			expectedErr: "invalid UTF-8 in key",
+		},
+		{
+			name: "map[string]string with invalid value",
+			value: map[string]string{
+				"test_key": invalidUTF8,
+			},
+			expectedErr: "failed to convert value for key test_key to proto",
+		},
+		{
+			name: "valid map[string]map[string]string",
 			value: map[string]map[string]string{
 				"test": {
 					"key": "value",
@@ -81,29 +122,62 @@ func TestToProto(t *testing.T) {
 			}),
 		},
 		{
-			name:  "interface list",
+			name: "map[string]map[string]string with invalid key",
+			value: map[string]map[string]string{
+				invalidUTF8: {
+					"key": "value",
+				},
+			},
+			expectedErr: "invalid UTF-8 in key",
+		},
+		{
+			name: "map[string]map[string]string with invalid value",
+			value: map[string]map[string]string{
+				"test_key": {
+					"key": invalidUTF8,
+				},
+			},
+			expectedErr: "failed to convert embedded value for key test_key to proto",
+		},
+		{
+			name:  "valid list",
 			value: []interface{}{"value", 1},
 			expectedValue: structpb.NewListValue(&structpb.ListValue{Values: []*structpb.Value{
 				structpb.NewStringValue("value"),
 				structpb.NewNumberValue(float64(1)),
 			}}),
 		},
 		{
-			name:  "string list",
+			name:        "list with invalid value",
+			value:       []interface{}{make(chan int)},
+			expectedErr: "failed to convert index 0 of list to proto",
+		},
+		{
+			name:  "valid string list",
 			value: []string{"value", "test"},
 			expectedValue: structpb.NewListValue(&structpb.ListValue{Values: []*structpb.Value{
 				structpb.NewStringValue("value"),
 				structpb.NewStringValue("test"),
 			}}),
 		},
+		{
+			name:        "string list with invalid utf-8",
+			value:       []string{invalidUTF8},
+			expectedErr: "failed to convert index 0 to proto",
+		},
+		{
+			name:          "valid byte array",
+			value:         []byte("test"),
+			expectedValue: structpb.NewStringValue("dGVzdA=="),
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			proto, err := toProto(tc.value)
+			proto, err := convertToProto(tc.value)
 			if tc.expectedErr != "" {
 				require.Error(t, err)
-				require.Contains(t, err, tc.expectedErr)
+				require.Contains(t, err.Error(), tc.expectedErr)
 				return
 			}