From ba656a37f792d8f0d9554826a35115bd6dcff27b Mon Sep 17 00:00:00 2001
From: Bryan Boreham <bjboreham@gmail.com>
Date: Wed, 25 Jul 2018 10:15:59 +0000
Subject: [PATCH] Fix WithLatests() de-duplication

The fixup() method was modifying a copy of its input, so you could get
duplicate keys in the output. Change it to return the new slice.

Thankfully this is rare: in most cases WithLatests() is called with
fields that are not duplicates of existing ones.
---
 report/map_helpers.go | 11 +++++------
 report/node_test.go   | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/report/map_helpers.go b/report/map_helpers.go
index 5339dc6c0f..169f953f49 100644
--- a/report/map_helpers.go
+++ b/report/map_helpers.go
@@ -147,8 +147,8 @@ func (m StringLatestMap) Len() int           { return len(m) }
 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 entries and shuffle down any duplicates. NOTE: may modify contents of m.
+func (m StringLatestMap) sortedAndDeduplicated() StringLatestMap {
 	sort.Sort(m)
 	for i := 1; i < len(m); {
 		if m[i-1].key == m[i].key {
@@ -161,6 +161,7 @@ func (m StringLatestMap) fixup() {
 			i++
 		}
 	}
+	return m
 }
 
 // add several entries at the same timestamp
@@ -170,8 +171,7 @@ func (m StringLatestMap) addMapEntries(ts time.Time, n map[string]string) String
 	for k, v := range n {
 		out = append(out, stringLatestEntry{key: k, Value: v, Timestamp: ts})
 	}
-	out.fixup()
-	return out
+	return out.sortedAndDeduplicated()
 }
 
 // Propagate a set of latest values from one set to another.
@@ -183,6 +183,5 @@ func (m StringLatestMap) Propagate(from StringLatestMap, keys ...string) StringL
 			out = append(out, stringLatestEntry{key: k, Value: v, Timestamp: ts})
 		}
 	}
-	out.fixup()
-	return out
+	return out.sortedAndDeduplicated()
 }
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()