diff --git a/internal/serialize/keyvalues.go b/internal/serialize/keyvalues.go index 193796f2..cb97576b 100644 --- a/internal/serialize/keyvalues.go +++ b/internal/serialize/keyvalues.go @@ -46,55 +46,6 @@ func WithValues(oldKV, newKV []interface{}) []interface{} { return kv } -// TrimDuplicates deduplicates elements provided in multiple key/value tuple -// slices, whilst maintaining the distinction between where the items are -// contained. -func TrimDuplicates(kvLists ...[]interface{}) [][]interface{} { - // maintain a map of all seen keys - seenKeys := map[interface{}]struct{}{} - // build the same number of output slices as inputs - outs := make([][]interface{}, len(kvLists)) - // iterate over the input slices backwards, as 'later' kv specifications - // of the same key will take precedence over earlier ones - for i := len(kvLists) - 1; i >= 0; i-- { - // initialise this output slice - outs[i] = []interface{}{} - // obtain a reference to the kvList we are processing - // and make sure it has an even number of entries - kvList := kvLists[i] - if len(kvList)%2 != 0 { - kvList = append(kvList, missingValue) - } - - // start iterating at len(kvList) - 2 (i.e. the 2nd last item) for - // slices that have an even number of elements. - // We add (len(kvList) % 2) here to handle the case where there is an - // odd number of elements in a kvList. - // If there is an odd number, then the last element in the slice will - // have the value 'null'. - for i2 := len(kvList) - 2 + (len(kvList) % 2); i2 >= 0; i2 -= 2 { - k := kvList[i2] - // if we have already seen this key, do not include it again - if _, ok := seenKeys[k]; ok { - continue - } - // make a note that we've observed a new key - seenKeys[k] = struct{}{} - // attempt to obtain the value of the key - var v interface{} - // i2+1 should only ever be out of bounds if we handling the first - // iteration over a slice with an odd number of elements - if i2+1 < len(kvList) { - v = kvList[i2+1] - } - // add this KV tuple to the *start* of the output list to maintain - // the original order as we are iterating over the slice backwards - outs[i] = append([]interface{}{k, v}, outs[i]...) - } - } - return outs -} - // MergeKVs deduplicates elements provided in two key/value slices. // // Keys in each slice are expected to be unique, so duplicates can only occur diff --git a/internal/serialize/keyvalues_test.go b/internal/serialize/keyvalues_test.go index 7fd95e7e..79260092 100644 --- a/internal/serialize/keyvalues_test.go +++ b/internal/serialize/keyvalues_test.go @@ -153,106 +153,66 @@ No whitespace.`, func TestDuplicates(t *testing.T) { for name, test := range map[string]struct { - first, second []interface{} - expectedTrimmed [][]interface{} - expectedMerged []interface{} + first, second []interface{} + expected []interface{} }{ - "empty": { - expectedTrimmed: [][]interface{}{{}, {}}, - }, + "empty": {}, "no-duplicates": { - first: makeKV("a", 3), - second: makeKV("b", 3), - expectedTrimmed: [][]interface{}{ - makeKV("a", 3), - makeKV("b", 3), - }, - expectedMerged: append(makeKV("a", 3), makeKV("b", 3)...), + first: makeKV("a", 3), + second: makeKV("b", 3), + expected: append(makeKV("a", 3), makeKV("b", 3)...), }, "all-duplicates": { - first: makeKV("a", 3), - second: makeKV("a", 3), - expectedTrimmed: [][]interface{}{ - {}, - makeKV("a", 3), - }, - expectedMerged: makeKV("a", 3), + first: makeKV("a", 3), + second: makeKV("a", 3), + expected: makeKV("a", 3), }, "start-duplicate": { - first: append([]interface{}{"x", 1}, makeKV("a", 3)...), - second: append([]interface{}{"x", 2}, makeKV("b", 3)...), - expectedTrimmed: [][]interface{}{ - makeKV("a", 3), - append([]interface{}{"x", 2}, makeKV("b", 3)...), - }, - expectedMerged: append(append(makeKV("a", 3), "x", 2), makeKV("b", 3)...), + first: append([]interface{}{"x", 1}, makeKV("a", 3)...), + second: append([]interface{}{"x", 2}, makeKV("b", 3)...), + expected: append(append(makeKV("a", 3), "x", 2), makeKV("b", 3)...), }, "subset-first": { - first: append([]interface{}{"x", 1}, makeKV("a", 3)...), - second: append([]interface{}{"x", 2}, makeKV("a", 3)...), - expectedTrimmed: [][]interface{}{ - {}, - append([]interface{}{"x", 2}, makeKV("a", 3)...), - }, - expectedMerged: append([]interface{}{"x", 2}, makeKV("a", 3)...), + first: append([]interface{}{"x", 1}, makeKV("a", 3)...), + second: append([]interface{}{"x", 2}, makeKV("a", 3)...), + expected: append([]interface{}{"x", 2}, makeKV("a", 3)...), }, "subset-second": { - first: append([]interface{}{"x", 1}, makeKV("a", 1)...), - second: append([]interface{}{"x", 2}, makeKV("b", 2)...), - expectedTrimmed: [][]interface{}{ - makeKV("a", 1), - append([]interface{}{"x", 2}, makeKV("b", 2)...), - }, - expectedMerged: append(append(makeKV("a", 1), "x", 2), makeKV("b", 2)...), + first: append([]interface{}{"x", 1}, makeKV("a", 1)...), + second: append([]interface{}{"x", 2}, makeKV("b", 2)...), + expected: append(append(makeKV("a", 1), "x", 2), makeKV("b", 2)...), }, "end-duplicate": { - first: append(makeKV("a", 3), "x", 1), - second: append(makeKV("b", 3), "x", 2), - expectedTrimmed: [][]interface{}{ - makeKV("a", 3), - append(makeKV("b", 3), "x", 2), - }, - expectedMerged: append(makeKV("a", 3), append(makeKV("b", 3), "x", 2)...), + first: append(makeKV("a", 3), "x", 1), + second: append(makeKV("b", 3), "x", 2), + expected: append(makeKV("a", 3), append(makeKV("b", 3), "x", 2)...), }, "middle-duplicate": { - first: []interface{}{"a-0", 0, "x", 1, "a-1", 2}, - second: []interface{}{"b-0", 0, "x", 2, "b-1", 2}, - expectedTrimmed: [][]interface{}{ - {"a-0", 0, "a-1", 2}, - {"b-0", 0, "x", 2, "b-1", 2}, - }, - expectedMerged: []interface{}{"a-0", 0, "a-1", 2, "b-0", 0, "x", 2, "b-1", 2}, + first: []interface{}{"a-0", 0, "x", 1, "a-1", 2}, + second: []interface{}{"b-0", 0, "x", 2, "b-1", 2}, + expected: []interface{}{"a-0", 0, "a-1", 2, "b-0", 0, "x", 2, "b-1", 2}, }, "internal-duplicates": { first: []interface{}{"a", 0, "x", 1, "a", 2}, second: []interface{}{"b", 0, "x", 2, "b", 2}, - expectedTrimmed: [][]interface{}{ - {"a", 2}, - {"x", 2, "b", 2}, - }, // This is the case where Merged keeps key/value pairs // that were already duplicated inside the slices, for // performance. - expectedMerged: []interface{}{"a", 0, "a", 2, "b", 0, "x", 2, "b", 2}, + expected: []interface{}{"a", 0, "a", 2, "b", 0, "x", 2, "b", 2}, }, } { t.Run(name, func(t *testing.T) { - t.Run("TrimDuplicates", func(t *testing.T) { - actual := serialize.TrimDuplicates(test.first, test.second) - expectEqual(t, "trimmed key/value pairs", test.expectedTrimmed, actual) - }) - t.Run("Merged", func(t *testing.T) { actual := serialize.MergeKVs(test.first, test.second) - expectEqual(t, "merged key/value pairs", test.expectedMerged, actual) + expectEqual(t, "merged key/value pairs", test.expected, actual) }) }) } } -// BenchmarkTrimDuplicates checks performance when TrimDuplicates is called with two slices. +// BenchmarkMergeKVs checks performance when MergeKVs is called with two slices. // In practice that is how the function is used. -func BenchmarkTrimDuplicates(b *testing.B) { +func BenchmarkMergeKVs(b *testing.B) { for firstLength := 0; firstLength < 10; firstLength++ { firstA := makeKV("a", firstLength) for secondLength := 0; secondLength < 10; secondLength++ { @@ -262,7 +222,7 @@ func BenchmarkTrimDuplicates(b *testing.B) { // This is the most common case: all key/value pairs are kept. b.Run("no-duplicates", func(b *testing.B) { expected := append(firstA, secondB...) - benchTrimDuplicates(b, expected, firstA, secondB) + benchMergeKVs(b, expected, firstA, secondB) }) // Fairly unlikely... @@ -272,7 +232,7 @@ func BenchmarkTrimDuplicates(b *testing.B) { expected = firstA[secondLength*2:] } expected = append(expected, secondA...) - benchTrimDuplicates(b, expected, firstA, secondA) + benchMergeKVs(b, expected, firstA, secondA) }) // First entry is the same. @@ -282,7 +242,7 @@ func BenchmarkTrimDuplicates(b *testing.B) { second := []interface{}{"x", 1} second = append(second, secondB...) expected := append(firstA, second...) - benchTrimDuplicates(b, expected, first, second) + benchMergeKVs(b, expected, first, second) }) // Last entry is the same. @@ -292,7 +252,7 @@ func BenchmarkTrimDuplicates(b *testing.B) { second := secondB[:] second = append(second, "x", 1) expected := append(firstA, second...) - benchTrimDuplicates(b, expected, first, second) + benchMergeKVs(b, expected, first, second) }) }) } @@ -310,7 +270,7 @@ func makeKV(prefix string, length int) []interface{} { return kv } -func benchTrimDuplicates(b *testing.B, expected []interface{}, first, second []interface{}) { +func benchMergeKVs(b *testing.B, expected []interface{}, first, second []interface{}) { if len(expected) == 0 { expected = nil }