Fix non-vai sorting involving string arrays, add tests. (#618)

- Bump wrangler to get fix for 'GetValueFromAny'.
  This adds string arrays to the types it supports.

- Use a newer go library sort function to maintain a stable sort

- Use the map-sort-map pattern to avoid repeatedly calculating the
  same underlying value for `object[arg1][arg2][...]`

go.mod

@@ -26,7 +26,7 @@ require (
 	github.com/rancher/lasso v0.2.2
 	github.com/rancher/norman v0.6.0
 	github.com/rancher/remotedialer v0.4.5-rc.1
-	github.com/rancher/wrangler/v3 v3.2.1-rc.5
+	github.com/rancher/wrangler/v3 v3.2.1
 	github.com/sirupsen/logrus v1.9.3
 	github.com/stretchr/testify v1.10.0
 	github.com/urfave/cli/v2 v2.27.5

go.sum

@@ -230,8 +230,8 @@ github.com/rancher/norman v0.6.0 h1:8CyY9cVcw0L+YYZRGvXHPMENefLaDkmi6vpscrVjXew=
 github.com/rancher/norman v0.6.0/go.mod h1:CQ0/1CoEqbeJXvOXuRQvsmiBd7QIgxZG5IvQFOPAHOg=
 github.com/rancher/remotedialer v0.4.5-rc.1 h1:LwGrOqt6hrKjf9Er5LBEKP6xk21RFF7PO2d0+E+mavk=
 github.com/rancher/remotedialer v0.4.5-rc.1/go.mod h1:x31ZR9714VzudfHVke40+WN5wDSDckxjRGr1bWgpgc0=
-github.com/rancher/wrangler/v3 v3.2.1-rc.5 h1:hbhOcF0YyQty7PzTpEQpRdH4DZ74bWDkCp2a4TrUeRA=
+github.com/rancher/wrangler/v3 v3.2.1 h1:V51PnoGb8bZ5jJdxFlqKQApzWdSp4sEy5OPGuEGqVbI=
-github.com/rancher/wrangler/v3 v3.2.1-rc.5/go.mod h1:RV8kkv5br5HaxXWamIbr95pOjvVeoC5CeBldcdw5Fv0=
+github.com/rancher/wrangler/v3 v3.2.1/go.mod h1:RV8kkv5br5HaxXWamIbr95pOjvVeoC5CeBldcdw5Fv0=
 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE=
 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=

pkg/stores/partition/listprocessor/processor.go

@@ -4,6 +4,7 @@ package listprocessor
 import (
 	"fmt"
 	"regexp"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -13,6 +14,7 @@ import (
 	"github.com/rancher/wrangler/v3/pkg/data"
 	"github.com/rancher/wrangler/v3/pkg/data/convert"
 	corecontrollers "github.com/rancher/wrangler/v3/pkg/generated/controllers/core/v1"
+	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -353,26 +355,61 @@ func matchesAll(obj map[string]interface{}, filters []OrFilter) bool {
 }
 
 // SortList sorts the slice by the provided sort criteria.
+// Convert the sorted list so we calculate the sort values only once per node,
+// making this an O(n) operation rather than an O(n**2) one.
+//
+// list of u.U => list of { *u.U, []stringValues } =>
+// sorted list of { *u.U, []stringValues } =>
+// final list of u.U from the above sorted list
+//
+// We do this because it's much faster to compare two string arrays then to compare
+// the string arrays based on walking objects repeatedly
 func SortList(list []unstructured.Unstructured, s Sort) []unstructured.Unstructured {
 	if len(s.Fields) == 0 {
 		return list
 	}
-	sort.Slice(list, func(i, j int) bool {
+	type transformedData struct {
-		leftNode := list[i].Object
+		originalItem *unstructured.Unstructured
-		rightNode := list[j].Object
+		stringValues []string
-		for i, field := range s.Fields {
+	}
-			leftValue := convert.ToString(data.GetValueN(leftNode, field...))
+	transformedList := make([]transformedData, len(list))
-			rightValue := convert.ToString(data.GetValueN(rightNode, field...))
+	for i := range list {
-			if leftValue != rightValue {
+		td := &transformedList[i]
-				if s.Orders[i] == ASC {
+		td.originalItem = &list[i]
-					return leftValue < rightValue
+		td.stringValues = make([]string, len(s.Fields))
-				}
+		for j, fields := range s.Fields {
-				return rightValue < leftValue
+			val, ok := data.GetValueFromAny(list[i].Object, fields...)
+			if ok {
+				td.stringValues[j] = val.(string)
+			} else {
+				logrus.Debugf("Failed to walk list item %d with fields %s", i, fields)
+				td.stringValues[j] = ""
 			}
 		}
-		return false
+	}
-	})
+	// We can't use slices.Compare(leftNode.stringValues, rightNode.stringValues)
-	return list
+	// because some comparisons might use ASC-order, and others DESC-order.
+	sortFunc := func(leftNode, rightNode transformedData) int {
+		for i := range leftNode.stringValues {
+			diff := strings.Compare(leftNode.stringValues[i], rightNode.stringValues[i])
+			if diff == 0 {
+				// the two substrings are the same, so continue walking the arrays
+				continue
+			}
+			if s.Orders[i] == DESC {
+				// reversing sort order just means inverting the comparison value
+				diff *= -1
+			}
+			return diff
+		}
+		return 0
+	}
+	slices.SortStableFunc(transformedList, sortFunc)
+	newList := make([]unstructured.Unstructured, len(list))
+	for i := range transformedList {
+		newList[i] = *transformedList[i].originalItem
+	}
+	return newList
 }
 
 // PaginateList returns a subset of the result based on the pagination criteria as well as the total number of pages the caller can expect.

pkg/stores/partition/listprocessor/processor_test.go

@@ -2362,6 +2362,309 @@ func TestSortList(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "sort metadata.fields[1]",
+			objects: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3", // this is where we expect to see this block after sorting
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position2",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+			},
+			sort: Sort{
+				Fields: [][]string{{"metadata", "fields", "1"}},
+				Orders: []SortOrder{ASC},
+			},
+			want: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position2",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "sort metadata.fields[1] reverse order",
+			objects: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3", // this is where we expect to see this block after sorting, but in reverse
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position2",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+			},
+			sort: Sort{
+				Fields: [][]string{{"metadata", "fields", "1"}},
+				Orders: []SortOrder{DESC},
+			},
+			want: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position2",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "sort metadata.fields[1], preserve ties",
+			objects: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3", // this is where we expect to see this block after sorting
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position1", // but should be second
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+			},
+			sort: Sort{
+				Fields: [][]string{{"metadata", "fields", "1"}},
+				Orders: []SortOrder{ASC},
+			},
+			want: []unstructured.Unstructured{
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "honeycrisp",
+							"fields": []string{
+								"a2",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "granny-smith",
+							"fields": []string{
+								"a3",
+								"position1",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "green",
+						},
+					},
+				},
+				{
+					Object: map[string]interface{}{
+						"kind": "apple",
+						"metadata": map[string]interface{}{
+							"name": "fuji",
+							"fields": []string{
+								"a1",
+								"position3",
+							},
+						},
+						"data": map[string]interface{}{
+							"color": "pink",
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {