From f018364f9138f283d6206566558ac766d017f98d Mon Sep 17 00:00:00 2001
From: Matt Butcher <technosophos@gmail.com>
Date: Wed, 1 Feb 2017 17:17:49 -0700
Subject: [PATCH] fix(helm): fix sort order on helm search

During search index construction, records were not correctly sorted by
version number, which resulted in the wrong records being inserted into
the index.

Along the way, added tests and fixed a few comments.

Closes #1897
---
 cmd/helm/search/search.go      |  5 ++++-
 cmd/helm/search/search_test.go | 30 +++++++++++++++++++++++++++---
 cmd/helm/search_test.go        |  4 ++--
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/cmd/helm/search/search.go b/cmd/helm/search/search.go
index 4d394e0f5..828ebeeb5 100644
--- a/cmd/helm/search/search.go
+++ b/cmd/helm/search/search.go
@@ -62,6 +62,7 @@ const verSep = "$$"
 
 // AddRepo adds a repository index to the search index.
 func (i *Index) AddRepo(rname string, ind *repo.IndexFile, all bool) {
+	ind.SortEntries()
 	for name, ref := range ind.Entries {
 		if len(ref) == 0 {
 			// Skip chart names that have zero releases.
@@ -175,7 +176,7 @@ func (i *Index) SearchRegexp(re string, threshold int) ([]*Result, error) {
 	return buf, nil
 }
 
-// Chart returns the ChartRef for a particular name.
+// Chart returns the ChartVersion for a particular name.
 func (i *Index) Chart(name string) (*repo.ChartVersion, error) {
 	c, ok := i.charts[name]
 	if !ok {
@@ -220,6 +221,8 @@ func (s scoreSorter) Less(a, b int) bool {
 		if err != nil {
 			return true
 		}
+		// Sort so that the newest chart is higher than the oldest chart. This is
+		// the opposite of what you'd expect in a function called Less.
 		return v1.GreaterThan(v2)
 	}
 	return first.Name < second.Name
diff --git a/cmd/helm/search/search_test.go b/cmd/helm/search/search_test.go
index a57865031..5f09f5876 100644
--- a/cmd/helm/search/search_test.go
+++ b/cmd/helm/search/search_test.go
@@ -26,14 +26,16 @@ import (
 
 func TestSortScore(t *testing.T) {
 	in := []*Result{
-		{Name: "bbb", Score: 0},
+		{Name: "bbb", Score: 0, Chart: &repo.ChartVersion{Metadata: &chart.Metadata{Version: "1.2.3"}}},
 		{Name: "aaa", Score: 5},
 		{Name: "abb", Score: 5},
 		{Name: "aab", Score: 0},
 		{Name: "bab", Score: 5},
+		{Name: "ver", Score: 5, Chart: &repo.ChartVersion{Metadata: &chart.Metadata{Version: "1.2.4"}}},
+		{Name: "ver", Score: 5, Chart: &repo.ChartVersion{Metadata: &chart.Metadata{Version: "1.2.3"}}},
 	}
-	expect := []string{"aab", "bbb", "aaa", "abb", "bab"}
-	expectScore := []int{0, 0, 5, 5, 5}
+	expect := []string{"aab", "bbb", "aaa", "abb", "bab", "ver", "ver"}
+	expectScore := []int{0, 0, 5, 5, 5, 5, 5}
 	SortScore(in)
 
 	// Test Score
@@ -48,6 +50,14 @@ func TestSortScore(t *testing.T) {
 			t.Errorf("Sort error: expected %s, got %s", expect[i], in[i].Name)
 		}
 	}
+
+	// Test version of last two items
+	if in[5].Chart.Version != "1.2.4" {
+		t.Errorf("Expected 1.2.4, got %s", in[5].Chart.Version)
+	}
+	if in[6].Chart.Version != "1.2.3" {
+		t.Error("Expected 1.2.3 to be last")
+	}
 }
 
 var indexfileEntries = map[string]repo.ChartVersions{
@@ -123,6 +133,20 @@ func TestAll(t *testing.T) {
 	}
 }
 
+func TestAddRepo_Sort(t *testing.T) {
+	i := loadTestIndex(t, true)
+	sr, err := i.Search("testing/santa-maria", 100, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ch := sr[0]
+	expect := "1.2.3"
+	if ch.Chart.Version != expect {
+		t.Errorf("Expected %q, got %q", expect, ch.Chart.Version)
+	}
+}
+
 func TestSearchByName(t *testing.T) {
 
 	tests := []struct {
diff --git a/cmd/helm/search_test.go b/cmd/helm/search_test.go
index b81a3536d..f53480060 100644
--- a/cmd/helm/search_test.go
+++ b/cmd/helm/search_test.go
@@ -39,7 +39,7 @@ func TestSearchCmd(t *testing.T) {
 		{
 			name:   "search for 'alpine', expect two matches",
 			args:   []string{"alpine"},
-			expect: "NAME          \tVERSION\tDESCRIPTION                    \ntesting/alpine\t0.1.0  \tDeploy a basic Alpine Linux pod",
+			expect: "NAME          \tVERSION\tDESCRIPTION                    \ntesting/alpine\t0.2.0  \tDeploy a basic Alpine Linux pod",
 		},
 		{
 			name:   "search for 'alpine' with versions, expect three matches",
@@ -56,7 +56,7 @@ func TestSearchCmd(t *testing.T) {
 			name:   "search for 'alp[a-z]+', expect two matches",
 			args:   []string{"alp[a-z]+"},
 			flags:  []string{"--regexp"},
-			expect: "NAME          \tVERSION\tDESCRIPTION                    \ntesting/alpine\t0.1.0  \tDeploy a basic Alpine Linux pod",
+			expect: "NAME          \tVERSION\tDESCRIPTION                    \ntesting/alpine\t0.2.0  \tDeploy a basic Alpine Linux pod",
 			regexp: true,
 		},
 		{