fix(controller): Fix nested maps. Fixes #3653 (#3661)

argoproj · Aug 3, 2020 · fb367f5 · fb367f5
1 parent 2385cca
commit fb367f5
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/pkg/apis/workflow/v1alpha1/item.go b/pkg/apis/workflow/v1alpha1/item.go
@@ -63,6 +63,7 @@ func (i *Item) String() string {
 	if err != nil {
 		panic(err)
 	}
+	// this convenience to remove quotes from strings will cause many problems
 	if jsonBytes[0] == '"' {
 		return string(jsonBytes[1 : len(jsonBytes)-1])
 	}
@@ -95,15 +96,15 @@ func (i Item) OpenAPISchemaType() []string {
 func (i Item) OpenAPISchemaFormat() string { return "" }
 
 // you MUST assert `GetType() == Map` before invocation as this does not return errors
-func (i *Item) GetMapVal() map[string]interface{} {
-	val := make(map[string]interface{})
+func (i *Item) GetMapVal() map[string]Item {
+	val := make(map[string]Item)
 	_ = json.Unmarshal(i.Value, &val)
 	return val
 }
 
 // you MUST assert `GetType() == List` before invocation as this does not return errors
-func (i *Item) GetListVal() []interface{} {
-	val := make([]interface{}, 0)
+func (i *Item) GetListVal() []Item {
+	val := make([]Item, 0)
 	_ = json.Unmarshal(i.Value, &val)
 	return val
 }

diff --git a/pkg/apis/workflow/v1alpha1/item_test.go b/pkg/apis/workflow/v1alpha1/item_test.go
@@ -18,7 +18,7 @@ func TestItem(t *testing.T) {
 		"{\"val\":\"123\"}":               Map,
 		"[\"1\",\"2\",\"3\",\"4\",\"5\"]": List,
 	} {
-		t.Run(string(expectedType), func(t *testing.T) {
+		t.Run(fmt.Sprintf("%v", expectedType), func(t *testing.T) {
 			t.Run("Item", func(t *testing.T) {
 				runItemTest(t, data, expectedType)
 			})
@@ -47,15 +47,15 @@ func TestItem_GetMapVal(t *testing.T) {
 	err := json.Unmarshal([]byte(`{"foo":"bar"}`), &item)
 	assert.NoError(t, err)
 	val := item.GetMapVal()
-	assert.Len(t, val, 1)
+	assert.Equal(t, map[string]Item{"foo": {Value: []byte(`"bar"`)}}, val)
 }
 
 func TestItem_GetListVal(t *testing.T) {
 	item := Item{}
 	err := json.Unmarshal([]byte(`["foo"]`), &item)
 	assert.NoError(t, err)
 	val := item.GetListVal()
-	assert.Len(t, val, 1)
+	assert.Equal(t, []Item{{Value: []byte(`"foo"`)}}, val)
 }
 
 func TestItem_GetStrVal(t *testing.T) {

diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
@@ -4437,7 +4437,7 @@ func TestFailSuspendedAndPendingNodesAfterShutdown(t *testing.T) {
 	})
 }
 
-func TestProcessItem(t *testing.T) {
+func Test_processItem(t *testing.T) {
 	task := wfv1.DAGTask{
 		WithParam: `[{"number": 2, "string": "foo", "list": [0, "1"], "json": {"number": 2, "string": "foo", "list": [0, "1"]}}]`,
 	}
@@ -4453,6 +4453,6 @@ func TestProcessItem(t *testing.T) {
 	var newTask wfv1.DAGTask
 	newTaskName, err := processItem(fstTmpl, "task-name", 0, items[0], &newTask)
 	if assert.NoError(t, err) {
-		assert.Equal(t, `task-name(0:json:map[list:[0 1] number:2 string:foo],list:[0 1],number:2,string:foo)`, newTaskName)
+		assert.Equal(t, `task-name(0:json:{"number":2,"string":"foo","list":[0,"1"]},list:[0,"1"],number:2,string:foo)`, newTaskName)
 	}
 }