Address reviewer comments

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Author: Per Goncalves da Silva

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 {

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)

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)

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

internal/util/filter/predicates.go renamed to 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,20 @@ 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 keeps all elements from s for which test returns true and removes all others.
+// Elements between new length and original length are zeroed out.
+func InPlace[T any](s []T, test Predicate[T]) []T {
+	return slices.DeleteFunc(s, Not(test))
+}

internal/util/filter/predicates_test.go renamed to 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)
+		})
+	}
+}