From a0a81ed34cb37cf0c82abaa356301914e6e3ce00 Mon Sep 17 00:00:00 2001 From: ze0s <43699394+zze0s@users.noreply.github.com> Date: Sun, 31 Dec 2023 14:59:12 +0100 Subject: [PATCH] fix(filters): ensure sort by priority (#1325) * fix(filters): sort filters from filtersMap * fix(filters): use slices.SortStableFunc and fix tests * fix(filters): add local cmp pkg before go 1.21 --- internal/database/filter.go | 8 ++++ internal/database/filter_test.go | 64 +++++++++++++++++++++++++++++--- pkg/cmp/cmp.go | 59 +++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 pkg/cmp/cmp.go diff --git a/internal/database/filter.go b/internal/database/filter.go index c21bae0..ec3e0c5 100644 --- a/internal/database/filter.go +++ b/internal/database/filter.go @@ -12,11 +12,13 @@ import ( "github.com/autobrr/autobrr/internal/domain" "github.com/autobrr/autobrr/internal/logger" + "github.com/autobrr/autobrr/pkg/cmp" "github.com/autobrr/autobrr/pkg/errors" sq "github.com/Masterminds/squirrel" "github.com/lib/pq" "github.com/rs/zerolog" + "golang.org/x/exp/slices" ) type FilterRepo struct { @@ -713,6 +715,12 @@ func (r *FilterRepo) findByIndexerIdentifier(ctx context.Context, indexer string filters = append(filters, filter) } + // the filterMap messes up the order, so we need to sort the filters slice + slices.SortStableFunc(filters, func(a, b *domain.Filter) int { + // TODO remove with Go 1.21 and use std lib cmp + return cmp.Compare(b.Priority, a.Priority) + }) + return filters, nil } diff --git a/internal/database/filter_test.go b/internal/database/filter_test.go index 0b1e846..146435f 100644 --- a/internal/database/filter_test.go +++ b/internal/database/filter_test.go @@ -467,20 +467,63 @@ func TestFilterRepo_FindByIndexerIdentifier(t *testing.T) { log := setupLoggerForTest() repo := NewFilterRepo(log, db) indexerRepo := NewIndexerRepo(log, db) - mockData := getMockFilter() + //mockData := getMockFilter() indexerMockData := getMockIndexer() + filtersData := []*domain.Filter{ + { + Enabled: true, + Name: "filter 1", + Priority: 20, + Resolutions: []string{}, + Codecs: []string{}, + Sources: []string{}, + Containers: []string{}, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, + { + Enabled: true, + Name: "filter 2", + Priority: 30, + Resolutions: []string{}, + Codecs: []string{}, + Sources: []string{}, + Containers: []string{}, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, + { + Enabled: true, + Name: "filter 20", + Priority: 100, + Resolutions: []string{}, + Codecs: []string{}, + Sources: []string{}, + Containers: []string{}, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + }, + } + t.Run(fmt.Sprintf("FindByIndexerIdentifier_Succeeds [%s]", dbType), func(t *testing.T) { // Setup - err := repo.Store(context.Background(), mockData) - assert.NoError(t, err) - indexer, err := indexerRepo.Store(context.Background(), indexerMockData) assert.NoError(t, err) assert.NotNil(t, indexer) - err = repo.StoreIndexerConnection(context.Background(), mockData.ID, int(indexer.ID)) + for _, filter := range filtersData { + filter := filter + err := repo.Store(context.Background(), filter) + assert.NoError(t, err) + + err = repo.StoreIndexerConnection(context.Background(), filter.ID, int(indexer.ID)) + assert.NoError(t, err) + } + + filtersList, err := repo.ListFilters(context.Background()) assert.NoError(t, err) + assert.NotEmpty(t, filtersList) // Execute filters, err := repo.FindByIndexerIdentifier(context.Background(), indexerMockData.Identifier) @@ -488,9 +531,18 @@ func TestFilterRepo_FindByIndexerIdentifier(t *testing.T) { assert.NotNil(t, filters) assert.NotEmpty(t, filters) + assert.Equal(t, filters[0].Priority, int32(100)) + assert.Equal(t, filters[1].Priority, int32(30)) + assert.Equal(t, filters[2].Priority, int32(20)) + // Cleanup _ = indexerRepo.Delete(context.Background(), int(indexer.ID)) - _ = repo.Delete(context.Background(), mockData.ID) + + for _, filter := range filtersData { + filter := filter + + _ = repo.Delete(context.Background(), filter.ID) + } }) t.Run(fmt.Sprintf("FindByIndexerIdentifier_Fails_Invalid_Identifier [%s]", dbType), func(t *testing.T) { diff --git a/pkg/cmp/cmp.go b/pkg/cmp/cmp.go new file mode 100644 index 0000000..0fba5c1 --- /dev/null +++ b/pkg/cmp/cmp.go @@ -0,0 +1,59 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package cmp provides types and functions related to comparing +// ordered values. +package cmp + +// Ordered is a constraint that permits any ordered type: any type +// that supports the operators < <= >= >. +// If future releases of Go add new ordered types, +// this constraint will be modified to include them. +// +// Note that floating-point types may contain NaN ("not-a-number") values. +// An operator such as == or < will always report false when +// comparing a NaN value with any other value, NaN or not. +// See the [Compare] function for a consistent way to compare NaN values. +type Ordered interface { + ~int | ~int8 | ~int16 | ~int32 | ~int64 | + ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr | + ~float32 | ~float64 | + ~string +} + +// Less reports whether x is less than y. +// For floating-point types, a NaN is considered less than any non-NaN, +// and -0.0 is not less than (is equal to) 0.0. +func Less[T Ordered](x, y T) bool { + return (isNaN(x) && !isNaN(y)) || x < y +} + +// Compare returns +// +// -1 if x is less than y, +// 0 if x equals y, +// +1 if x is greater than y. +// +// For floating-point types, a NaN is considered less than any non-NaN, +// a NaN is considered equal to a NaN, and -0.0 is equal to 0.0. +func Compare[T Ordered](x, y T) int { + xNaN := isNaN(x) + yNaN := isNaN(y) + if xNaN && yNaN { + return 0 + } + if xNaN || x < y { + return -1 + } + if yNaN || x > y { + return +1 + } + return 0 +} + +// isNaN reports whether x is a NaN without requiring the math package. +// This will always return false if T is not floating-point. +func isNaN[T Ordered](x T) bool { + return x != x +}