feat: removes empty interfaces and uses a json parser to reduce refle…

…ction usage

Signed-off-by: Jennifer Power <[email protected]>

api/client/v1alpha1/config_types.go

@@ -62,13 +62,9 @@ type File struct {
 	// permissions for files that can be used with container runtimes.
 	FileInfo empspec.File `json:"fileInfo,omitempty"`
 	// Attributes is the lists of to associate to the file.
-	Attributes Attributes `json:"attributes,omitempty"`
+	Attributes json.RawMessage `json:"attributes,omitempty"`
 }
 
-// Attributes is a map structure that holds all
-// attribute information provided by the user.
-type Attributes map[string]interface{}
-
 // UnmarshalJSON sets custom unmarshalling logic to File.
 // In this case it sets the default UID and GID to invalid
 // ID numbers to differentiate between values intentionally set at 0.

attributes/bool_test.go

@@ -45,3 +45,17 @@ func TestBoolAttribute_IsNull(t *testing.T) {
 	test := NewBool(false)
 	require.False(t, test.IsNull())
 }
+
+func TestBoolAttribute_AsList(t *testing.T) {
+	test := NewBool(false)
+	s, err := test.AsList()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, []model.AttributeValue(nil), s)
+}
+
+func TestBoolAttribute_AsObject(t *testing.T) {
+	test := NewBool(false)
+	s, err := test.AsObject()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, map[string]model.AttributeValue(nil), s)
+}

nodes/descriptor/errors.go → attributes/errors.go

@@ -1,6 +1,9 @@
-package descriptor
+package attributes
 
-import "fmt"
+import (
+	"errors"
+	"fmt"
+)
 
 // ParseError defines an error when parsing attributes into Properties.
 type ParseError struct {
@@ -11,3 +14,7 @@ type ParseError struct {
 func (e ParseError) Error() string {
 	return fmt.Sprintf("parse property key %q: %v", e.Key, e.Err)
 }
+
+// ErrInvalidAttribute defines the error thrown when an attribute has an invalid
+// type.
+var ErrInvalidAttribute = errors.New("invalid attribute type")

attributes/float_test.go

@@ -45,3 +45,17 @@ func TestFloatAttribute_IsNull(t *testing.T) {
 	test := NewFloat(1.0)
 	require.False(t, test.IsNull())
 }
+
+func TestFloatAttribute_AsList(t *testing.T) {
+	test := NewFloat(1)
+	s, err := test.AsList()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, []model.AttributeValue(nil), s)
+}
+
+func TestFloatAttribute_AsObject(t *testing.T) {
+	test := NewFloat(1)
+	s, err := test.AsObject()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, map[string]model.AttributeValue(nil), s)
+}

attributes/int_test.go

@@ -45,3 +45,17 @@ func TestIntAttribute_IsNull(t *testing.T) {
 	test := NewInt(1)
 	require.False(t, test.IsNull())
 }
+
+func TestIntAttribute_AsList(t *testing.T) {
+	test := NewInt(1)
+	s, err := test.AsList()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, []model.AttributeValue(nil), s)
+}
+
+func TestIntAttribute_AsObject(t *testing.T) {
+	test := NewInt(1)
+	s, err := test.AsObject()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, map[string]model.AttributeValue(nil), s)
+}

attributes/list.go

@@ -1,6 +1,6 @@
 package attributes
 
-import "github.com/uor-framework/uor-client-go/model"
+import "github.com/emporous/emporous-go/model"
 
 type sliceAttribute []model.AttributeValue
 

attributes/list_test.go

@@ -1 +1,76 @@
 package attributes
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/emporous/emporous-go/model"
+)
+
+func TestListAttribute_Kind(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	require.Equal(t, model.KindList, test.Kind())
+}
+
+func TestListAttribute_AsBool(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	s, err := test.AsBool()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, false, s)
+}
+
+func TestListAttribute_AsFloat(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	s, err := test.AsFloat()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, float64(0), s)
+}
+
+func TestListAttribute_AsInt(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	s, err := test.AsInt()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, int64(0), s)
+}
+
+func TestListAttribute_AsString(t *testing.T) {
+	test := NewList([]model.AttributeValue{})
+	s, err := test.AsString()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, "", s)
+}
+
+func TestListAttribute_IsNull(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	require.False(t, test.IsNull())
+}
+
+func TestListAttribute_AsList(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	s, err := test.AsList()
+	require.NoError(t, err)
+	require.Equal(t, []model.AttributeValue{NewString("testvalue")}, s)
+
+}
+
+func TestListAttribute_AsObject(t *testing.T) {
+	test := NewList([]model.AttributeValue{
+		NewString("testvalue"),
+	})
+	s, err := test.AsObject()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, map[string]model.AttributeValue(nil), s)
+}

attributes/null_test.go

@@ -45,3 +45,17 @@ func TestNullAttribute_IsNull(t *testing.T) {
 	test := NewNull()
 	require.True(t, test.IsNull())
 }
+
+func TestNullAttribute_AsList(t *testing.T) {
+	test := NewNull()
+	s, err := test.AsList()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, []model.AttributeValue(nil), s)
+}
+
+func TestNullAttribute_AsObject(t *testing.T) {
+	test := NewNull()
+	s, err := test.AsObject()
+	require.ErrorIs(t, ErrWrongKind, err)
+	require.Equal(t, map[string]model.AttributeValue(nil), s)
+}

attributes/object.go

@@ -7,7 +7,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/uor-framework/uor-client-go/model"
+	"github.com/emporous/emporous-go/model"
 )
 
 // ErrWrongKind defines a type error try to cast an attributes value
@@ -104,22 +104,16 @@ func (a mapAttribute) Exists(key string, value model.AttributeValue) (bool, erro
 		return false, nil
 	}
 
