From fa1a98735bcaccd2e9a426864f560b701cf4232c Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Mon, 18 Sep 2023 13:59:24 -0700 Subject: [PATCH 1/4] Add unit test for data.GetValue --- pkg/data/values_test.go | 94 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 pkg/data/values_test.go diff --git a/pkg/data/values_test.go b/pkg/data/values_test.go new file mode 100644 index 00000000..8466d44b --- /dev/null +++ b/pkg/data/values_test.go @@ -0,0 +1,94 @@ +package data + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetValue(t *testing.T) { + t.Parallel() + tests := []struct { + name string + data map[string]interface{} + keys []string + wantValue interface{} + wantSuccess bool + }{ + { + name: "nil map", + data: nil, + keys: []string{"somekey"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "key is not in map", + data: map[string]interface{}{ + "realKey": "realVal", + }, + keys: []string{"badKey"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "key is in first level of map", + data: map[string]interface{}{ + "realKey": "realVal", + }, + keys: []string{"realKey"}, + wantValue: "realVal", + wantSuccess: true, + }, + { + name: "key is nested in map", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "child": map[string]interface{}{ + "grandchild": "someValue", + }, + }, + }, + keys: []string{"parent", "child", "grandchild"}, + wantValue: "someValue", + wantSuccess: true, + }, + { + name: "incorrected nested key", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "child": map[string]interface{}{ + "grandchild": "someValue", + }, + }, + }, + keys: []string{"parent", "grandchild", "child"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "get index of slice", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "children": []string{ + "alice", + "bob", + "eve", + }, + }, + }, + keys: []string{"parent", "children", "2"}, + wantValue: nil, + wantSuccess: false, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + gotValue, gotSuccess := GetValue(test.data, test.keys...) + assert.Equal(t, test.wantValue, gotValue) + assert.Equal(t, test.wantSuccess, gotSuccess) + }) + } +} From 63eb31bb73e0fa5d58846de9c99c3a057c3319c0 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Mon, 18 Sep 2023 14:01:06 -0700 Subject: [PATCH 2/4] Add support for getting array index in GetValue Add the ability to use an index number to get an item out of a slice. The number will still be a string representation of an integer. For example, to get the last element from a nested slice like: ``` data := map[string]interface{}{ "data": []interface{}{ "first", "second", "third", }, } ``` Use keys like ``` val := GetValues(data, "data", "2") ``` --- pkg/data/values.go | 32 +++++++++++++++++++++++++---- pkg/data/values_test.go | 45 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/pkg/data/values.go b/pkg/data/values.go index 87ff6159..1942ec24 100644 --- a/pkg/data/values.go +++ b/pkg/data/values.go @@ -1,5 +1,9 @@ package data +import ( + "strconv" +) + func RemoveValue(data map[string]interface{}, keys ...string) (interface{}, bool) { for i, key := range keys { if i == len(keys)-1 { @@ -18,18 +22,38 @@ func GetValueN(data map[string]interface{}, keys ...string) interface{} { return val } -func GetValue(data map[string]interface{}, keys ...string) (interface{}, bool) { +func GetValue(data interface{}, keys ...string) (interface{}, bool) { for i, key := range keys { if i == len(keys)-1 { - val, ok := data[key] - return val, ok + if dataMap, ok := data.(map[string]interface{}); ok { + val, ok := dataMap[key] + return val, ok + } + if dataSlice, ok := data.([]interface{}); ok { + return itemByIndex(dataSlice, key) + } + } + if dataMap, ok := data.(map[string]interface{}); ok { + data, _ = dataMap[key] + } else if dataSlice, ok := data.([]interface{}); ok { + data, _ = itemByIndex(dataSlice, key) } - data, _ = data[key].(map[string]interface{}) } return nil, false } +func itemByIndex(dataSlice []interface{}, key string) (interface{}, bool) { + keyInt, err := strconv.Atoi(key) + if err != nil { + return nil, false + } + if keyInt >= len(dataSlice) || keyInt < 0 { + return nil, false + } + return dataSlice[keyInt], true +} + func PutValue(data map[string]interface{}, val interface{}, keys ...string) { if data == nil { return diff --git a/pkg/data/values_test.go b/pkg/data/values_test.go index 8466d44b..6b01f6b7 100644 --- a/pkg/data/values_test.go +++ b/pkg/data/values_test.go @@ -10,7 +10,7 @@ func TestGetValue(t *testing.T) { t.Parallel() tests := []struct { name string - data map[string]interface{} + data interface{} keys []string wantValue interface{} wantSuccess bool @@ -70,7 +70,7 @@ func TestGetValue(t *testing.T) { name: "get index of slice", data: map[string]interface{}{ "parent": map[string]interface{}{ - "children": []string{ + "children": []interface{}{ "alice", "bob", "eve", @@ -78,6 +78,47 @@ func TestGetValue(t *testing.T) { }, }, keys: []string{"parent", "children", "2"}, + wantValue: "eve", + wantSuccess: true, + }, + { + name: "get index of top levelslice", + data: []interface{}{ + "alice", + "bob", + "eve", + }, + keys: []string{"2"}, + wantValue: "eve", + wantSuccess: true, + }, + { + name: "index is too big", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "children": []interface{}{ + "alice", + "bob", + "eve", + }, + }, + }, + keys: []string{"parent", "children", "3"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "index is negative", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "children": []interface{}{ + "alice", + "bob", + "eve", + }, + }, + }, + keys: []string{"parent", "children", "-3"}, wantValue: nil, wantSuccess: false, }, From 159271212254836e7523583f701514b445f57724 Mon Sep 17 00:00:00 2001 From: Michael Bolot Date: Mon, 9 Oct 2023 12:54:34 -0500 Subject: [PATCH 3/4] Adding package comments for data package Adds package comments and extra test cases --- pkg/data/values.go | 13 ++++++ pkg/data/values_test.go | 91 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/pkg/data/values.go b/pkg/data/values.go index 1942ec24..a0f07f42 100644 --- a/pkg/data/values.go +++ b/pkg/data/values.go @@ -1,9 +1,15 @@ +// Package data contains functions for working with unstructured values like []interface or map[string]interface{}. +// It allows reading/writing to these values without having to convert to structured items. package data import ( "strconv" ) +// RemoveValue removes a value from data. Keys should be in order denoting the path to the value in the nested +// structure of the map. For example, passing []string{"metadata", "annotations"} will make the function remove the +// "annotations" key from the "metadata" sub-map. Returns the removed value (if any) and a bool indicating if the value +// was found. func RemoveValue(data map[string]interface{}, keys ...string) (interface{}, bool) { for i, key := range keys { if i == len(keys)-1 { @@ -22,6 +28,11 @@ func GetValueN(data map[string]interface{}, keys ...string) interface{} { return val } +// GetValue retrieves a value from the provided collection, which must be a map[string]interface or a []interface. +// Keys are always strings. +// For a map, a key denotes the key in the map whose value we want to retrieve. +// For the slice, it denotes the index (starting at 0) of the value we want to retrieve. +// Returns the retrieved value (if any) and a bool indicating if the value was found. func GetValue(data interface{}, keys ...string) (interface{}, bool) { for i, key := range keys { if i == len(keys)-1 { @@ -54,6 +65,8 @@ func itemByIndex(dataSlice []interface{}, key string) (interface{}, bool) { return dataSlice[keyInt], true } +// PutValue updates the value of a given map at the index specified by keys that denote the path to the value in the +// nested structure of the map. If there is no current entry at a key, a new map is created for that value. func PutValue(data map[string]interface{}, val interface{}, keys ...string) { if data == nil { return diff --git a/pkg/data/values_test.go b/pkg/data/values_test.go index 6b01f6b7..a3a1a5ab 100644 --- a/pkg/data/values_test.go +++ b/pkg/data/values_test.go @@ -82,7 +82,7 @@ func TestGetValue(t *testing.T) { wantSuccess: true, }, { - name: "get index of top levelslice", + name: "get index of top level slice", data: []interface{}{ "alice", "bob", @@ -92,6 +92,26 @@ func TestGetValue(t *testing.T) { wantValue: "eve", wantSuccess: true, }, + { + name: "slice of maps", + data: []interface{}{ + map[string]interface{}{ + "notthisone": "val", + }, + map[string]interface{}{ + "parent": map[string]interface{}{ + "children": []interface{}{ + "alice", + "bob", + "eve", + }, + }, + }, + }, + keys: []string{"1", "parent", "children", "0"}, + wantValue: "alice", + wantSuccess: true, + }, { name: "index is too big", data: map[string]interface{}{ @@ -122,6 +142,75 @@ func TestGetValue(t *testing.T) { wantValue: nil, wantSuccess: false, }, + { + name: "index not parseable to int", + data: map[string]interface{}{ + "parent": map[string]interface{}{ + "children": []interface{}{ + "alice", + "bob", + "eve", + }, + }, + }, + keys: []string{"parent", "children", "notanint"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "slice blank index", + data: []interface{}{ + "bob", + }, + keys: []string{""}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "slice no index", + data: []interface{}{ + "bob", + }, + wantValue: nil, + wantSuccess: false, + }, + { + name: "keys nested too far", + data: []interface{}{ + "alice", + "bob", + "eve", + }, + keys: []string{"2", "1"}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "map blank key with value", + data: map[string]interface{}{ + "": "bob", + }, + keys: []string{""}, + wantValue: "bob", + wantSuccess: true, + }, + { + name: "map blank key no value", + data: map[string]interface{}{ + "alice": "bob", + }, + keys: []string{""}, + wantValue: nil, + wantSuccess: false, + }, + { + name: "map no key", + data: map[string]interface{}{ + "": "bob", + }, + wantValue: nil, + wantSuccess: false, + }, } for _, test := range tests { test := test From 53fba45d47b9b184f7b385dea890c05aa399cd3f Mon Sep 17 00:00:00 2001 From: Michael Bolot Date: Wed, 21 Feb 2024 12:19:50 -0600 Subject: [PATCH 4/4] Restoring GetValue Restores GetValue to it's original interface to avoid breaking changes. --- pkg/data/values.go | 17 +++++++++++++++-- pkg/data/values_test.go | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/data/values.go b/pkg/data/values.go index a0f07f42..0f6c634a 100644 --- a/pkg/data/values.go +++ b/pkg/data/values.go @@ -28,12 +28,25 @@ func GetValueN(data map[string]interface{}, keys ...string) interface{} { return val } -// GetValue retrieves a value from the provided collection, which must be a map[string]interface or a []interface. +// GetValue works similar to GetValueFromAny, but can only process maps. Kept this way to avoid breaking changes with +// the previous interface, GetValueFromAny should be used in most cases since that can handle slices as well. +func GetValue(data map[string]interface{}, keys ...string) (interface{}, bool) { + for i, key := range keys { + if i == len(keys)-1 { + val, ok := data[key] + return val, ok + } + data, _ = data[key].(map[string]interface{}) + } + return nil, false +} + +// GetValueFromAny retrieves a value from the provided collection, which must be a map[string]interface or a []interface. // Keys are always strings. // For a map, a key denotes the key in the map whose value we want to retrieve. // For the slice, it denotes the index (starting at 0) of the value we want to retrieve. // Returns the retrieved value (if any) and a bool indicating if the value was found. -func GetValue(data interface{}, keys ...string) (interface{}, bool) { +func GetValueFromAny(data interface{}, keys ...string) (interface{}, bool) { for i, key := range keys { if i == len(keys)-1 { if dataMap, ok := data.(map[string]interface{}); ok { diff --git a/pkg/data/values_test.go b/pkg/data/values_test.go index a3a1a5ab..327459d0 100644 --- a/pkg/data/values_test.go +++ b/pkg/data/values_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetValue(t *testing.T) { +func TestGetValueFromAny(t *testing.T) { t.Parallel() tests := []struct { name string @@ -216,7 +216,7 @@ func TestGetValue(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { t.Parallel() - gotValue, gotSuccess := GetValue(test.data, test.keys...) + gotValue, gotSuccess := GetValueFromAny(test.data, test.keys...) assert.Equal(t, test.wantValue, gotValue) assert.Equal(t, test.wantSuccess, gotSuccess) })