diff --git a/pkg/sqlcache/informer/listoption_indexer.go b/pkg/sqlcache/informer/listoption_indexer.go index 682ef3df..412ab9ef 100644 --- a/pkg/sqlcache/informer/listoption_indexer.go +++ b/pkg/sqlcache/informer/listoption_indexer.go @@ -389,26 +389,24 @@ func (l *ListOptionIndexer) constructQuery(lo sqltypes.ListOptions, partitions [ countParams := params[:] // 3- Sorting: ORDER BY clauses (from lo.Sort) - if len(lo.Sort.Fields) != len(lo.Sort.Orders) { - return nil, fmt.Errorf("sort fields length %d != sort orders length %d", len(lo.Sort.Fields), len(lo.Sort.Orders)) - } - if len(lo.Sort.Fields) > 0 { + if len(lo.SortList.SortDirectives) > 0 { orderByClauses := []string{} - for i, field := range lo.Sort.Fields { - if isLabelsFieldList(field) { - clause, sortParam, err := buildSortLabelsClause(field[2], joinTableIndexByLabelName, lo.Sort.Orders[i] == sqltypes.ASC) + for _, sortDirective := range lo.SortList.SortDirectives { + fields := sortDirective.Fields + if isLabelsFieldList(fields) { + clause, sortParam, err := buildSortLabelsClause(fields[2], joinTableIndexByLabelName, sortDirective.Order == sqltypes.ASC) if err != nil { return nil, err } orderByClauses = append(orderByClauses, clause) params = append(params, sortParam) } else { - columnName := toColumnName(field) + columnName := toColumnName(fields) if err := l.validateColumn(columnName); err != nil { return queryInfo, err } direction := "ASC" - if lo.Sort.Orders[i] == sqltypes.DESC { + if sortDirective.Order == sqltypes.DESC { direction = "DESC" } orderByClauses = append(orderByClauses, fmt.Sprintf(`f."%s" %s`, columnName, direction)) @@ -596,13 +594,14 @@ func buildSortLabelsClause(labelName string, joinTableIndexByLabelName map[strin // created in Store.ListByPartitions, and that ends up calling ListOptionIndexer.ConstructQuery. // No other goroutines access this object. func ensureSortLabelsAreSelected(lo *sqltypes.ListOptions) { - if len(lo.Sort.Fields) == 0 { + if len(lo.SortList.SortDirectives) == 0 { return } unboundSortLabels := make(map[string]bool) - for _, fieldList := range lo.Sort.Fields { - if isLabelsFieldList(fieldList) { - unboundSortLabels[fieldList[2]] = true + for _, sortDirective := range lo.SortList.SortDirectives { + fields := sortDirective.Fields + if isLabelsFieldList(fields) { + unboundSortLabels[fields[2]] = true } } if len(unboundSortLabels) == 0 { diff --git a/pkg/sqlcache/informer/listoption_indexer_test.go b/pkg/sqlcache/informer/listoption_indexer_test.go index 1c31f062..16255ecd 100644 --- a/pkg/sqlcache/informer/listoption_indexer_test.go +++ b/pkg/sqlcache/informer/listoption_indexer_test.go @@ -647,9 +647,13 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "ListByOptions with only one Sort.Field set should sort on that field only, in ascending order in prepared sql.Stmt", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "somefield"}, + Order: sqltypes.ASC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -670,9 +674,13 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "sort one field descending", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "somefield"}, + Order: sqltypes.DESC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -693,9 +701,13 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "sort one unbound label descending", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "labels", "flip"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "labels", "flip"}, + Order: sqltypes.DESC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -718,9 +730,17 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "ListByOptions sorting on two complex fields should sort on the first field in ascending order first and then sort on the second labels field in ascending order in prepared sql.Stmt", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "fields", "3"}, {"metadata", "labels", "stub.io/candy"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC, sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "fields", "3"}, + Order: sqltypes.ASC, + }, + { + Fields: []string{"metadata", "labels", "stub.io/candy"}, + Order: sqltypes.ASC, + }, + }, }, }, extraIndexedFields: []string{"metadata.fields[3]", "metadata.labels[stub.io/candy]"}, @@ -742,9 +762,17 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "ListByOptions sorting on two fields should sort on the first field in ascending order first and then sort on the second field in ascending order in prepared sql.Stmt", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}, {"status", "someotherfield"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC, sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "somefield"}, + Order: sqltypes.ASC, + }, + { + Fields: []string{"status", "someotherfield"}, + Order: sqltypes.ASC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -763,9 +791,17 @@ func TestListByOptions(t *testing.T) { tests = append(tests, testCase{ description: "ListByOptions sorting on two fields should sort on the first field in descending order first and then sort on the second field in ascending order in prepared sql.Stmt", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}, {"status", "someotherfield"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC, sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "somefield"}, + Order: sqltypes.DESC, + }, + { + Fields: []string{"status", "someotherfield"}, + Order: sqltypes.ASC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -781,27 +817,6 @@ func TestListByOptions(t *testing.T) { expectedErr: nil, }) - tests = append(tests, testCase{ - description: "ListByOptions sorting when # fields != # sort orders should return an error", - listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}, {"status", "someotherfield"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC, sqltypes.ASC, sqltypes.ASC}, - }, - }, - partitions: []partition.Partition{}, - ns: "", - expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o - JOIN "something_fields" f ON o.key = f.key - WHERE - (FALSE) - ORDER BY f."metadata.somefield" DESC, f."status.someotherfield" ASC`, - returnList: []any{&unstructured.Unstructured{Object: unstrTestObjectMap}, &unstructured.Unstructured{Object: unstrTestObjectMap}}, - expectedList: &unstructured.UnstructuredList{Object: map[string]interface{}{"items": []map[string]interface{}{unstrTestObjectMap, unstrTestObjectMap}}, Items: []unstructured.Unstructured{{Object: unstrTestObjectMap}, {Object: unstrTestObjectMap}}}, - expectedContToken: "", - expectedErr: fmt.Errorf("sort fields length 2 != sort orders length 3"), - }) - tests = append(tests, testCase{ description: "ListByOptions with Pagination.PageSize set should set limit to PageSize in prepared sql.Stmt", listOptions: sqltypes.ListOptions{ @@ -1548,9 +1563,13 @@ func TestConstructQuery(t *testing.T) { }, }, }, - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "queryField1"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "queryField1"}, + Order: sqltypes.ASC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -1566,27 +1585,16 @@ func TestConstructQuery(t *testing.T) { expectedErr: nil, }) - tests = append(tests, testCase{ - description: "ConstructQuery: sorting when # fields < # sort orders should return an error", - listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}, {"status", "someotherfield"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC, sqltypes.ASC, sqltypes.ASC}, - }, - }, - partitions: []partition.Partition{}, - ns: "", - expectedStmt: "", - expectedStmtArgs: []any{}, - expectedErr: fmt.Errorf("sort fields length 2 != sort orders length 3"), - }) - tests = append(tests, testCase{ description: "TestConstructQuery: sort on label statements with no query", listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "labels", "this"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "labels", "this"}, + Order: sqltypes.ASC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -1621,9 +1629,17 @@ func TestConstructQuery(t *testing.T) { }, }, }, - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "labels", "this"}, {"status", "queryField2"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC, sqltypes.DESC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "labels", "this"}, + Order: sqltypes.ASC, + }, + { + Fields: []string{"status", "queryField2"}, + Order: sqltypes.DESC, + }, + }, }, }, partitions: []partition.Partition{}, @@ -1640,21 +1656,6 @@ func TestConstructQuery(t *testing.T) { expectedErr: nil, }) - tests = append(tests, testCase{ - description: "ConstructQuery: sorting when # fields > # sort orders should return an error", - listOptions: sqltypes.ListOptions{ - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "somefield"}, {"status", "someotherfield"}, {"metadata", "labels", "a1"}, {"metadata", "labels", "a2"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC, sqltypes.ASC, sqltypes.ASC}, - }, - }, - partitions: []partition.Partition{}, - ns: "", - expectedStmt: "", - expectedStmtArgs: []any{}, - expectedErr: fmt.Errorf("sort fields length 4 != sort orders length 3"), - }) - t.Parallel() for _, test := range tests { t.Run(test.description, func(t *testing.T) { diff --git a/pkg/sqlcache/sqltypes/types.go b/pkg/sqlcache/sqltypes/types.go index a90a1311..6c052648 100644 --- a/pkg/sqlcache/sqltypes/types.go +++ b/pkg/sqlcache/sqltypes/types.go @@ -28,7 +28,7 @@ type ListOptions struct { ChunkSize int Resume string Filters []OrFilter - Sort Sort + SortList SortList Pagination Pagination } @@ -57,8 +57,8 @@ type OrFilter struct { // The order is represented by prefixing the sort key by '-', e.g. sort=-metadata.name. // e.g. To sort internal clusters first followed by clusters in alpha order: sort=-spec.internal,spec.displayName type Sort struct { - Fields [][]string - Orders []SortOrder + Fields []string + Order SortOrder } type SortList struct { diff --git a/pkg/stores/sqlpartition/listprocessor/processor.go b/pkg/stores/sqlpartition/listprocessor/processor.go index e60bdc8b..f847e7d8 100644 --- a/pkg/stores/sqlpartition/listprocessor/processor.go +++ b/pkg/stores/sqlpartition/listprocessor/processor.go @@ -50,15 +50,6 @@ var mapK8sOpToRancherOp = map[selection.Operator]sqltypes.Op{ selection.GreaterThan: sqltypes.Gt, } -// ListOptions represents the query parameters that may be included in a list request. -type ListOptions struct { - ChunkSize int - Resume string - Filters []sqltypes.OrFilter - Sort sqltypes.Sort - Pagination sqltypes.Pagination -} - type Cache interface { // ListByOptions returns objects according to the specified list options and partitions. // Specifically: @@ -118,9 +109,9 @@ func ParseQuery(apiOp *types.APIRequest, namespaceCache Cache) (sqltypes.ListOpt } opts.Filters = filterOpts - sortOpts := sqltypes.Sort{} sortKeys := q.Get(sortParam) if sortKeys != "" { + sortList := *sqltypes.NewSortList() sortParts := strings.Split(sortKeys, ",") for _, sortPart := range sortParts { field := sortPart @@ -131,13 +122,16 @@ func ParseQuery(apiOp *types.APIRequest, namespaceCache Cache) (sqltypes.ListOpt field = field[1:] } if len(field) > 0 { - sortOpts.Fields = append(sortOpts.Fields, queryhelper.SafeSplit(field)) - sortOpts.Orders = append(sortOpts.Orders, sortOrder) + sortDirective := sqltypes.Sort{ + Fields: queryhelper.SafeSplit(field), + Order: sortOrder, + } + sortList.SortDirectives = append(sortList.SortDirectives, sortDirective) } } } + opts.SortList = sortList } - opts.Sort = sortOpts var err error pagination := sqltypes.Pagination{} diff --git a/pkg/stores/sqlpartition/listprocessor/processor_test.go b/pkg/stores/sqlpartition/listprocessor/processor_test.go index 59280401..8048dd1e 100644 --- a/pkg/stores/sqlpartition/listprocessor/processor_test.go +++ b/pkg/stores/sqlpartition/listprocessor/processor_test.go @@ -744,10 +744,13 @@ func TestParseQuery(t *testing.T) { }, expectedLO: sqltypes.ListOptions{ ChunkSize: defaultLimit, - Sort: sqltypes.Sort{ - Fields: [][]string{ - {"metadata", "name"}}, - Orders: []sqltypes.SortOrder{sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "name"}, + Order: sqltypes.ASC, + }, + }, }, Filters: make([]sqltypes.OrFilter, 0), Pagination: sqltypes.Pagination{ @@ -765,9 +768,13 @@ func TestParseQuery(t *testing.T) { }, expectedLO: sqltypes.ListOptions{ ChunkSize: defaultLimit, - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "name"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "name"}, + Order: sqltypes.DESC, + }, + }, }, Filters: make([]sqltypes.OrFilter, 0), Pagination: sqltypes.Pagination{ @@ -785,14 +792,16 @@ func TestParseQuery(t *testing.T) { }, expectedLO: sqltypes.ListOptions{ ChunkSize: defaultLimit, - Sort: sqltypes.Sort{ - Fields: [][]string{ - {"metadata", "name"}, - {"spec", "something"}, - }, - Orders: []sqltypes.SortOrder{ - sqltypes.DESC, - sqltypes.ASC, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "name"}, + Order: sqltypes.DESC, + }, + { + Fields: []string{"spec", "something"}, + Order: sqltypes.ASC, + }, }, }, Filters: make([]sqltypes.OrFilter, 0), @@ -811,12 +820,25 @@ func TestParseQuery(t *testing.T) { }, expectedLO: sqltypes.ListOptions{ ChunkSize: defaultLimit, - Sort: sqltypes.Sort{ - Fields: [][]string{{"metadata", "labels", "beef.cattle.io/snort"}, - {"metadata", "labels", "steer"}, - {"metadata", "labels", "bossie.cattle.io/moo"}, - {"spec", "something"}}, - Orders: []sqltypes.SortOrder{sqltypes.DESC, sqltypes.ASC, sqltypes.ASC, sqltypes.ASC}, + SortList: sqltypes.SortList{ + SortDirectives: []sqltypes.Sort{ + { + Fields: []string{"metadata", "labels", "beef.cattle.io/snort"}, + Order: sqltypes.DESC, + }, + { + Fields: []string{"metadata", "labels", "steer"}, + Order: sqltypes.ASC, + }, + { + Fields: []string{"metadata", "labels", "bossie.cattle.io/moo"}, + Order: sqltypes.ASC, + }, + { + Fields: []string{"spec", "something"}, + Order: sqltypes.ASC, + }, + }, }, Filters: make([]sqltypes.OrFilter, 0), Pagination: sqltypes.Pagination{