🌱 Refactor filter package (#1734)

* Move filter code to util/slices

Signed-off-by: Per Goncalves da Silva <[email protected]>

* Rename Filter to RemoveInPlace

Signed-off-by: Per Goncalves da Silva <[email protected]>

* Add Filter function and update unit tests

Signed-off-by: Per Goncalves da Silva <[email protected]>

* Refactor slice utils package structure

Signed-off-by: Per Goncalves da Silva <[email protected]>

* Address reviewer comments

Signed-off-by: Per Goncalves da Silva <[email protected]>

---------

Signed-off-by: Per Goncalves da Silva <[email protected]>
Co-authored-by: Per Goncalves da Silva <[email protected]>

Author: perdasilva

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 {

internal/catalogmetadata/filter/filter_test.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)

internal/catalogmetadata/filter/successors.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)

internal/catalogmetadata/filter/successors_test.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

internal/resolve/catalog.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))
+}