Only pass literals to execution if they users provided values (#105)

* Only pass literals to execution if they users provided values Signed-off-by: Haytham Abuelfutuh <[email protected]> * Adding unit tests and a bit of refactoring Signed-off-by: Haytham Abuelfutuh <[email protected]>
flyteorg · Jun 17, 2021 · 5e967f7 · 5e967f7
1 parent f9dbf79
commit 5e967f7
Show file tree

Hide file tree

Showing 2 changed files with 189 additions and 8 deletions.
diff --git a/flytectl/cmd/create/serialization_utils.go b/flytectl/cmd/create/serialization_utils.go
@@ -1,29 +1,62 @@
 package create
 
 import (
+	"fmt"
+
 	"github.com/flyteorg/flyteidl/clients/go/coreutils"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
 )
 
 // TODO: Move all functions to flyteidl
+// MakeLiteralForVariables builds a map of literals for the provided serialized values. If a provided value does not have
+// a corresponding variable or if that variable is invalid (e.g. doesn't have Type property populated), it returns an
+// error.
 func MakeLiteralForVariables(serialize map[string]interface{}, variables map[string]*core.Variable) (map[string]*core.Literal, error) {
-	result := make(map[string]*core.Literal)
-	var err error
+	types := make(map[string]*core.LiteralType)
 	for k, v := range variables {
-		if result[k], err = coreutils.MakeLiteralForType(v.Type, serialize[k]); err != nil {
-			return nil, err
+		t := v.GetType()
+		if t == nil {
+			return nil, fmt.Errorf("variable [%v] has nil type", k)
 		}
+
+		types[k] = t
 	}
-	return result, nil
+
+	return MakeLiteralForTypes(serialize, types)
 }
 
+// MakeLiteralForParams builds a map of literals for the provided serialized values. If a provided value does not have
+// a corresponding parameter or if that parameter is invalid (e.g. doesn't have Type property populated), it returns an
+// error.
 func MakeLiteralForParams(serialize map[string]interface{}, parameters map[string]*core.Parameter) (map[string]*core.Literal, error) {
+	types := make(map[string]*core.LiteralType)
+	for k, v := range parameters {
+		if variable := v.GetVar(); variable == nil {
+			return nil, fmt.Errorf("parameter [%v] has nil Variable", k)
+		} else if t := variable.GetType(); t == nil {
+			return nil, fmt.Errorf("parameter [%v] has nil variable type", k)
+		} else {
+			types[k] = t
+		}
+	}
+
+	return MakeLiteralForTypes(serialize, types)
+}
+
+// MakeLiteralForTypes builds a map of literals for the provided serialized values. If a provided value does not have
+// a corresponding type or if it fails to create a literal for the given type and value, it returns an error.
+func MakeLiteralForTypes(serialize map[string]interface{}, types map[string]*core.LiteralType) (map[string]*core.Literal, error) {
 	result := make(map[string]*core.Literal)
 	var err error
-	for k, v := range parameters {
-		if result[k], err = coreutils.MakeLiteralForType(v.GetVar().Type, serialize[k]); err != nil {
-			return nil, err
+	for k, v := range serialize {
+		if t, typeFound := types[k]; typeFound {
+			if result[k], err = coreutils.MakeLiteralForType(t, v); err != nil {
+				return nil, err
+			}
+		} else {
+			return nil, fmt.Errorf("no matching type for [%v]", k)
 		}
 	}
+
 	return result, nil
 }
diff --git a/flytectl/cmd/create/serialization_utils_test.go b/flytectl/cmd/create/serialization_utils_test.go
@@ -0,0 +1,148 @@
+package create
+
+import (
+	"testing"
+
+	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMakeLiteralForTypes(t *testing.T) {
+	inputTypes := map[string]*core.LiteralType{
+		"a": {
+			Type: &core.LiteralType_Simple{
+				Simple: core.SimpleType_INTEGER,
+			},
+		},
+		"x": {
+			Type: &core.LiteralType_Simple{
+				Simple: core.SimpleType_INTEGER,
+			},
+		},
+		"b": {
+			Type: &core.LiteralType_Simple{
+				Simple: core.SimpleType_STRING,
+			},
+		},
+		"y": {
+			Type: &core.LiteralType_Simple{
+				Simple: core.SimpleType_STRING,
+			},
+		},
+	}
+
+	t.Run("Happy path", func(t *testing.T) {
+		inputValues := map[string]interface{}{
+			"a": 5,
+			"b": "hello",
+		}
+
+		m, err := MakeLiteralForTypes(inputValues, inputTypes)
+		assert.NoError(t, err)
+		assert.Len(t, m, len(inputValues))
+	})
+
+	t.Run("Type not found", func(t *testing.T) {
+		inputValues := map[string]interface{}{
+			"notfound": 5,
+		}
+
+		_, err := MakeLiteralForTypes(inputValues, inputTypes)
+		assert.Error(t, err)
+	})
+
+	t.Run("Invalid value", func(t *testing.T) {
+		inputValues := map[string]interface{}{
+			"a": "hello",
+		}
+
+		_, err := MakeLiteralForTypes(inputValues, inputTypes)
+		assert.Error(t, err)
+	})
+}
+
+func TestMakeLiteralForParams(t *testing.T) {
+	inputValues := map[string]interface{}{
+		"a": "hello",
+	}
+
+	t.Run("Happy path", func(t *testing.T) {
+		inputParams := map[string]*core.Parameter{
+			"a": {
+				Var: &core.Variable{
+					Type: &core.LiteralType{
+						Type: &core.LiteralType_Simple{
+							Simple: core.SimpleType_STRING,
+						},
+					},
+				},
+			},
+		}
+
+		m, err := MakeLiteralForParams(inputValues, inputParams)
+		assert.NoError(t, err)
+		assert.Len(t, m, len(inputValues))
+	})
+
+	t.Run("Invalid Param", func(t *testing.T) {
+		inputParams := map[string]*core.Parameter{
+			"a": nil,
+		}
+
+		_, err := MakeLiteralForParams(inputValues, inputParams)
+		assert.Error(t, err)
+	})
+
+	t.Run("Invalid Type", func(t *testing.T) {
+		inputParams := map[string]*core.Parameter{
+			"a": {
+				Var: &core.Variable{},
+			},
+		}
+
+		_, err := MakeLiteralForParams(inputValues, inputParams)
+		assert.Error(t, err)
+	})
+}
+
+func TestMakeLiteralForVariables(t *testing.T) {
+	inputValues := map[string]interface{}{
+		"a": "hello",
+	}
+
+	t.Run("Happy path", func(t *testing.T) {
+		inputVariables := map[string]*core.Variable{
+			"a": {
+				Type: &core.LiteralType{
+					Type: &core.LiteralType_Simple{
+						Simple: core.SimpleType_STRING,
+					},
+				},
+			},
+		}
+
+		m, err := MakeLiteralForVariables(inputValues, inputVariables)
+		assert.NoError(t, err)
+		assert.Len(t, m, len(inputValues))
+	})
+
+	t.Run("Invalid Variable", func(t *testing.T) {
+		inputVariables := map[string]*core.Variable{
+			"a": nil,
+		}
+
+		_, err := MakeLiteralForVariables(inputValues, inputVariables)
+		assert.Error(t, err)
+	})
+
+	t.Run("Invalid Type", func(t *testing.T) {
+		inputVariables := map[string]*core.Variable{
+			"a": {
+				Type: nil,
+			},
+		}
+
+		_, err := MakeLiteralForVariables(inputValues, inputVariables)
+		assert.Error(t, err)
+	})
+}