From 1742fa8f70e05a90ecbf8168f5ae86842f254f98 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 25 Jul 2018 10:15:59 +0000 Subject: [PATCH] Fix WithLatests() fixup on duplicate keys The fixup() method was modifying a copy of its input, so you could get duplicate keys in the output. Thankfully this is rare: in most cases WithLatests() is called with fields that are not duplicates of existing ones. --- report/map_helpers.go | 16 +++++++++------- report/node_test.go | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/report/map_helpers.go b/report/map_helpers.go index 5339dc6c0f..9ccabb6345 100644 --- a/report/map_helpers.go +++ b/report/map_helpers.go @@ -148,19 +148,21 @@ func (m StringLatestMap) Swap(i, j int) { m[i], m[j] = m[j], m[i] } func (m StringLatestMap) Less(i, j int) bool { return m[i].key < m[j].key } // sort entries and shuffle down any duplicates -func (m StringLatestMap) fixup() { - sort.Sort(m) - for i := 1; i < len(m); { - if m[i-1].key == m[i].key { - if m[i-1].Timestamp.Before(m[i].Timestamp) { - m = append(m[:i-1], m[i:]...) +func (m *StringLatestMap) fixup() { + n := *m + sort.Sort(n) + for i := 1; i < len(n); { + if n[i-1].key == n[i].key { + if n[i-1].Timestamp.Before(n[i].Timestamp) { + n = append(n[:i-1], n[i:]...) } else { - m = append(m[:i], m[i+1:]...) + n = append(n[:i], n[i+1:]...) } } else { i++ } } + *m = n } // add several entries at the same timestamp diff --git a/report/node_test.go b/report/node_test.go index f1f2c15ceb..5751fe922c 100644 --- a/report/node_test.go +++ b/report/node_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/weaveworks/common/mtime" "github.com/weaveworks/common/test" "github.com/weaveworks/scope/report" @@ -16,6 +17,22 @@ const ( Domain = "domain" ) +func TestWithLatest(t *testing.T) { + mtime.NowForce(time.Now()) + defer mtime.NowReset() + + latests1 := map[string]string{Name: "x"} + latests2 := map[string]string{PID: "123"} + node1 := report.MakeNode("node1").WithLatests(latests1) + assert.Equal(t, 1, node1.Latest.Len()) + node2 := node1.WithLatests(latests1) + assert.Equal(t, node1, node2) + node3 := node1.WithLatests(latests2) + assert.Equal(t, 2, node3.Latest.Len()) + node4 := node1.WithLatests(latests2) + assert.Equal(t, node3, node4) +} + func TestMergeNodes(t *testing.T) { mtime.NowForce(time.Now()) defer mtime.NowReset()