From e3c129b4808c3761279667c53a528e8ec2db2fd5 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 10 Feb 2025 12:27:57 +0100 Subject: [PATCH 1/5] Move filter code to util/slices Signed-off-by: Per Goncalves da Silva --- .../catalogmetadata/filter/bundle_predicates.go | 5 +++-- internal/catalogmetadata/filter/successors.go | 7 ++++--- internal/catalogmetadata/filter/successors_test.go | 3 ++- internal/resolve/catalog.go | 5 +++-- .../filter => util/slices}/filter.go | 2 +- .../filter => util/slices}/filter_test.go | 14 +++++++------- 6 files changed, 20 insertions(+), 16 deletions(-) rename internal/{catalogmetadata/filter => util/slices}/filter.go (98%) rename internal/{catalogmetadata/filter => util/slices}/filter_test.go (85%) diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 98e3ab4cd..ef4a24787 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -6,9 +6,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-controller/internal/bundleutil" + slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[declcfg.Bundle] { +func InMastermindsSemverRange(semverRange *mmsemver.Constraints) slicesutil.Predicate[declcfg.Bundle] { return func(b declcfg.Bundle) bool { bVersion, err := bundleutil.GetVersion(b) if err != nil { @@ -26,7 +27,7 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[declc } } -func InAnyChannel(channels ...declcfg.Channel) Predicate[declcfg.Bundle] { +func InAnyChannel(channels ...declcfg.Channel) slicesutil.Predicate[declcfg.Bundle] { return func(bundle declcfg.Bundle) bool { for _, ch := range channels { for _, entry := range ch.Entries { diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go index cd2ea4f5b..28064cdc7 100644 --- a/internal/catalogmetadata/filter/successors.go +++ b/internal/catalogmetadata/filter/successors.go @@ -9,9 +9,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" + slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) -func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { +func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (slicesutil.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err) @@ -28,13 +29,13 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann } // We need either successors or current version (no upgrade) - return Or( + return slicesutil.Or( successorsPredicate, InMastermindsSemverRange(installedVersionConstraint), ), nil } -func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (Predicate[declcfg.Bundle], error) { +func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (slicesutil.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := bsemver.Parse(installedBundle.Version) if err != nil { return nil, fmt.Errorf("error parsing installed bundle version: %w", err) diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go index 2a0a53348..60b55a07d 100644 --- a/internal/catalogmetadata/filter/successors_test.go +++ b/internal/catalogmetadata/filter/successors_test.go @@ -16,6 +16,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" + slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) func TestSuccessorsPredicate(t *testing.T) { @@ -160,7 +161,7 @@ func TestSuccessorsPredicate(t *testing.T) { for _, bundle := range bundleSet { allBundles = append(allBundles, bundle) } - result := Filter(allBundles, successors) + result := slicesutil.Filter(allBundles, successors) // sort before comparison for stable order slices.SortFunc(result, compare.ByVersion) diff --git a/internal/resolve/catalog.go b/internal/resolve/catalog.go index 31b3d15ec..3c23ed36c 100644 --- a/internal/resolve/catalog.go +++ b/internal/resolve/catalog.go @@ -22,6 +22,7 @@ import ( "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) type ValidationFunc func(*declcfg.Bundle) error @@ -96,7 +97,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio cs.PackageFound = true cs.TotalBundles = len(packageFBC.Bundles) - var predicates []filter.Predicate[declcfg.Bundle] + var predicates []slicesutil.Predicate[declcfg.Bundle] if len(channels) > 0 { channelSet := sets.New(channels...) filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool { @@ -118,7 +119,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } // Apply the predicates to get the candidate bundles - packageFBC.Bundles = filter.Filter(packageFBC.Bundles, filter.And(predicates...)) + packageFBC.Bundles = slicesutil.Filter(packageFBC.Bundles, slicesutil.And(predicates...)) cs.MatchedBundles = len(packageFBC.Bundles) if len(packageFBC.Bundles) == 0 { return nil diff --git a/internal/catalogmetadata/filter/filter.go b/internal/util/slices/filter.go similarity index 98% rename from internal/catalogmetadata/filter/filter.go rename to internal/util/slices/filter.go index 9074c926d..1501a0cbc 100644 --- a/internal/catalogmetadata/filter/filter.go +++ b/internal/util/slices/filter.go @@ -1,4 +1,4 @@ -package filter +package slices import ( "slices" diff --git a/internal/catalogmetadata/filter/filter_test.go b/internal/util/slices/filter_test.go similarity index 85% rename from internal/catalogmetadata/filter/filter_test.go rename to internal/util/slices/filter_test.go index 77d12c99f..5a90c9026 100644 --- a/internal/catalogmetadata/filter/filter_test.go +++ b/internal/util/slices/filter_test.go @@ -1,4 +1,4 @@ -package filter_test +package slices_test import ( "testing" @@ -7,13 +7,13 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" - "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + "github.com/operator-framework/operator-controller/internal/util/slices" ) func TestFilter(t *testing.T) { for _, tt := range []struct { name string - predicate filter.Predicate[declcfg.Bundle] + predicate slices.Predicate[declcfg.Bundle] want []declcfg.Bundle }{ { @@ -27,7 +27,7 @@ func TestFilter(t *testing.T) { }, { name: "filter with Not predicate", - predicate: filter.Not(func(bundle declcfg.Bundle) bool { + predicate: slices.Not(func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }), want: []declcfg.Bundle{ @@ -37,7 +37,7 @@ func TestFilter(t *testing.T) { }, { name: "filter with And predicate", - predicate: filter.And( + predicate: slices.And( func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }, @@ -51,7 +51,7 @@ func TestFilter(t *testing.T) { }, { name: "filter with Or predicate", - predicate: filter.Or( + predicate: slices.Or( func(bundle declcfg.Bundle) bool { return bundle.Name == "extension1.v1" }, @@ -72,7 +72,7 @@ func TestFilter(t *testing.T) { {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, } - actual := filter.Filter(in, tt.predicate) + actual := slices.Filter(in, tt.predicate) assert.Equal(t, tt.want, actual) }) } From c8f05780c09578f0d62f085b693cac5421c67b49 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 10 Feb 2025 15:37:01 +0100 Subject: [PATCH 2/5] Rename Filter to RemoveInPlace Signed-off-by: Per Goncalves da Silva --- internal/catalogmetadata/filter/successors_test.go | 2 +- internal/resolve/catalog.go | 2 +- internal/util/slices/filter.go | 7 ++++--- internal/util/slices/filter_test.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go index 60b55a07d..90e772d77 100644 --- a/internal/catalogmetadata/filter/successors_test.go +++ b/internal/catalogmetadata/filter/successors_test.go @@ -161,7 +161,7 @@ func TestSuccessorsPredicate(t *testing.T) { for _, bundle := range bundleSet { allBundles = append(allBundles, bundle) } - result := slicesutil.Filter(allBundles, successors) + result := slicesutil.RemoveInPlace(allBundles, successors) // sort before comparison for stable order slices.SortFunc(result, compare.ByVersion) diff --git a/internal/resolve/catalog.go b/internal/resolve/catalog.go index 3c23ed36c..70c8a8ddf 100644 --- a/internal/resolve/catalog.go +++ b/internal/resolve/catalog.go @@ -119,7 +119,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } // Apply the predicates to get the candidate bundles - packageFBC.Bundles = slicesutil.Filter(packageFBC.Bundles, slicesutil.And(predicates...)) + packageFBC.Bundles = slicesutil.RemoveInPlace(packageFBC.Bundles, slicesutil.And(predicates...)) cs.MatchedBundles = len(packageFBC.Bundles) if len(packageFBC.Bundles) == 0 { return nil diff --git a/internal/util/slices/filter.go b/internal/util/slices/filter.go index 1501a0cbc..86b30c1af 100644 --- a/internal/util/slices/filter.go +++ b/internal/util/slices/filter.go @@ -7,9 +7,10 @@ import ( // Predicate returns true if the object should be kept when filtering type Predicate[T any] func(entity T) bool -// Filter filters a slice accordingly to -func Filter[T any](in []T, test Predicate[T]) []T { - return slices.DeleteFunc(in, Not(test)) +// RemoveInPlace removes all elements from s for which test returns true. +// Elements between new length and original length are zeroed out. +func RemoveInPlace[T any](s []T, test Predicate[T]) []T { + return slices.DeleteFunc(s, Not(test)) } func And[T any](predicates ...Predicate[T]) Predicate[T] { diff --git a/internal/util/slices/filter_test.go b/internal/util/slices/filter_test.go index 5a90c9026..66ea52804 100644 --- a/internal/util/slices/filter_test.go +++ b/internal/util/slices/filter_test.go @@ -72,7 +72,7 @@ func TestFilter(t *testing.T) { {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, } - actual := slices.Filter(in, tt.predicate) + actual := slices.RemoveInPlace(in, tt.predicate) assert.Equal(t, tt.want, actual) }) } From 8240a1b357e7b0dba962037714d7caa09b6b4d1a Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 10 Feb 2025 15:37:48 +0100 Subject: [PATCH 3/5] Add Filter function and update unit tests Signed-off-by: Per Goncalves da Silva --- internal/util/slices/filter.go | 11 ++ internal/util/slices/filter_test.go | 262 ++++++++++++++++++++++------ 2 files changed, 222 insertions(+), 51 deletions(-) diff --git a/internal/util/slices/filter.go b/internal/util/slices/filter.go index 86b30c1af..dec44f9b0 100644 --- a/internal/util/slices/filter.go +++ b/internal/util/slices/filter.go @@ -7,6 +7,17 @@ import ( // Predicate returns true if the object should be kept when filtering type Predicate[T any] func(entity T) bool +// Filter creates a new slice with all elements from s for which the test returns true +func Filter[T any](s []T, test Predicate[T]) []T { + out := make([]T, 0, len(s)) + for i := 0; i < len(s); i++ { + if test(s[i]) { + out = append(out, s[i]) + } + } + return slices.Clip(out) +} + // RemoveInPlace removes all elements from s for which test returns true. // Elements between new length and original length are zeroed out. func RemoveInPlace[T any](s []T, test Predicate[T]) []T { diff --git a/internal/util/slices/filter_test.go b/internal/util/slices/filter_test.go index 66ea52804..1d7e7759b 100644 --- a/internal/util/slices/filter_test.go +++ b/internal/util/slices/filter_test.go @@ -3,77 +3,237 @@ package slices_test import ( "testing" - "github.com/stretchr/testify/assert" - - "github.com/operator-framework/operator-registry/alpha/declcfg" + "github.com/stretchr/testify/require" "github.com/operator-framework/operator-controller/internal/util/slices" ) -func TestFilter(t *testing.T) { - for _, tt := range []struct { - name string - predicate slices.Predicate[declcfg.Bundle] - want []declcfg.Bundle +func TestAnd(t *testing.T) { + tests := []struct { + name string + predicates []slices.Predicate[int] + input int + want bool }{ { - name: "simple filter with one predicate", - predicate: func(bundle declcfg.Bundle) bool { - return bundle.Name == "extension1.v1" + name: "all true", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 10 }, }, - want: []declcfg.Bundle{ - {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, + input: 5, + want: true, + }, + { + name: "one false", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 5 }, }, + input: 5, + want: false, + }, + { + name: "all false", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 10 }, + func(i int) bool { return i < 0 }, + }, + input: 5, + want: false, + }, + { + name: "no predicates", + predicates: []slices.Predicate[int]{}, + input: 5, + want: true, + }, + { + name: "nil predicates", + predicates: nil, + input: 5, + want: true, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := slices.And(tt.predicates...)(tt.input) + require.Equal(t, tt.want, got, "And() = %v, want %v", got, tt.want) + }) + } +} + +func TestOr(t *testing.T) { + tests := []struct { + name string + predicates []slices.Predicate[int] + input int + want bool + }{ { - name: "filter with Not predicate", - predicate: slices.Not(func(bundle declcfg.Bundle) bool { - return bundle.Name == "extension1.v1" - }), - want: []declcfg.Bundle{ - {Name: "extension1.v2", Package: "extension1", Image: "fake2"}, - {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, + name: "all true", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 10 }, }, + input: 5, + want: true, }, { - name: "filter with And predicate", - predicate: slices.And( - func(bundle declcfg.Bundle) bool { - return bundle.Name == "extension1.v1" - }, - func(bundle declcfg.Bundle) bool { - return bundle.Image == "fake1" - }, - ), - want: []declcfg.Bundle{ - {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, + name: "one false", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 5 }, }, + input: 5, + want: true, }, { - name: "filter with Or predicate", - predicate: slices.Or( - func(bundle declcfg.Bundle) bool { - return bundle.Name == "extension1.v1" - }, - func(bundle declcfg.Bundle) bool { - return bundle.Image == "fake1" - }, - ), - want: []declcfg.Bundle{ - {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, - {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, + name: "all false", + predicates: []slices.Predicate[int]{ + func(i int) bool { return i > 10 }, + func(i int) bool { return i < 0 }, }, + input: 5, + want: false, + }, + { + name: "no predicates", + predicates: []slices.Predicate[int]{}, + input: 5, + want: false, + }, + { + name: "nil predicates", + predicates: nil, + input: 5, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := slices.Or(tt.predicates...)(tt.input) + require.Equal(t, tt.want, got, "Or() = %v, want %v", got, tt.want) + }) + } +} + +func TestNot(t *testing.T) { + tests := []struct { + name string + predicate slices.Predicate[int] + input int + want bool + }{ + { + name: "predicate is true", + predicate: func(i int) bool { return i > 0 }, + input: 5, + want: false, + }, + { + name: "predicate is false", + predicate: func(i int) bool { return i > 3 }, + input: 2, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := slices.Not(tt.predicate)(tt.input) + require.Equal(t, tt.want, got, "Not() = %v, want %v", got, tt.want) + }) + } +} + +func TestFilter(t *testing.T) { + tests := []struct { + name string + slice []int + predicate slices.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, }, - } { + } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - in := []declcfg.Bundle{ - {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, - {Name: "extension1.v2", Package: "extension1", Image: "fake2"}, - {Name: "extension2.v1", Package: "extension2", Image: "fake1"}, - } + got := slices.Filter(tt.slice, tt.predicate) + require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) + }) + } +} - actual := slices.RemoveInPlace(in, tt.predicate) - assert.Equal(t, tt.want, actual) +func TestRemoveInPlace(t *testing.T) { + tests := []struct { + name string + slice []int + predicate slices.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5, 0, 0, 0}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: []int{0, 0, 0, 0, 0}, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _ = slices.RemoveInPlace(tt.slice, tt.predicate) + require.Equal(t, tt.want, tt.slice, "Filter() = %v, want %v", tt.slice, tt.want) }) } } From ce8a227968b250211f0a32217357e7799a620143 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 10 Feb 2025 15:46:08 +0100 Subject: [PATCH 4/5] Refactor slice utils package structure Signed-off-by: Per Goncalves da Silva --- .../filter/bundle_predicates.go | 6 +- internal/catalogmetadata/filter/successors.go | 8 +- internal/resolve/catalog.go | 7 +- .../filter.go => filter/predicates.go} | 23 +- internal/util/filter/predicates_test.go | 147 +++++++++++ internal/util/slices/filter_test.go | 239 ------------------ internal/util/slices/slices.go | 24 ++ internal/util/slices/slices_test.go | 102 ++++++++ 8 files changed, 285 insertions(+), 271 deletions(-) rename internal/util/{slices/filter.go => filter/predicates.go} (52%) create mode 100644 internal/util/filter/predicates_test.go delete mode 100644 internal/util/slices/filter_test.go create mode 100644 internal/util/slices/slices.go create mode 100644 internal/util/slices/slices_test.go diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index ef4a24787..c73dfec98 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -6,10 +6,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-controller/internal/bundleutil" - slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" + filterutil "github.com/operator-framework/operator-controller/internal/util/filter" ) -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) slicesutil.Predicate[declcfg.Bundle] { +func InMastermindsSemverRange(semverRange *mmsemver.Constraints) filterutil.Predicate[declcfg.Bundle] { return func(b declcfg.Bundle) bool { bVersion, err := bundleutil.GetVersion(b) if err != nil { @@ -27,7 +27,7 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) slicesutil.Pred } } -func InAnyChannel(channels ...declcfg.Channel) slicesutil.Predicate[declcfg.Bundle] { +func InAnyChannel(channels ...declcfg.Channel) filterutil.Predicate[declcfg.Bundle] { return func(bundle declcfg.Bundle) bool { for _, ch := range channels { for _, entry := range ch.Entries { diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go index 28064cdc7..a9ae50147 100644 --- a/internal/catalogmetadata/filter/successors.go +++ b/internal/catalogmetadata/filter/successors.go @@ -9,10 +9,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" - slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" + filterutil "github.com/operator-framework/operator-controller/internal/util/filter" ) -func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (slicesutil.Predicate[declcfg.Bundle], error) { +func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filterutil.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err) @@ -29,13 +29,13 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann } // We need either successors or current version (no upgrade) - return slicesutil.Or( + return filterutil.Or( successorsPredicate, InMastermindsSemverRange(installedVersionConstraint), ), nil } -func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (slicesutil.Predicate[declcfg.Bundle], error) { +func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filterutil.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := bsemver.Parse(installedBundle.Version) if err != nil { return nil, fmt.Errorf("error parsing installed bundle version: %w", err) diff --git a/internal/resolve/catalog.go b/internal/resolve/catalog.go index 70c8a8ddf..9c94eddef 100644 --- a/internal/resolve/catalog.go +++ b/internal/resolve/catalog.go @@ -22,6 +22,7 @@ import ( "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" + filterutil "github.com/operator-framework/operator-controller/internal/util/filter" slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) @@ -76,7 +77,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio var catStats []*catStat - resolvedBundles := []foundBundle{} + var resolvedBundles []foundBundle var priorDeprecation *declcfg.Deprecation listOptions := []client.ListOption{ @@ -97,7 +98,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio cs.PackageFound = true cs.TotalBundles = len(packageFBC.Bundles) - var predicates []slicesutil.Predicate[declcfg.Bundle] + var predicates []filterutil.Predicate[declcfg.Bundle] if len(channels) > 0 { channelSet := sets.New(channels...) filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool { @@ -119,7 +120,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } // Apply the predicates to get the candidate bundles - packageFBC.Bundles = slicesutil.RemoveInPlace(packageFBC.Bundles, slicesutil.And(predicates...)) + packageFBC.Bundles = slicesutil.RemoveInPlace(packageFBC.Bundles, filterutil.And(predicates...)) cs.MatchedBundles = len(packageFBC.Bundles) if len(packageFBC.Bundles) == 0 { return nil diff --git a/internal/util/slices/filter.go b/internal/util/filter/predicates.go similarity index 52% rename from internal/util/slices/filter.go rename to internal/util/filter/predicates.go index dec44f9b0..bee339d67 100644 --- a/internal/util/slices/filter.go +++ b/internal/util/filter/predicates.go @@ -1,29 +1,8 @@ -package slices - -import ( - "slices" -) +package filter // Predicate returns true if the object should be kept when filtering type Predicate[T any] func(entity T) bool -// Filter creates a new slice with all elements from s for which the test returns true -func Filter[T any](s []T, test Predicate[T]) []T { - out := make([]T, 0, len(s)) - for i := 0; i < len(s); i++ { - if test(s[i]) { - out = append(out, s[i]) - } - } - return slices.Clip(out) -} - -// RemoveInPlace removes all elements from s for which test returns true. -// Elements between new length and original length are zeroed out. -func RemoveInPlace[T any](s []T, test Predicate[T]) []T { - return slices.DeleteFunc(s, Not(test)) -} - func And[T any](predicates ...Predicate[T]) Predicate[T] { return func(obj T) bool { for _, predicate := range predicates { diff --git a/internal/util/filter/predicates_test.go b/internal/util/filter/predicates_test.go new file mode 100644 index 000000000..1e5b4302f --- /dev/null +++ b/internal/util/filter/predicates_test.go @@ -0,0 +1,147 @@ +package filter_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-controller/internal/util/filter" +) + +func TestAnd(t *testing.T) { + tests := []struct { + name string + predicates []filter.Predicate[int] + input int + want bool + }{ + { + name: "all true", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 10 }, + }, + input: 5, + want: true, + }, + { + name: "one false", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 5 }, + }, + input: 5, + want: false, + }, + { + name: "all false", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 10 }, + func(i int) bool { return i < 0 }, + }, + input: 5, + want: false, + }, + { + name: "no predicates", + predicates: []filter.Predicate[int]{}, + input: 5, + want: true, + }, + { + name: "nil predicates", + predicates: nil, + input: 5, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.And(tt.predicates...)(tt.input) + require.Equal(t, tt.want, got, "And() = %v, want %v", got, tt.want) + }) + } +} + +func TestOr(t *testing.T) { + tests := []struct { + name string + predicates []filter.Predicate[int] + input int + want bool + }{ + { + name: "all true", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 10 }, + }, + input: 5, + want: true, + }, + { + name: "one false", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 0 }, + func(i int) bool { return i < 5 }, + }, + input: 5, + want: true, + }, + { + name: "all false", + predicates: []filter.Predicate[int]{ + func(i int) bool { return i > 10 }, + func(i int) bool { return i < 0 }, + }, + input: 5, + want: false, + }, + { + name: "no predicates", + predicates: []filter.Predicate[int]{}, + input: 5, + want: false, + }, + { + name: "nil predicates", + predicates: nil, + input: 5, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.Or(tt.predicates...)(tt.input) + require.Equal(t, tt.want, got, "Or() = %v, want %v", got, tt.want) + }) + } +} + +func TestNot(t *testing.T) { + tests := []struct { + name string + predicate filter.Predicate[int] + input int + want bool + }{ + { + name: "predicate is true", + predicate: func(i int) bool { return i > 0 }, + input: 5, + want: false, + }, + { + name: "predicate is false", + predicate: func(i int) bool { return i > 3 }, + input: 2, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.Not(tt.predicate)(tt.input) + require.Equal(t, tt.want, got, "Not() = %v, want %v", got, tt.want) + }) + } +} diff --git a/internal/util/slices/filter_test.go b/internal/util/slices/filter_test.go deleted file mode 100644 index 1d7e7759b..000000000 --- a/internal/util/slices/filter_test.go +++ /dev/null @@ -1,239 +0,0 @@ -package slices_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/operator-framework/operator-controller/internal/util/slices" -) - -func TestAnd(t *testing.T) { - tests := []struct { - name string - predicates []slices.Predicate[int] - input int - want bool - }{ - { - name: "all true", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 0 }, - func(i int) bool { return i < 10 }, - }, - input: 5, - want: true, - }, - { - name: "one false", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 0 }, - func(i int) bool { return i < 5 }, - }, - input: 5, - want: false, - }, - { - name: "all false", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 10 }, - func(i int) bool { return i < 0 }, - }, - input: 5, - want: false, - }, - { - name: "no predicates", - predicates: []slices.Predicate[int]{}, - input: 5, - want: true, - }, - { - name: "nil predicates", - predicates: nil, - input: 5, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := slices.And(tt.predicates...)(tt.input) - require.Equal(t, tt.want, got, "And() = %v, want %v", got, tt.want) - }) - } -} - -func TestOr(t *testing.T) { - tests := []struct { - name string - predicates []slices.Predicate[int] - input int - want bool - }{ - { - name: "all true", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 0 }, - func(i int) bool { return i < 10 }, - }, - input: 5, - want: true, - }, - { - name: "one false", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 0 }, - func(i int) bool { return i < 5 }, - }, - input: 5, - want: true, - }, - { - name: "all false", - predicates: []slices.Predicate[int]{ - func(i int) bool { return i > 10 }, - func(i int) bool { return i < 0 }, - }, - input: 5, - want: false, - }, - { - name: "no predicates", - predicates: []slices.Predicate[int]{}, - input: 5, - want: false, - }, - { - name: "nil predicates", - predicates: nil, - input: 5, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := slices.Or(tt.predicates...)(tt.input) - require.Equal(t, tt.want, got, "Or() = %v, want %v", got, tt.want) - }) - } -} - -func TestNot(t *testing.T) { - tests := []struct { - name string - predicate slices.Predicate[int] - input int - want bool - }{ - { - name: "predicate is true", - predicate: func(i int) bool { return i > 0 }, - input: 5, - want: false, - }, - { - name: "predicate is false", - predicate: func(i int) bool { return i > 3 }, - input: 2, - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := slices.Not(tt.predicate)(tt.input) - require.Equal(t, tt.want, got, "Not() = %v, want %v", got, tt.want) - }) - } -} - -func TestFilter(t *testing.T) { - tests := []struct { - name string - slice []int - predicate slices.Predicate[int] - want []int - }{ - { - name: "all match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 0 }, - want: []int{1, 2, 3, 4, 5}, - }, - { - name: "some match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 3 }, - want: []int{4, 5}, - }, - { - name: "none match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "empty slice", - slice: []int{}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "nil slice", - slice: nil, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := slices.Filter(tt.slice, tt.predicate) - require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) - }) - } -} - -func TestRemoveInPlace(t *testing.T) { - tests := []struct { - name string - slice []int - predicate slices.Predicate[int] - want []int - }{ - { - name: "all match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 0 }, - want: []int{1, 2, 3, 4, 5}, - }, - { - name: "some match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 3 }, - want: []int{4, 5, 0, 0, 0}, - }, - { - name: "none match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 5 }, - want: []int{0, 0, 0, 0, 0}, - }, - { - name: "empty slice", - slice: []int{}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "nil slice", - slice: nil, - predicate: func(i int) bool { return i > 5 }, - want: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _ = slices.RemoveInPlace(tt.slice, tt.predicate) - require.Equal(t, tt.want, tt.slice, "Filter() = %v, want %v", tt.slice, tt.want) - }) - } -} diff --git a/internal/util/slices/slices.go b/internal/util/slices/slices.go new file mode 100644 index 000000000..4029244e3 --- /dev/null +++ b/internal/util/slices/slices.go @@ -0,0 +1,24 @@ +package slices + +import ( + "slices" + + "github.com/operator-framework/operator-controller/internal/util/filter" +) + +// Filter creates a new slice with all elements from s for which the test returns true +func Filter[T any](s []T, test filter.Predicate[T]) []T { + out := make([]T, 0, len(s)) + for i := 0; i < len(s); i++ { + if test(s[i]) { + out = append(out, s[i]) + } + } + return slices.Clip(out) +} + +// RemoveInPlace removes all elements from s for which test returns true. +// Elements between new length and original length are zeroed out. +func RemoveInPlace[T any](s []T, test filter.Predicate[T]) []T { + return slices.DeleteFunc(s, filter.Not(test)) +} diff --git a/internal/util/slices/slices_test.go b/internal/util/slices/slices_test.go new file mode 100644 index 000000000..34de3db4b --- /dev/null +++ b/internal/util/slices/slices_test.go @@ -0,0 +1,102 @@ +package slices_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/operator-framework/operator-controller/internal/util/filter" + "github.com/operator-framework/operator-controller/internal/util/slices" +) + +func TestFilter(t *testing.T) { + tests := []struct { + name string + slice []int + predicate filter.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := slices.Filter(tt.slice, tt.predicate) + require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) + }) + } +} + +func TestRemoveInPlace(t *testing.T) { + tests := []struct { + name string + slice []int + predicate filter.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5, 0, 0, 0}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: []int{0, 0, 0, 0, 0}, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _ = slices.RemoveInPlace(tt.slice, tt.predicate) + require.Equal(t, tt.want, tt.slice, "Filter() = %v, want %v", tt.slice, tt.want) + }) + } +} From 5034d99116d17085fb649eda05e901a0867a6546 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 10 Feb 2025 18:02:17 +0100 Subject: [PATCH 5/5] Address reviewer comments Signed-off-by: Per Goncalves da Silva --- .../filter/bundle_predicates.go | 6 +- internal/catalogmetadata/filter/successors.go | 8 +- .../catalogmetadata/filter/successors_test.go | 4 +- internal/resolve/catalog.go | 3 +- .../util/filter/{predicates.go => filter.go} | 20 ++++ .../{predicates_test.go => filter_test.go} | 92 ++++++++++++++++ internal/util/slices/slices.go | 24 ----- internal/util/slices/slices_test.go | 102 ------------------ 8 files changed, 122 insertions(+), 137 deletions(-) rename internal/util/filter/{predicates.go => filter.go} (52%) rename internal/util/filter/{predicates_test.go => filter_test.go} (58%) delete mode 100644 internal/util/slices/slices.go delete mode 100644 internal/util/slices/slices_test.go diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index c73dfec98..74e933daa 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -6,10 +6,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-controller/internal/bundleutil" - filterutil "github.com/operator-framework/operator-controller/internal/util/filter" + "github.com/operator-framework/operator-controller/internal/util/filter" ) -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) filterutil.Predicate[declcfg.Bundle] { +func InMastermindsSemverRange(semverRange *mmsemver.Constraints) filter.Predicate[declcfg.Bundle] { return func(b declcfg.Bundle) bool { bVersion, err := bundleutil.GetVersion(b) if err != nil { @@ -27,7 +27,7 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) filterutil.Pred } } -func InAnyChannel(channels ...declcfg.Channel) filterutil.Predicate[declcfg.Bundle] { +func InAnyChannel(channels ...declcfg.Channel) filter.Predicate[declcfg.Bundle] { return func(bundle declcfg.Bundle) bool { for _, ch := range channels { for _, entry := range ch.Entries { diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go index a9ae50147..128c80cca 100644 --- a/internal/catalogmetadata/filter/successors.go +++ b/internal/catalogmetadata/filter/successors.go @@ -9,10 +9,10 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" ocv1 "github.com/operator-framework/operator-controller/api/v1" - filterutil "github.com/operator-framework/operator-controller/internal/util/filter" + "github.com/operator-framework/operator-controller/internal/util/filter" ) -func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filterutil.Predicate[declcfg.Bundle], error) { +func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version) if err != nil { return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err) @@ -29,13 +29,13 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann } // We need either successors or current version (no upgrade) - return filterutil.Or( + return filter.Or( successorsPredicate, InMastermindsSemverRange(installedVersionConstraint), ), nil } -func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filterutil.Predicate[declcfg.Bundle], error) { +func legacySuccessor(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (filter.Predicate[declcfg.Bundle], error) { installedBundleVersion, err := bsemver.Parse(installedBundle.Version) if err != nil { return nil, fmt.Errorf("error parsing installed bundle version: %w", err) diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go index 90e772d77..ab3beff7c 100644 --- a/internal/catalogmetadata/filter/successors_test.go +++ b/internal/catalogmetadata/filter/successors_test.go @@ -16,7 +16,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/bundleutil" "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" - slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" + "github.com/operator-framework/operator-controller/internal/util/filter" ) func TestSuccessorsPredicate(t *testing.T) { @@ -161,7 +161,7 @@ func TestSuccessorsPredicate(t *testing.T) { for _, bundle := range bundleSet { allBundles = append(allBundles, bundle) } - result := slicesutil.RemoveInPlace(allBundles, successors) + result := filter.InPlace(allBundles, successors) // sort before comparison for stable order slices.SortFunc(result, compare.ByVersion) diff --git a/internal/resolve/catalog.go b/internal/resolve/catalog.go index 9c94eddef..85a404cac 100644 --- a/internal/resolve/catalog.go +++ b/internal/resolve/catalog.go @@ -23,7 +23,6 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogmetadata/compare" "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" filterutil "github.com/operator-framework/operator-controller/internal/util/filter" - slicesutil "github.com/operator-framework/operator-controller/internal/util/slices" ) type ValidationFunc func(*declcfg.Bundle) error @@ -120,7 +119,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } // Apply the predicates to get the candidate bundles - packageFBC.Bundles = slicesutil.RemoveInPlace(packageFBC.Bundles, filterutil.And(predicates...)) + packageFBC.Bundles = filterutil.InPlace(packageFBC.Bundles, filterutil.And(predicates...)) cs.MatchedBundles = len(packageFBC.Bundles) if len(packageFBC.Bundles) == 0 { return nil diff --git a/internal/util/filter/predicates.go b/internal/util/filter/filter.go similarity index 52% rename from internal/util/filter/predicates.go rename to internal/util/filter/filter.go index bee339d67..96d60cfcd 100644 --- a/internal/util/filter/predicates.go +++ b/internal/util/filter/filter.go @@ -1,5 +1,7 @@ package filter +import "slices" + // Predicate returns true if the object should be kept when filtering type Predicate[T any] func(entity T) bool @@ -30,3 +32,21 @@ func Not[T any](predicate Predicate[T]) Predicate[T] { return !predicate(obj) } } + +// Filter creates a new slice with all elements from s for which the test returns true +func Filter[T any](s []T, test Predicate[T]) []T { + var out []T + for i := 0; i < len(s); i++ { + if test(s[i]) { + out = append(out, s[i]) + } + } + return slices.Clip(out) +} + +// InPlace modifies s by removing any element for which test returns false. +// InPlace zeroes the elements between the new length and the original length in s. +// The returned slice is of the new length. +func InPlace[T any](s []T, test Predicate[T]) []T { + return slices.DeleteFunc(s, Not(test)) +} diff --git a/internal/util/filter/predicates_test.go b/internal/util/filter/filter_test.go similarity index 58% rename from internal/util/filter/predicates_test.go rename to internal/util/filter/filter_test.go index 1e5b4302f..2622b4adf 100644 --- a/internal/util/filter/predicates_test.go +++ b/internal/util/filter/filter_test.go @@ -145,3 +145,95 @@ func TestNot(t *testing.T) { }) } } + +func TestFilter(t *testing.T) { + tests := []struct { + name string + slice []int + predicate filter.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.Filter(tt.slice, tt.predicate) + require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) + }) + } +} + +func TestInPlace(t *testing.T) { + tests := []struct { + name string + slice []int + predicate filter.Predicate[int] + want []int + }{ + { + name: "all match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 0 }, + want: []int{1, 2, 3, 4, 5}, + }, + { + name: "some match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 3 }, + want: []int{4, 5}, + }, + { + name: "none match", + slice: []int{1, 2, 3, 4, 5}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "empty slice", + slice: []int{}, + predicate: func(i int) bool { return i > 5 }, + want: []int{}, + }, + { + name: "nil slice", + slice: nil, + predicate: func(i int) bool { return i > 5 }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filter.InPlace(tt.slice, tt.predicate) + require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) + }) + } +} diff --git a/internal/util/slices/slices.go b/internal/util/slices/slices.go deleted file mode 100644 index 4029244e3..000000000 --- a/internal/util/slices/slices.go +++ /dev/null @@ -1,24 +0,0 @@ -package slices - -import ( - "slices" - - "github.com/operator-framework/operator-controller/internal/util/filter" -) - -// Filter creates a new slice with all elements from s for which the test returns true -func Filter[T any](s []T, test filter.Predicate[T]) []T { - out := make([]T, 0, len(s)) - for i := 0; i < len(s); i++ { - if test(s[i]) { - out = append(out, s[i]) - } - } - return slices.Clip(out) -} - -// RemoveInPlace removes all elements from s for which test returns true. -// Elements between new length and original length are zeroed out. -func RemoveInPlace[T any](s []T, test filter.Predicate[T]) []T { - return slices.DeleteFunc(s, filter.Not(test)) -} diff --git a/internal/util/slices/slices_test.go b/internal/util/slices/slices_test.go deleted file mode 100644 index 34de3db4b..000000000 --- a/internal/util/slices/slices_test.go +++ /dev/null @@ -1,102 +0,0 @@ -package slices_test - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/operator-framework/operator-controller/internal/util/filter" - "github.com/operator-framework/operator-controller/internal/util/slices" -) - -func TestFilter(t *testing.T) { - tests := []struct { - name string - slice []int - predicate filter.Predicate[int] - want []int - }{ - { - name: "all match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 0 }, - want: []int{1, 2, 3, 4, 5}, - }, - { - name: "some match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 3 }, - want: []int{4, 5}, - }, - { - name: "none match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "empty slice", - slice: []int{}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "nil slice", - slice: nil, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := slices.Filter(tt.slice, tt.predicate) - require.Equal(t, tt.want, got, "Filter() = %v, want %v", got, tt.want) - }) - } -} - -func TestRemoveInPlace(t *testing.T) { - tests := []struct { - name string - slice []int - predicate filter.Predicate[int] - want []int - }{ - { - name: "all match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 0 }, - want: []int{1, 2, 3, 4, 5}, - }, - { - name: "some match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 3 }, - want: []int{4, 5, 0, 0, 0}, - }, - { - name: "none match", - slice: []int{1, 2, 3, 4, 5}, - predicate: func(i int) bool { return i > 5 }, - want: []int{0, 0, 0, 0, 0}, - }, - { - name: "empty slice", - slice: []int{}, - predicate: func(i int) bool { return i > 5 }, - want: []int{}, - }, - { - name: "nil slice", - slice: nil, - predicate: func(i int) bool { return i > 5 }, - want: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _ = slices.RemoveInPlace(tt.slice, tt.predicate) - require.Equal(t, tt.want, tt.slice, "Filter() = %v, want %v", tt.slice, tt.want) - }) - } -}