kmap changes (#2355)

* add list variant to exclude keys

* Rename OrderedLookup to be KeyPriority

- make it a type alias of a string array
- add a update key function that can be used for migration

kmap/lookup.go

@@ -16,25 +16,24 @@ limitations under the License.
 
 package kmap
 
-// OrderedLookup is a utility struct for getting values from a map
+// KeyPriority is a utility struct for getting values from a map
 // given a list of ordered keys
 //
 // This is to help the migration/renaming of annotations & labels
-type OrderedLookup struct {
-	Keys []string
-}
+type KeyPriority []string
 
 // Key returns the default key that should be used for
 // accessing the map
-func (a *OrderedLookup) Key() string {
-	return a.Keys[0]
+func (p KeyPriority) Key() string {
+	// this intentionally panics rather than returning an empty string
+	return p[0]
 }
 
 // Value iterates looks up the ordered keys in the map and returns
 // a string value. An empty string will be returned if the keys
 // are not present in the map
-func (a *OrderedLookup) Value(m map[string]string) string {
-	_, v, _ := a.Get(m)
+func (p KeyPriority) Value(m map[string]string) string {
+	_, v, _ := p.Get(m)
 	return v
 }
 
@@ -46,26 +45,33 @@ func (a *OrderedLookup) Value(m map[string]string) string {
 //
 // If no key is present the default key (lowest ordinal) is returned
 // with an empty string as the value
-func (a *OrderedLookup) Get(m map[string]string) (string, string, bool) {
+func (p KeyPriority) Get(m map[string]string) (string, string, bool) {
 	var k, v string
 	var ok bool
-	for _, k = range a.Keys {
+	for _, k = range p {
 		v, ok = m[k]
 		if ok {
 			return k, v, ok
 		}
 	}
 
-	return a.Keys[0], "", false
+	return p.Key(), "", false
 }
 
-// NewOrderedLookup builds a utilty struct for looking up N keys
-// in a map in a specific order
-//
-// If no keys are supplied this method will panic
-func NewOrderedLookup(keys ...string) *OrderedLookup {
-	if len(keys) == 0 {
-		panic("expected to have at least a single key")
+// UpdateKey will update the map with the KeyPriority's default
+// key iff any of the other synonym keys are present
+func (p KeyPriority) UpdateKey(m map[string]string) {
+	if k, v, ok := p.Get(m); ok && k != p.Key() {
+		delete(m, k)
+		m[p.Key()] = v
 	}
-	return &OrderedLookup{Keys: keys}
+}
+
+// UpdateKeys iterates over the lookups and updates entries in the map
+// to use the default key
+func UpdateKeys(m map[string]string, keys ...KeyPriority) map[string]string {
+	for _, key := range keys {
+		key.UpdateKey(m)
+	}
+	return m
 }

kmap/lookup_test.go

@@ -22,49 +22,31 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-func TestNewOrderedLookup(t *testing.T) {
-	keys := []string{"1", "2", "3"}
-	a := NewOrderedLookup(keys...)
-
-	if diff := cmp.Diff(keys, a.Keys); diff != "" {
-		t.Error("NewOrderedLookup unexpected diff (-want, +got):", diff)
-	}
-}
-
-func TestNewOrderedLookup_BadInput(t *testing.T) {
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("expected no keys to panic")
-		}
-	}()
-	NewOrderedLookup()
-}
-
-func TestOrderedLookup_Get(t *testing.T) {
+func TestKeyPriority(t *testing.T) {
 	tests := []struct {
 		name     string
-		keys     []string
+		keys     KeyPriority
 		in       map[string]string
 		outKey   string
 		outValue string
 		outOk    bool
 	}{{
 		name:     "single key in map",
-		keys:     []string{"old", "new"},
+		keys:     KeyPriority{"old", "new"},
 		in:       map[string]string{"old": "1"},
 		outKey:   "old",
 		outValue: "1",
 		outOk:    true,
 	}, {
 		name:     "another single key in map",
-		keys:     []string{"old", "new"},
+		keys:     KeyPriority{"old", "new"},
 		in:       map[string]string{"new": "1"},
 		outKey:   "new",
 		outValue: "1",
 		outOk:    true,
 	}, {
 		name:     "lower ordinal takes priority",
-		keys:     []string{"old", "new"},
+		keys:     KeyPriority{"old", "new"},
 		in:       map[string]string{"new": "1", "old": "2"},
 		outKey:   "old",
 		outValue: "2",
@@ -82,9 +64,7 @@ func TestOrderedLookup_Get(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			a := NewOrderedLookup(tc.keys...)
-
-			k, v, ok := a.Get(tc.in)
+			k, v, ok := tc.keys.Get(tc.in)
 
 			if tc.outOk != ok {
 				t.Error("expected ok to be", tc.outOk)
@@ -98,7 +78,54 @@ func TestOrderedLookup_Get(t *testing.T) {
 				t.Errorf("expected key to be %q got: %q", tc.outKey, k)
 			}
 
+			if want, got := v, tc.keys.Value(tc.in); got != want {
+				t.Errorf("Value() diff %q != %q", want, got)
+			}
+		})
+	}
+}
+
+func TestUpdateKeys(t *testing.T) {
+	tests := []struct {
+		name string
+		in   map[string]string
+		out  map[string]string
+		ols  []KeyPriority
+	}{{
+		name: "identity",
+		in:   map[string]string{"1": "2"},
+		out:  map[string]string{"1": "2"},
+	}, {
+		name: "single replacement",
+		in:   map[string]string{"1": "2", "4": "5"},
+		ols: []KeyPriority{
+			{"3", "1"},
+		},
+		out: map[string]string{"3": "2", "4": "5"},
+	}, {
+		name: "multiple replacement",
+		in:   map[string]string{"1": "2", "6": "5", "8": "9"},
+		ols: []KeyPriority{
+			{"3", "1"},
+			{"4", "6"},
+		},
+		out: map[string]string{"3": "2", "4": "5", "8": "9"},
+	}, {
+		name: "default not replaced",
+		in:   map[string]string{"1": "2", "4": "5"},
+		ols: []KeyPriority{
+			{"1", "3"},
+		},
+		out: map[string]string{"1": "2", "4": "5"},
+	}}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+
+			out := UpdateKeys(tc.in, tc.ols...)
+			if diff := cmp.Diff(tc.out, out); diff != "" {
+				t.Error("Migrate diff (-want,+got):", diff)
+			}
 		})
 	}
-
 }

kmap/map.go

@@ -57,6 +57,11 @@ func Filter(in map[string]string, filter func(string) bool) map[string]string {
 
 // ExcludeKeys creates a copy of the provided map filtering out the excluded `keys`
 func ExcludeKeys(in map[string]string, keys ...string) map[string]string {
+	return ExcludeKeyList(in, keys)
+}
+
+// ExcludeKeyList creates a copy of the provided map filtering out excluded `keys`
+func ExcludeKeyList(in map[string]string, keys []string) map[string]string {
 	ret := make(map[string]string, len(in))
 
 outer:

kmap/map_test.go

@@ -186,6 +186,10 @@ func TestExcludeKeys(t *testing.T) {
 			if diff := cmp.Diff(test.want, got); diff != "" {
 				t.Error("ExcludeKeys (-want, +got) =", diff)
 			}
+			got = ExcludeKeyList(test.in, test.exclude)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Error("ExcludeKeyList (-want, +got) =", diff)
+			}
 		})
 	}
 }