fix: sorting quota (#19538)

fix: sort Project Quotas

Signed-off-by: Shengwen Yu <yshengwen@vmware.com>

api/v2.0/swagger.yaml

@@ -6175,7 +6175,7 @@ parameters:
     required: false
   sort:
     name: sort
-    description: Sort the resource list in ascending or descending order. e.g. sort by field1 in ascending orderr and field2 in descending order with "sort=field1,-field2"
+    description: Sort the resource list in ascending or descending order. e.g. sort by field1 in ascending order and field2 in descending order with "sort=field1,-field2"
     in: query
     type: string
     required: false

src/lib/q/builder.go

@@ -38,7 +38,7 @@ func Build(q, sort string, pageNumber, pageSize int64) (*Query, error) {
 	if err != nil {
 		return nil, err
 	}
-	sorts := parseSorting(sort)
+	sorts := ParseSorting(sort)
 	return &Query{
 		Keywords:   keywords,
 		Sorts:      sorts,
@@ -77,7 +77,10 @@ func parseKeywords(q string) (map[string]interface{}, error) {
 	return keywords, nil
 }
 
-func parseSorting(sort string) []*Sort {
+func ParseSorting(sort string) []*Sort {
+	if sort == "" {
+		return []*Sort{}
+	}
 	var sorts []*Sort
 	for _, sorting := range strings.Split(sort, ",") {
 		key := sorting

src/lib/q/builder_test.go

@@ -15,6 +15,7 @@
 package q
 
 import (
+	"reflect"
 	"testing"
 	"time"
 
@@ -266,3 +267,60 @@ func TestParseKeywords(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, "tags=nil", keywords["q"].(string))
 }
+
+func TestParseSorting(t *testing.T) {
+	testCases := []struct {
+		description string
+		sortQuery   string
+		expectRet   []*Sort
+	}{
+		{
+			description: "sortQuery is empty",
+			sortQuery:   "",
+			expectRet:   []*Sort{},
+		},
+		{
+			description: "ascending without the hyphen",
+			sortQuery:   "key1",
+			expectRet: []*Sort{
+				{
+					Key:  "key1",
+					DESC: false,
+				},
+			},
+		},
+		{
+			description: "descending with the hyphen",
+			sortQuery:   "-key1",
+			expectRet: []*Sort{
+				{
+					Key:  "key1",
+					DESC: true,
+				},
+			},
+		},
+		{
+			description: "ascending without the hyphen and descending with hyphen",
+			sortQuery:   "key1,-key2",
+			expectRet: []*Sort{
+				{
+					Key:  "key1",
+					DESC: false,
+				},
+				{
+					Key:  "key2",
+					DESC: true,
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			got := ParseSorting(tc.sortQuery)
+			if !reflect.DeepEqual(got, tc.expectRet) {
+				t.Errorf("ParseSorting() = %#v, want %#v", got, tc.expectRet)
+			}
+		})
+	}
+}

src/pkg/quota/dao/dao_test.go

@@ -220,7 +220,7 @@ func (suite *DaoTestSuite) TestList() {
 
 	{
 		// List quotas by sorting
-		quotas, err := suite.dao.List(ctx, &q.Query{Keywords: q.KeyWords{"reference": reference}, Sorting: "-hard.storage"})
+		quotas, err := suite.dao.List(ctx, &q.Query{Keywords: q.KeyWords{"reference": reference}, Sorts: q.ParseSorting("-hard.storage")})
 		suite.Nil(err)
 		suite.Equal(reference, quotas[0].Reference)
 		suite.Equal("1", quotas[0].ReferenceID)

src/pkg/quota/dao/util.go

@@ -86,32 +86,45 @@ func castQuantity(field string) string {
 
 func listOrderBy(query *q.Query) string {
 	orderBy := "b.creation_time DESC"
+	if query == nil {
+		return orderBy
+	}
 
-	if query != nil && query.Sorting != "" {
-		if val, ok := quotaOrderMap[query.Sorting]; ok {
-			orderBy = val
-		} else {
-			sort := query.Sorting
+	var orderBySlice []string
+	for _, sortByItem := range query.Sorts {
+		sortKey := ""
+		if sortByItem.DESC {
+			sortKey = "-"
+		}
+		sortKey = sortKey + sortByItem.Key
 
-			order := "ASC"
-			if sort[0] == '-' {
-				order = "DESC"
-				sort = sort[1:]
-			}
+		// check if sortKey is in quotaOrderMap
+		if val, ok := quotaOrderMap[sortKey]; ok {
+			orderBySlice = append(orderBySlice, val)
+			continue
+		}
 
-			prefixes := []string{"hard.", "used."}
-			for _, prefix := range prefixes {
-				if strings.HasPrefix(sort, prefix) {
-					resource := strings.TrimPrefix(sort, prefix)
-					if types.IsValidResource(types.ResourceName(resource)) {
-						field := fmt.Sprintf("%s->>%s", strings.TrimSuffix(prefix, "."), orm.QuoteLiteral(resource))
-						orderBy = fmt.Sprintf("(%s) %s", castQuantity(field), order)
-						break
-					}
+		// now: check SortByItem against "hard.resource_name", "-hard.resource_name", "used.resource_name", "-used.resource_name"
+		order := "ASC"
+		if sortByItem.DESC {
+			order = "DESC"
+		}
+		prefixes := []string{"hard.", "used."}
+		for _, prefix := range prefixes {
+			if strings.HasPrefix(sortByItem.Key, prefix) {
+				resource := strings.TrimPrefix(sortByItem.Key, prefix)
+				if types.IsValidResource(types.ResourceName(resource)) {
+					field := fmt.Sprintf("%s->>%s", strings.TrimSuffix(prefix, "."), orm.QuoteLiteral(resource))
+					orderBy = fmt.Sprintf("(%s) %s", castQuantity(field), order)
+					orderBySlice = append(orderBySlice, orderBy)
 				}
 			}
 		}
 	}
 
-	return orderBy
+	if len(orderBySlice) == 0 {
+		return orderBy
+	}
+
+	return strings.Join(orderBySlice, ", ")
 }

src/pkg/quota/dao/util_test.go

@@ -23,7 +23,7 @@ import (
 func Test_listOrderBy(t *testing.T) {
 	query := func(sort string) *q.Query {
 		return &q.Query{
-			Sorting: sort,
+			Sorts: q.ParseSorting(sort),
 		}
 	}
 
@@ -36,11 +36,13 @@ func Test_listOrderBy(t *testing.T) {
 		want string
 	}{
 		{"no query", args{nil}, "b.creation_time DESC"},
+		{"empty query", args{query("")}, "b.creation_time DESC"},
 		{"order by unsupported field", args{query("unknown")}, "b.creation_time DESC"},
 		{"order by storage of hard", args{query("hard.storage")}, "(CAST( (CASE WHEN (hard->>'storage') IS NULL THEN '0' WHEN (hard->>'storage') = '-1' THEN '9223372036854775807' ELSE (hard->>'storage') END) AS BIGINT )) ASC"},
 		{"order by unsupported hard resource", args{query("hard.unknown")}, "b.creation_time DESC"},
 		{"order by storage of used", args{query("used.storage")}, "(CAST( (CASE WHEN (used->>'storage') IS NULL THEN '0' WHEN (used->>'storage') = '-1' THEN '9223372036854775807' ELSE (used->>'storage') END) AS BIGINT )) ASC"},
 		{"order by unsupported used resource", args{query("used.unknown")}, "b.creation_time DESC"},
+		{"order by multiple fields", args{query("-hard.storage,used.storage")}, "(CAST( (CASE WHEN (hard->>'storage') IS NULL THEN '0' WHEN (hard->>'storage') = '-1' THEN '9223372036854775807' ELSE (hard->>'storage') END) AS BIGINT )) DESC, (CAST( (CASE WHEN (used->>'storage') IS NULL THEN '0' WHEN (used->>'storage') = '-1' THEN '9223372036854775807' ELSE (used->>'storage') END) AS BIGINT )) ASC"},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {