Refactor tie-breakers in sortfilter

Author: christeredvartsen

internal/graph/sortfilter/sortfilter.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"slices"
+	"strings"
 
 	"github.com/nais/api/internal/graph/model"
 	"github.com/sirupsen/logrus"
@@ -23,27 +24,27 @@ type ConcurrentSortFunc[T any] func(ctx context.Context, a T) int
 // Filter is a function that returns true if the given value should be included in the result.
 type Filter[T any, FilterObj any] func(ctx context.Context, v T, filter FilterObj) bool
 
-// TieBreaker is a combination of a SortField and a direction that might be able to resolve equal fields during sorting.
+// tieBreaker is a combination of a SortField and a direction that might be able to resolve equal fields during sorting.
 // If the direction is not supplied, the direction used for the original sort will be used. The referenced field must be
 // registered with RegisterSort (concurrent tie-break sorters are not supported).
-type TieBreaker[SortField comparable] struct {
-	Field     SortField
-	Direction *model.OrderDirection
+type tieBreaker[SortField ~string] struct {
+	field     SortField
+	direction model.OrderDirection
 }
 
-type funcs[T any, SortField comparable] struct {
+type funcs[T any, SortField ~string] struct {
 	concurrentSort ConcurrentSortFunc[T]
 	sort           SortFunc[T]
-	tieBreakers    []TieBreaker[SortField]
+	tieBreakers    []tieBreaker[SortField]
 }
 
-type SortFilter[T any, SortField comparable, FilterObj comparable] struct {
+type SortFilter[T any, SortField ~string, FilterObj comparable] struct {
 	sorters map[SortField]funcs[T, SortField]
 	filters []Filter[T, FilterObj]
 }
 
 // New creates a new SortFilter
-func New[T any, SortField comparable, FilterObj comparable]() *SortFilter[T, SortField, FilterObj] {
+func New[T any, SortField ~string, FilterObj comparable]() *SortFilter[T, SortField, FilterObj] {
 	return &SortFilter[T, SortField, FilterObj]{
 		sorters: make(map[SortField]funcs[T, SortField]),
 	}
@@ -55,29 +56,59 @@ func (s *SortFilter[T, SortField, FilterObj]) SupportsSort(field SortField) bool
 	return exists
 }
 
+// sortFieldsToTieBreakers takes a slice of SortField values and returns a list of tie-breakers. The values can be
+// suffixed with :ASC or :DESC to specify the direction. If no direction is supplied, ASC is assumed.
+func sortFieldsToTieBreakers[SortField ~string](fields []SortField) []tieBreaker[SortField] {
+	ret := make([]tieBreaker[SortField], len(fields))
+	for i, field := range fields {
+		direction := model.OrderDirectionAsc
+		if parts := strings.Split(string(field), ":"); len(parts) == 2 {
+			if parts[1] != "ASC" && parts[1] != "DESC" {
+				panic(fmt.Sprintf("invalid direction in sort field: %q", field))
+			}
+
+			direction = model.OrderDirection(parts[1])
+			field = SortField(parts[0])
+		} else if len(parts) > 2 {
+			panic(fmt.Sprintf("invalid sort field: %q", field))
+		}
+
+		ret[i] = tieBreaker[SortField]{
+			field:     field,
+			direction: direction,
+		}
+	}
+
+	return ret
+}
+
+func tieBreakerToSortField[SortField ~string](tb tieBreaker[SortField]) SortField {
+	return tb.field + ":" + SortField(tb.direction)
+}
+
 // RegisterSort will add support for sorting on a specific field. Optional tie-breakers can be supplied to resolve equal
 // values, and will be executed in the given order.
-func (s *SortFilter[T, SortField, FilterObj]) RegisterSort(field SortField, sort SortFunc[T], tieBreakers ...TieBreaker[SortField]) {
+func (s *SortFilter[T, SortField, FilterObj]) RegisterSort(field SortField, sort SortFunc[T], tieBreakers ...SortField) {
 	if _, ok := s.sorters[field]; ok {
 		panic(fmt.Sprintf("sort field is already registered: %v", field))
 	}
 
 	s.sorters[field] = funcs[T, SortField]{
 		sort:        sort,
-		tieBreakers: tieBreakers,
+		tieBreakers: sortFieldsToTieBreakers(tieBreakers),
 	}
 }
 
 // RegisterConcurrentSort will add support for doing concurrent sorting on a specific field. Optional tie-breakers can
 // be supplied to resolve equal values, and will be executed in the given order.
-func (s *SortFilter[T, SortField, FilterObj]) RegisterConcurrentSort(field SortField, sort ConcurrentSortFunc[T], tieBreakers ...TieBreaker[SortField]) {
+func (s *SortFilter[T, SortField, FilterObj]) RegisterConcurrentSort(field SortField, sort ConcurrentSortFunc[T], tieBreakers ...SortField) {
 	if _, ok := s.sorters[field]; ok {
 		panic(fmt.Sprintf("sort field is already registered: %v", field))
 	}
 
 	s.sorters[field] = funcs[T, SortField]{
 		concurrentSort: sort,
-		tieBreakers:    tieBreakers,
+		tieBreakers:    sortFieldsToTieBreakers(tieBreakers),
 	}
 }
 
@@ -139,14 +170,14 @@ func (s *SortFilter[T, SortField, FilterObj]) Sort(ctx context.Context, items []
 	}
 
 	if sorter.concurrentSort != nil {
-		s.sortConcurrent(ctx, items, sorter.concurrentSort, field, direction, sorter.tieBreakers...)
+		s.sortConcurrent(ctx, items, sorter.concurrentSort, field, direction, sorter.tieBreakers)
 		return
 	}
 
-	s.sort(ctx, items, sorter.sort, field, direction, sorter.tieBreakers...)
+	s.sort(ctx, items, sorter.sort, field, direction, sorter.tieBreakers)
 }
 
-func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context, items []T, sort ConcurrentSortFunc[T], field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) {
+func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context, items []T, sort ConcurrentSortFunc[T], field SortField, direction model.OrderDirection, tieBreakers []tieBreaker[SortField]) {
 	type sortable struct {
 		item T
 		key  int
@@ -170,7 +201,7 @@ func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context
 
 	slices.SortStableFunc(res, func(a, b sortable) int {
 		if b.key == a.key {
-			return s.tieBreak(ctx, a.item, b.item, field, direction, tieBreakers...)
+			return s.tieBreak(ctx, a.item, b.item, field, tieBreakers)
 		}
 
 		if direction == model.OrderDirectionDesc {
@@ -184,7 +215,7 @@ func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context
 	}
 }
 
-func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []T, sort SortFunc[T], field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) {
+func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []T, sort SortFunc[T], field SortField, direction model.OrderDirection, tieBreakers []tieBreaker[SortField]) {
 	slices.SortStableFunc(items, func(a, b T) int {
 		var ret int
 		if direction == model.OrderDirectionDesc {
@@ -194,38 +225,33 @@ func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []
 		}
 
 		if ret == 0 {
-			return s.tieBreak(ctx, a, b, field, direction, tieBreakers...)
+			return s.tieBreak(ctx, a, b, field, tieBreakers)
 		}
 		return ret
 	})
 }
 
 // tieBreak will resolve equal fields after the initial sort by using the supplied tie-breakers. The function will
 // return as soon as a tie-breaker returns a non-zero value.
-func (s *SortFilter[T, SortField, FilterObj]) tieBreak(ctx context.Context, a, b T, field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) int {
+func (s *SortFilter[T, SortField, FilterObj]) tieBreak(ctx context.Context, a, b T, originalSortField SortField, tieBreakers []tieBreaker[SortField]) int {
 	for _, tb := range tieBreakers {
-		dir := direction
-		if tb.Direction != nil {
-			dir = *tb.Direction
-		}
-
-		sorter, ok := s.sorters[tb.Field]
+		sorter, ok := s.sorters[tb.field]
 		if !ok {
 			logrus.WithFields(logrus.Fields{
-				"field_type":  fmt.Sprintf("%T", field),
-				"tie_breaker": tb.Field,
+				"field_type":  fmt.Sprintf("%T", originalSortField),
+				"tie_breaker": tieBreakerToSortField(tb),
 			}).Errorf("no sort registered for tie-breaker")
 			continue
 		} else if sorter.sort == nil {
 			logrus.WithFields(logrus.Fields{
-				"field_type":  fmt.Sprintf("%T", field),
-				"tie_breaker": tb.Field,
+				"field_type":  fmt.Sprintf("%T", originalSortField),
+				"tie_breaker": tieBreakerToSortField(tb),
 			}).Errorf("tie-breaker can not be a concurrent sort")
 			continue
 		}
 
 		var v int
-		if dir == model.OrderDirectionDesc {
+		if tb.direction == model.OrderDirectionDesc {
 			v = sorter.sort(ctx, b, a)
 		} else {
 			v = sorter.sort(ctx, a, b)
@@ -238,9 +264,15 @@ func (s *SortFilter[T, SortField, FilterObj]) tieBreak(ctx context.Context, a, b
 
 	logrus.
 		WithFields(logrus.Fields{
-			"field_type":   fmt.Sprintf("%T", field),
-			"sort_field":   field,
-			"tie_breakers": tieBreakers,
+			"field_type": fmt.Sprintf("%T", originalSortField),
+			"sort_field": originalSortField,
+			"tie_breakers": func(tbs []tieBreaker[SortField]) []SortField {
+				ret := make([]SortField, len(tbs))
+				for i, tb := range tbs {
+					ret[i] = tieBreakerToSortField(tb)
+				}
+				return ret
+			}(tieBreakers),
 		}).
 		Errorf("unable to tie-break sort, gotta have more tie-breakers")
 	return 0

internal/persistence/bigquery/sortfilter.go

@@ -4,33 +4,21 @@ import (
 	"context"
 	"strings"
 
-	"github.com/nais/api/internal/graph/model"
 	"github.com/nais/api/internal/graph/sortfilter"
-	"k8s.io/utils/ptr"
 )
 
 var (
 	SortFilter       = sortfilter.New[*BigQueryDataset, BigQueryDatasetOrderField, struct{}]()
 	SortFilterAccess = sortfilter.New[*BigQueryDatasetAccess, BigQueryDatasetAccessOrderField, struct{}]()
 )
 
-type (
-	SortFilterTieBreaker = sortfilter.TieBreaker[BigQueryDatasetOrderField]
-)
-
 func init() {
 	SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *BigQueryDataset) int {
 		return strings.Compare(a.GetName(), b.GetName())
-	}, SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "ENVIRONMENT")
 	SortFilter.RegisterSort("ENVIRONMENT", func(ctx context.Context, a, b *BigQueryDataset) int {
 		return strings.Compare(a.EnvironmentName, b.EnvironmentName)
-	}, SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME")
 
 	SortFilterAccess.RegisterSort("EMAIL", func(ctx context.Context, a, b *BigQueryDatasetAccess) int {
 		return strings.Compare(a.Email, b.Email)

internal/persistence/sqlinstance/sortfilter.go

@@ -5,31 +5,21 @@ import (
 	"strings"
 
 	"github.com/nais/api/internal/cost"
-	"github.com/nais/api/internal/graph/model"
 	"github.com/nais/api/internal/graph/sortfilter"
-	"k8s.io/utils/ptr"
 )
 
 var (
 	SortFilterSQLInstance     = sortfilter.New[*SQLInstance, SQLInstanceOrderField, struct{}]()
 	SortFilterSQLInstanceUser = sortfilter.New[*SQLInstanceUser, SQLInstanceUserOrderField, struct{}]()
 )
 
-type SortFilterTieBreaker = sortfilter.TieBreaker[SQLInstanceOrderField]
-
 func init() {
 	SortFilterSQLInstance.RegisterSort("NAME", func(ctx context.Context, a, b *SQLInstance) int {
 		return strings.Compare(a.GetName(), b.GetName())
-	}, SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "ENVIRONMENT")
 	SortFilterSQLInstance.RegisterSort("ENVIRONMENT", func(ctx context.Context, a, b *SQLInstance) int {
 		return strings.Compare(a.EnvironmentName, b.EnvironmentName)
-	}, SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME")
 	SortFilterSQLInstance.RegisterSort("VERSION", func(ctx context.Context, a, b *SQLInstance) int {
 		if a.Version == nil && b.Version == nil {
 			return 0
@@ -39,13 +29,7 @@ func init() {
 			return -1
 		}
 		return strings.Compare(*a.Version, *b.Version)
-	}, SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	}, SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME", "ENVIRONMENT")
 	SortFilterSQLInstance.RegisterConcurrentSort("STATUS", func(ctx context.Context, a *SQLInstance) int {
 		stateOrder := map[string]int{
 			"UNSPECIFIED":    0,

internal/status/sortfilter.go

@@ -3,41 +3,21 @@ package status
 import (
 	"context"
 
-	"github.com/nais/api/internal/graph/model"
 	"github.com/nais/api/internal/workload"
 	"github.com/nais/api/internal/workload/application"
 	"github.com/nais/api/internal/workload/job"
-	"k8s.io/utils/ptr"
 )
 
 func init() {
 	application.SortFilter.RegisterConcurrentSort("STATUS", func(ctx context.Context, a *application.Application) int {
 		return int(ForWorkload(ctx, a).State)
-	}, application.SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	}, application.SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME", "ENVIRONMENT")
 
 	job.SortFilter.RegisterConcurrentSort("STATUS", func(ctx context.Context, a *job.Job) int {
 		return int(ForWorkload(ctx, a).State)
-	}, job.SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	}, job.SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME", "ENVIRONMENT")
 
 	workload.SortFilter.RegisterConcurrentSort("STATUS", func(ctx context.Context, a workload.Workload) int {
 		return int(ForWorkload(ctx, a).State)
-	}, workload.SortFilterTieBreaker{
-		Field:     "NAME",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	}, workload.SortFilterTieBreaker{
-		Field:     "ENVIRONMENT",
-		Direction: ptr.To(model.OrderDirectionAsc),
-	})
+	}, "NAME", "ENVIRONMENT")
 }