diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 98e3ab4cd..74e933daa 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" + "github.com/operator-framework/operator-controller/internal/util/filter" ) -func InMastermindsSemverRange(semverRange *mmsemver.Constraints) 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 { @@ -26,7 +27,7 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[declc } } -func InAnyChannel(channels ...declcfg.Channel) 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/filter_test.go b/internal/catalogmetadata/filter/filter_test.go deleted file mode 100644 index 77d12c99f..000000000 --- a/internal/catalogmetadata/filter/filter_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package filter_test - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/operator-framework/operator-registry/alpha/declcfg" - - "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter" -) - -func TestFilter(t *testing.T) { - for _, tt := range []struct { - name string - predicate filter.Predicate[declcfg.Bundle] - want []declcfg.Bundle - }{ - { - name: "simple filter with one predicate", - predicate: func(bundle declcfg.Bundle) bool { - return bundle.Name == "extension1.v1" - }, - want: []declcfg.Bundle{ - {Name: "extension1.v1", Package: "extension1", Image: "fake1"}, - }, - }, - { - name: "filter with Not predicate", - predicate: filter.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: "filter with And predicate", - predicate: filter.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: "filter with Or predicate", - predicate: filter.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"}, - }, - }, - } { - 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"}, - } - - actual := filter.Filter(in, tt.predicate) - assert.Equal(t, tt.want, actual) - }) - } -} diff --git a/internal/catalogmetadata/filter/successors.go b/internal/catalogmetadata/filter/successors.go index cd2ea4f5b..128c80cca 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" + "github.com/operator-framework/operator-controller/internal/util/filter" ) -func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Channel) (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) @@ -28,13 +29,13 @@ func SuccessorsOf(installedBundle ocv1.BundleMetadata, channels ...declcfg.Chann } // We need either successors or current version (no upgrade) - return Or( + return filter.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) (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 2a0a53348..ab3beff7c 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" + "github.com/operator-framework/operator-controller/internal/util/filter" ) 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 := 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 31b3d15ec..85a404cac 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" ) type ValidationFunc func(*declcfg.Bundle) error @@ -75,7 +76,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{ @@ -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 []filterutil.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 = filterutil.InPlace(packageFBC.Bundles, filterutil.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/filter/filter.go similarity index 52% rename from internal/catalogmetadata/filter/filter.go rename to internal/util/filter/filter.go index 9074c926d..96d60cfcd 100644 --- a/internal/catalogmetadata/filter/filter.go +++ b/internal/util/filter/filter.go @@ -1,17 +1,10 @@ package filter -import ( - "slices" -) +import "slices" // 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)) -} - func And[T any](predicates ...Predicate[T]) Predicate[T] { return func(obj T) bool { for _, predicate := range predicates { @@ -39,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/filter_test.go b/internal/util/filter/filter_test.go new file mode 100644 index 000000000..2622b4adf --- /dev/null +++ b/internal/util/filter/filter_test.go @@ -0,0 +1,239 @@ +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) + }) + } +} + +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) + }) + } +}