-	switch value.Kind() {
-	case model.KindObject:
-		return checkObject(value, setVal)
-	case model.KindList:
-		return checkObject(value, setVal)
-	default:
-		return checkValue(value, setVal)
-	}
+	return checkValue(value, setVal)
 }
 
-func checkObject(input, val model.AttributeValue) (bool, error) {
-	inputMap, err := input.AsObject()
+func checkObject(inputVal, setVal model.AttributeValue) (bool, error) {
+	inputMap, err := inputVal.AsObject()
 	if err != nil {
 		return false, err
 	}
-	valMap, err := val.AsObject()
+
+	valMap, err := setVal.AsObject()
 	if err != nil {
 		return false, err
 	}
@@ -211,16 +205,13 @@ func checkValue(input, val model.AttributeValue) (bool, error) {
 		}
 		return outB == inB, nil
 	case model.KindNull:
-		if val.IsNull() {
-			return true, nil
-		}
-		return false, nil
+		return val.IsNull(), nil
 	case model.KindObject:
 		return checkObject(input, val)
 	case model.KindList:
 		return checkList(input, val)
 	default:
-		return false, nil
+		return false, fmt.Errorf("unsupported type")
 	}
 }
 
@@ -245,8 +236,14 @@ func (a mapAttribute) Len() int {
 	return len(a)
 }
 
-// Merge attempts to merge multiple attribute object types.
-func Merge(original map[string]model.AttributeValue, patches ...map[string]model.AttributeValue) (map[string]model.AttributeValue, error) {
+// MergeOptions define options for attribute merging.
+type MergeOptions struct {
+	AllowSameTypeOverwrites bool
+}
+
+// Merge complete a two-way merge of multiple sets of attributes. If AllowSameTypeOverwrites is set to true, patches can overwrite
+// values of the same type (default false). If false, the function will return an error  when a key collision occurs.
+func Merge(original map[string]model.AttributeValue, opts MergeOptions, patches ...map[string]model.AttributeValue) (map[string]model.AttributeValue, error) {
 	if len(patches) == 0 {
 		return original, nil
 	}
@@ -255,18 +252,17 @@ func Merge(original map[string]model.AttributeValue, patches ...map[string]model
 	currSet := NewObject(original)
 	for _, patch := range patches {
 		patchObject := NewObject(patch)
-		currSet, err = mergeObjects(currSet, patchObject, nil)
+		currSet, err = mergeObjects(currSet, patchObject, nil, opts)
 		if err != nil {
 			return nil, err
 		}
 	}
 
 	mergedList := currSet.(mapAttribute)
-
 	return mergedList, nil
 }
 
-func mergeValue(path []string, patch model.AttributeValue, key string, value model.AttributeValue) (model.AttributeValue, error) {
+func mergeValue(path []string, patch model.AttributeValue, key string, value model.AttributeValue, opts MergeOptions) (model.AttributeValue, error) {
 	patchObject, err := patch.AsObject()
 	if err != nil {
 		return nil, err
@@ -284,21 +280,34 @@ func mergeValue(path []string, patch model.AttributeValue, key string, value mod
 
 	if value.Kind() == model.KindObject {
 		if !patchValueIsObject {
-			err := fmt.Errorf("patch value must be object for key \"%v\"", pathStr)
-			return value, err
+			return value, fmt.Errorf("patch value must be object for key %q", pathStr)
 		}
 
-		return mergeObjects(value, patchValue, path)
+		return mergeObjects(value, patchValue, path, opts)
 	}
 
 	if value.Kind() == model.KindList && patchValueIsObject {
-		return mergeObjects(value, patchValue, path)
+		return mergeObjects(value, patchValue, path, opts)
+	}
+
+	if value.Kind() != patchValue.Kind() {
+		return nil, fmt.Errorf("path %q: %w", pathStr, ErrWrongKind)
+	}
+
+	if !opts.AllowSameTypeOverwrites {
+		match, err := checkValue(value, patchValue)
+		if err != nil {
+			return nil, fmt.Errorf("path %q: %w", pathStr, err)
+		}
+		if !match {
+			return nil, fmt.Errorf("cannot overwrite value at %q", pathStr)
+		}
 	}
 
 	return patchValue, nil
 }
 
-func mergeObjects(data, patch model.AttributeValue, path []string) (model.AttributeValue, error) {
+func mergeObjects(data, patch model.AttributeValue, path []string, opts MergeOptions) (model.AttributeValue, error) {
 	if patch.Kind() == model.KindObject {
 		if data.Kind() == model.KindList {
 			dataArray, err := data.AsList()
@@ -309,7 +318,7 @@ func mergeObjects(data, patch model.AttributeValue, path []string) (model.Attrib
 			ret := make([]model.AttributeValue, len(dataArray))
 
 			for i, val := range dataArray {
-				ret[i], err = mergeValue(path, patch, strconv.Itoa(i), val)
+				ret[i], err = mergeValue(path, patch, strconv.Itoa(i), val, opts)
 				if err != nil {
 					return NewList(ret), err
 				}
@@ -324,12 +333,24 @@ func mergeObjects(data, patch model.AttributeValue, path []string) (model.Attrib
 			ret := mapAttribute{}
 
 			for k, v := range dataObject {
-				ret[k], err = mergeValue(path, patch, k, v)
+				ret[k], err = mergeValue(path, patch, k, v, opts)
 				if err != nil {
 					return ret, err
 				}
 			}
 
+			patchObject, err := patch.AsObject()
+			if err != nil {
+				return nil, err
+			}
+			// Add in new objects from patches
+			for key, value := range patchObject {
+				_, ok := ret[key]
+				if !ok {
+					ret[key] = value
+				}
+			}
+
 			return ret, nil
 		}
 	}