From 09673426e3faa58f7b0dc51cf9950d4f133fd6dd Mon Sep 17 00:00:00 2001
From: Pierre Souchay <p.souchay@criteo.com>
Date: Wed, 25 Nov 2020 21:01:07 +0100
Subject: [PATCH] Applied suggestions from @dnephin

* Renamed `cachedHealResultSorter` into `sortCheckServiceNodes`
* Use `<` instead of `strings.Compare`
* Single line comparison in unit test
---
 agent/cache-types/streaming_health_services.go     | 14 ++++++--------
 .../cache-types/streaming_health_services_test.go  |  8 +++-----
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/agent/cache-types/streaming_health_services.go b/agent/cache-types/streaming_health_services.go
index dd6b4bc80..efb1dba87 100644
--- a/agent/cache-types/streaming_health_services.go
+++ b/agent/cache-types/streaming_health_services.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
-	"strings"
 	"time"
 
 	"github.com/hashicorp/go-bexpr"
@@ -208,18 +207,17 @@ func (noopFilterEvaluator) Evaluate(_ interface{}) (bool, error) {
 	return true, nil
 }
 
-// cachedHealResultSorter sorts the results to match memdb semantics
+// sortCheckServiceNodes sorts the results to match memdb semantics
 // Sort results by Node.Node, if 2 instances match, order by Service.ID
 // Will allow result to be stable sorted and match queries without cache
-func cachedHealResultSorter(serviceNodes *structs.IndexedCheckServiceNodes) {
+func sortCheckServiceNodes(serviceNodes *structs.IndexedCheckServiceNodes) {
 	sort.SliceStable(serviceNodes.Nodes, func(i, j int) bool {
 		left := serviceNodes.Nodes[i]
 		right := serviceNodes.Nodes[j]
-		res := strings.Compare(left.Node.Node, right.Node.Node)
-		if res != 0 {
-			return res < 0
+		if left.Node.Node == right.Node.Node {
+			return left.Service.ID < right.Service.ID
 		}
-		return strings.Compare(left.Service.ID, right.Service.ID) < 0
+		return left.Node.Node < right.Node.Node
 	})
 }
 
@@ -234,7 +232,7 @@ func (s *healthView) Result(index uint64) (interface{}, error) {
 	for _, node := range s.state {
 		result.Nodes = append(result.Nodes, node)
 	}
-	cachedHealResultSorter(&result)
+	sortCheckServiceNodes(&result)
 
 	return &result, nil
 }
diff --git a/agent/cache-types/streaming_health_services_test.go b/agent/cache-types/streaming_health_services_test.go
index 570aa8e52..b2581380f 100644
--- a/agent/cache-types/streaming_health_services_test.go
+++ b/agent/cache-types/streaming_health_services_test.go
@@ -273,11 +273,9 @@ func TestOrderingConsistentWithMemDb(t *testing.T) {
 			Index: index,
 		},
 	}
-	cachedHealResultSorter(&result)
-	require.Equal(t, result.Nodes[0], zero)
-	require.Equal(t, result.Nodes[1], one)
-	require.Equal(t, result.Nodes[2], two)
-	require.Equal(t, result.Nodes[3], three)
+	sortCheckServiceNodes(&result)
+	expected := structs.CheckServiceNodes{zero, one, two, three}
+	require.Equal(t, expected, result.Nodes)
 }
 
 func TestStreamingHealthServices_FullSnapshot(t *testing.T) {