Skip to content

Commit f03b39d

Browse files
christeredvartsenchristeredvartsen
committed
Improve handling of tie-breaks while sorting by allowing each sort to have its own list of tie-breakers
1 parent f002401 commit f03b39d

File tree

14 files changed

+117
-57
lines changed

14 files changed

+117
-57
lines changed

‎internal/environment/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"github.com/nais/api/internal/graph/sortfilter"
88
)
99

10-
var SortFilter = sortfilter.New[*Environment, EnvironmentOrderField, *struct{}]("NAME")
10+
var SortFilter = sortfilter.New[*Environment, EnvironmentOrderField, struct{}]()
1111

1212
func init() {
1313
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *Environment) int {

‎internal/graph/sortfilter/sortfilter.go

+82-34
Original file line numberDiff line numberDiff line change
@@ -6,40 +6,46 @@ import (
66
"slices"
77

88
"github.com/nais/api/internal/graph/model"
9+
"github.com/sirupsen/logrus"
910
"github.com/sourcegraph/conc/pool"
1011
)
1112

1213
// SortFunc compares two values of type V and returns an integer indicating their order.
1314
// If a < b, the function should return a negative value.
1415
// If a == b, the function should return 0.
1516
// If a > b, the function should return a positive value.
16-
type SortFunc[V any] func(ctx context.Context, a, b V) int
17+
type SortFunc[T any] func(ctx context.Context, a, b T) int
1718

1819
// ConcurrentSortFunc should return an integer indicating the order of the given value.
1920
// The results will later be sorted by the returned value.
20-
type ConcurrentSortFunc[V any] func(ctx context.Context, a V) int
21+
type ConcurrentSortFunc[T any] func(ctx context.Context, a T) int
2122

2223
// Filter is a function that returns true if the given value should be included in the result.
23-
type Filter[V any, FilterObj any] func(ctx context.Context, v V, filter FilterObj) bool
24+
type Filter[T any, FilterObj any] func(ctx context.Context, v T, filter FilterObj) bool
25+
26+
// TieBreaker is a combination of a SortField and a direction that might be able to resolve equal fields during sorting.
27+
// If the direction is not supplied, the direction used for the original sort will be used. The referenced field must be
28+
// registered with RegisterSort (concurrent tie-break sorters are not supported).
29+
type TieBreaker[SortField comparable] struct {
30+
Field SortField
31+
Direction *model.OrderDirection
32+
}
2433

25-
type funcs[V any] struct {
26-
concurrentSort ConcurrentSortFunc[V]
27-
sort SortFunc[V]
34+
type funcs[T any, SortField comparable] struct {
35+
concurrentSort ConcurrentSortFunc[T]
36+
sort SortFunc[T]
37+
tieBreakers []TieBreaker[SortField]
2838
}
2939

30-
type SortFilter[V any, SortField comparable, FilterObj comparable] struct {
31-
sorters map[SortField]funcs[V]
32-
filters []Filter[V, FilterObj]
33-
tieBreakSortField SortField
40+
type SortFilter[T any, SortField comparable, FilterObj comparable] struct {
41+
sorters map[SortField]funcs[T, SortField]
42+
filters []Filter[T, FilterObj]
3443
}
3544

36-
// New creates a new SortFilter with the given tieBreakSortField.
37-
// The tieBreakSortField is used when two values are equal in the Sort function, and will use the direction supplied
38-
// when calling Sort. The tieBreakSortField must not be registered as a ConcurrentSort.
39-
func New[V any, SortField comparable, FilterObj comparable](tieBreakSortField SortField) *SortFilter[V, SortField, FilterObj] {
40-
return &SortFilter[V, SortField, FilterObj]{
41-
sorters: make(map[SortField]funcs[V]),
42-
tieBreakSortField: tieBreakSortField,
45+
// New creates a new SortFilter
46+
func New[T any, SortField comparable, FilterObj comparable]() *SortFilter[T, SortField, FilterObj] {
47+
return &SortFilter[T, SortField, FilterObj]{
48+
sorters: make(map[SortField]funcs[T, SortField]),
4349
}
4450
}
4551

@@ -49,25 +55,29 @@ func (s *SortFilter[T, SortField, FilterObj]) SupportsSort(field SortField) bool
4955
return exists
5056
}
5157

52-
func (s *SortFilter[T, SortField, FilterObj]) RegisterSort(field SortField, sort SortFunc[T]) {
58+
// RegisterSort will add support for sorting on a specific field. Optional tie-breakers can be supplied to resolve equal
59+
// values, and will be executed in the given order.
60+
func (s *SortFilter[T, SortField, FilterObj]) RegisterSort(field SortField, sort SortFunc[T], tieBreakers ...TieBreaker[SortField]) {
5361
if _, ok := s.sorters[field]; ok {
5462
panic(fmt.Sprintf("sort field is already registered: %v", field))
5563
}
5664

57-
s.sorters[field] = funcs[T]{
58-
sort: sort,
65+
s.sorters[field] = funcs[T, SortField]{
66+
sort: sort,
67+
tieBreakers: tieBreakers,
5968
}
6069
}
6170

62-
func (s *SortFilter[T, SortField, FilterObj]) RegisterConcurrentSort(field SortField, sort ConcurrentSortFunc[T]) {
71+
// RegisterConcurrentSort will add support for doing concurrent sorting on a specific field. Optional tie-breakers can
72+
// be supplied to resolve equal values, and will be executed in the given order.
73+
func (s *SortFilter[T, SortField, FilterObj]) RegisterConcurrentSort(field SortField, sort ConcurrentSortFunc[T], tieBreakers ...TieBreaker[SortField]) {
6374
if _, ok := s.sorters[field]; ok {
6475
panic(fmt.Sprintf("sort field is already registered: %v", field))
65-
} else if field == s.tieBreakSortField {
66-
panic(fmt.Sprintf("sort field is used for tie break and can not be concurrent: %v", field))
6776
}
6877

69-
s.sorters[field] = funcs[T]{
78+
s.sorters[field] = funcs[T, SortField]{
7079
concurrentSort: sort,
80+
tieBreakers: tieBreakers,
7181
}
7282
}
7383

@@ -129,14 +139,14 @@ func (s *SortFilter[T, SortField, FilterObj]) Sort(ctx context.Context, items []
129139
}
130140

131141
if sorter.concurrentSort != nil {
132-
s.sortConcurrent(ctx, items, sorter.concurrentSort, direction)
142+
s.sortConcurrent(ctx, items, sorter.concurrentSort, field, direction, sorter.tieBreakers...)
133143
return
134144
}
135145

136-
s.sort(ctx, items, sorter.sort, direction)
146+
s.sort(ctx, items, sorter.sort, field, direction, sorter.tieBreakers...)
137147
}
138148

139-
func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context, items []T, sort ConcurrentSortFunc[T], direction model.OrderDirection) {
149+
func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context, items []T, sort ConcurrentSortFunc[T], field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) {
140150
type sortable struct {
141151
item T
142152
key int
@@ -160,7 +170,7 @@ func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context
160170

161171
slices.SortStableFunc(res, func(a, b sortable) int {
162172
if b.key == a.key {
163-
return s.tieBreak(ctx, a.item, b.item, direction)
173+
return s.tieBreak(ctx, a.item, b.item, field, direction, tieBreakers...)
164174
}
165175

166176
if direction == model.OrderDirectionDesc {
@@ -174,7 +184,7 @@ func (s *SortFilter[T, SortField, FilterObj]) sortConcurrent(ctx context.Context
174184
}
175185
}
176186

177-
func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []T, sort SortFunc[T], direction model.OrderDirection) {
187+
func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []T, sort SortFunc[T], field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) {
178188
slices.SortStableFunc(items, func(a, b T) int {
179189
var ret int
180190
if direction == model.OrderDirectionDesc {
@@ -184,16 +194,54 @@ func (s *SortFilter[T, SortField, FilterObj]) sort(ctx context.Context, items []
184194
}
185195

186196
if ret == 0 {
187-
return s.tieBreak(ctx, a, b, direction)
197+
return s.tieBreak(ctx, a, b, field, direction, tieBreakers...)
188198
}
189199
return ret
190200
})
191201
}
192202

193-
func (s *SortFilter[T, SortField, FilterObj]) tieBreak(ctx context.Context, a, b T, direction model.OrderDirection) int {
194-
if direction == model.OrderDirectionDesc {
195-
return s.sorters[s.tieBreakSortField].sort(ctx, b, a)
203+
// tieBreak will resolve equal fields after the initial sort by using the supplied tie-breakers. The function will
204+
// return as soon as a tie-breaker returns a non-zero value.
205+
func (s *SortFilter[T, SortField, FilterObj]) tieBreak(ctx context.Context, a, b T, field SortField, direction model.OrderDirection, tieBreakers ...TieBreaker[SortField]) int {
206+
for _, tb := range tieBreakers {
207+
dir := direction
208+
if tb.Direction != nil {
209+
dir = *tb.Direction
210+
}
211+
212+
sorter, ok := s.sorters[tb.Field]
213+
if !ok {
214+
logrus.WithFields(logrus.Fields{
215+
"field_type": fmt.Sprintf("%T", field),
216+
"tie_breaker": tb.Field,
217+
}).Errorf("no sort registered for tie-breaker")
218+
continue
219+
} else if sorter.sort == nil {
220+
logrus.WithFields(logrus.Fields{
221+
"field_type": fmt.Sprintf("%T", field),
222+
"tie_breaker": tb.Field,
223+
}).Errorf("tie-breaker can not be a concurrent sort")
224+
continue
225+
}
226+
227+
var v int
228+
if dir == model.OrderDirectionDesc {
229+
v = sorter.sort(ctx, b, a)
230+
} else {
231+
v = sorter.sort(ctx, a, b)
232+
}
233+
234+
if v != 0 {
235+
return v
236+
}
196237
}
197238

198-
return s.sorters[s.tieBreakSortField].sort(ctx, a, b)
239+
logrus.
240+
WithFields(logrus.Fields{
241+
"field_type": fmt.Sprintf("%T", field),
242+
"sort_field": field,
243+
"tie_breakers": tieBreakers,
244+
}).
245+
Errorf("unable to tie-break sort, gotta have more tie-breakers")
246+
return 0
199247
}

‎internal/persistence/bigquery/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
)
99

1010
var (
11-
SortFilter = sortfilter.New[*BigQueryDataset, BigQueryDatasetOrderField, struct{}]("NAME")
12-
SortFilterAccess = sortfilter.New[*BigQueryDatasetAccess, BigQueryDatasetAccessOrderField, struct{}]("EMAIL")
11+
SortFilter = sortfilter.New[*BigQueryDataset, BigQueryDatasetOrderField, struct{}]()
12+
SortFilterAccess = sortfilter.New[*BigQueryDatasetAccess, BigQueryDatasetAccessOrderField, struct{}]()
1313
)
1414

1515
func init() {

‎internal/persistence/bucket/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"github.com/nais/api/internal/graph/sortfilter"
88
)
99

10-
var SortFilter = sortfilter.New[*Bucket, BucketOrderField, struct{}]("NAME")
10+
var SortFilter = sortfilter.New[*Bucket, BucketOrderField, struct{}]()
1111

1212
func init() {
1313
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *Bucket) int {

‎internal/persistence/kafkatopic/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
)
99

1010
var (
11-
SortFilterTopic = sortfilter.New[*KafkaTopic, KafkaTopicOrderField, struct{}]("NAME")
12-
SortFilterTopicACL = sortfilter.New[*KafkaTopicACL, KafkaTopicACLOrderField, *KafkaTopicACLFilter]("TOPIC_NAME")
11+
SortFilterTopic = sortfilter.New[*KafkaTopic, KafkaTopicOrderField, struct{}]()
12+
SortFilterTopicACL = sortfilter.New[*KafkaTopicACL, KafkaTopicACLOrderField, *KafkaTopicACLFilter]()
1313
)
1414

1515
func init() {

‎internal/persistence/opensearch/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
)
99

1010
var (
11-
SortFilterOpenSearch = sortfilter.New[*OpenSearch, OpenSearchOrderField, struct{}]("NAME")
12-
SortFilterOpenSearchAccess = sortfilter.New[*OpenSearchAccess, OpenSearchAccessOrderField, struct{}]("ACCESS")
11+
SortFilterOpenSearch = sortfilter.New[*OpenSearch, OpenSearchOrderField, struct{}]()
12+
SortFilterOpenSearchAccess = sortfilter.New[*OpenSearchAccess, OpenSearchAccessOrderField, struct{}]()
1313
)
1414

1515
func init() {

‎internal/persistence/redis/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
)
99

1010
var (
11-
SortFilterRedisInstance = sortfilter.New[*RedisInstance, RedisInstanceOrderField, struct{}]("NAME")
12-
SortFilterRedisInstanceAccess = sortfilter.New[*RedisInstanceAccess, RedisInstanceAccessOrderField, struct{}]("ACCESS")
11+
SortFilterRedisInstance = sortfilter.New[*RedisInstance, RedisInstanceOrderField, struct{}]()
12+
SortFilterRedisInstanceAccess = sortfilter.New[*RedisInstanceAccess, RedisInstanceAccessOrderField, struct{}]()
1313
)
1414

1515
func init() {

‎internal/persistence/sqlinstance/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
)
1010

1111
var (
12-
SortFilterSQLInstance = sortfilter.New[*SQLInstance, SQLInstanceOrderField, struct{}]("NAME")
13-
SortFilterSQLInstanceUser = sortfilter.New[*SQLInstanceUser, SQLInstanceUserOrderField, struct{}]("NAME")
12+
SortFilterSQLInstance = sortfilter.New[*SQLInstance, SQLInstanceOrderField, struct{}]()
13+
SortFilterSQLInstanceUser = sortfilter.New[*SQLInstanceUser, SQLInstanceUserOrderField, struct{}]()
1414
)
1515

1616
func init() {

‎internal/persistence/valkey/sortfilter.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
)
99

1010
var (
11-
SortFilterValkeyInstance = sortfilter.New[*ValkeyInstance, ValkeyInstanceOrderField, struct{}]("NAME")
12-
SortFilterValkeyInstanceAccess = sortfilter.New[*ValkeyInstanceAccess, ValkeyInstanceAccessOrderField, struct{}]("ACCESS")
11+
SortFilterValkeyInstance = sortfilter.New[*ValkeyInstance, ValkeyInstanceOrderField, struct{}]()
12+
SortFilterValkeyInstanceAccess = sortfilter.New[*ValkeyInstanceAccess, ValkeyInstanceAccessOrderField, struct{}]()
1313
)
1414

1515
func init() {

‎internal/vulnerability/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"github.com/nais/api/internal/workload"
99
)
1010

11-
var SortFilterImageVulnerabilities = sortfilter.New[*ImageVulnerability, ImageVulnerabilityOrderField, *struct{}]("IDENTIFIER")
11+
var SortFilterImageVulnerabilities = sortfilter.New[*ImageVulnerability, ImageVulnerabilityOrderField, struct{}]()
1212

1313
func init() {
1414
workloadInit()

‎internal/workload/application/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"github.com/nais/api/internal/graph/sortfilter"
99
)
1010

11-
var SortFilter = sortfilter.New[*Application, ApplicationOrderField, *TeamApplicationsFilter]("NAME")
11+
var SortFilter = sortfilter.New[*Application, ApplicationOrderField, *TeamApplicationsFilter]()
1212

1313
func init() {
1414
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *Application) int {

‎internal/workload/job/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"github.com/nais/api/internal/graph/sortfilter"
99
)
1010

11-
var SortFilter = sortfilter.New[*Job, JobOrderField, *TeamJobsFilter]("NAME")
11+
var SortFilter = sortfilter.New[*Job, JobOrderField, *TeamJobsFilter]()
1212

1313
func init() {
1414
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *Job) int {

‎internal/workload/secret/sortfilter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/nais/api/internal/workload/job"
1111
)
1212

13-
var SortFilter = sortfilter.New[*Secret, SecretOrderField, *SecretFilter]("NAME")
13+
var SortFilter = sortfilter.New[*Secret, SecretOrderField, *SecretFilter]()
1414

1515
func init() {
1616
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b *Secret) int {

‎internal/workload/sortfilter.go

+17-5
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,18 @@ import (
55
"slices"
66
"strings"
77

8+
"github.com/nais/api/internal/graph/model"
89
"github.com/nais/api/internal/graph/sortfilter"
10+
"k8s.io/utils/ptr"
911
)
1012

1113
var (
12-
SortFilter = sortfilter.New[Workload, WorkloadOrderField, *TeamWorkloadsFilter]("NAME")
13-
SortFilterEnvironment = sortfilter.New[Workload, EnvironmentWorkloadOrderField, *struct{}]("NAME")
14+
SortFilter = sortfilter.New[Workload, WorkloadOrderField, *TeamWorkloadsFilter]()
15+
SortFilterEnvironment = sortfilter.New[Workload, EnvironmentWorkloadOrderField, struct{}]()
1416
)
1517

18+
type SortFilterEnvironmentTieBreaker = sortfilter.TieBreaker[EnvironmentWorkloadOrderField]
19+
1620
func init() {
1721
SortFilter.RegisterSort("NAME", func(ctx context.Context, a, b Workload) int {
1822
return strings.Compare(a.GetName(), b.GetName())
@@ -32,9 +36,17 @@ func init() {
3236
return false
3337
})
3438

35-
SortFilterEnvironment.RegisterSort("NAME", func(ctx context.Context, a, b Workload) int {
36-
return strings.Compare(a.GetName(), b.GetName())
37-
})
39+
SortFilterEnvironment.RegisterSort(
40+
"NAME",
41+
func(ctx context.Context, a, b Workload) int {
42+
return strings.Compare(a.GetName(), b.GetName())
43+
},
44+
// Sort by team when we have workloads with the same name
45+
SortFilterEnvironmentTieBreaker{
46+
Field: "TEAM_SLUG",
47+
Direction: ptr.To(model.OrderDirectionAsc),
48+
},
49+
)
3850
SortFilterEnvironment.RegisterSort("TEAM_SLUG", func(ctx context.Context, a, b Workload) int {
3951
return strings.Compare(a.GetTeamSlug().String(), b.GetTeamSlug().String())
4052
})

0 commit comments

Comments
 (0)