Address feedbacks on registry client interface

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

pkg/controller/registry/grpc/source.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
-	"github.com/operator-framework/operator-registry/pkg/client"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -191,8 +190,8 @@ func (s *SourceStore) Remove(key resolver.CatalogKey) error {
 	return source.Conn.Close()
 }
 
-func (s *SourceStore) AsClients(namespaces ...string) map[resolver.CatalogKey]registry.RegistryClientInterface {
-	refs := map[resolver.CatalogKey]registry.RegistryClientInterface{}
+func (s *SourceStore) AsClients(namespaces ...string) map[resolver.CatalogKey]registry.ClientInterface {
+	refs := map[resolver.CatalogKey]registry.ClientInterface{}
 	s.sourcesLock.RLock()
 	defer s.sourcesLock.RUnlock()
 	for key, source := range s.sources {
@@ -201,7 +200,7 @@ func (s *SourceStore) AsClients(namespaces ...string) map[resolver.CatalogKey]re
 		}
 		for _, namespace := range namespaces {
 			if key.Namespace == namespace {
-				refs[key] = registry.NewRegistryClient(client.NewClientFromConn(source.Conn))
+				refs[key] = registry.NewClientFromConn(source.Conn)
 			}
 		}
 		refs[key] = client.NewClientFromConn(source.Conn)

pkg/controller/registry/registry_client.go

@@ -1,35 +1,42 @@
-//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o resolver/fakes/fake_registry_client_interface.go . RegistryClientInterface
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -o resolver/fakes/fake_registry_client_interface.go . ClientInterface
 package registry
 
 import (
 	"context"
 	"fmt"
 	"io"
 
+	"google.golang.org/grpc"
+
 	registryapi "github.com/operator-framework/operator-registry/pkg/api"
 	"github.com/operator-framework/operator-registry/pkg/client"
 	opregistry "github.com/operator-framework/operator-registry/pkg/registry"
 )
 
+// ChannelEntryStream interface
 type ChannelEntryStream interface {
 	Recv() (*registryapi.ChannelEntry, error)
 }
 
-type RegistryClientInterface interface {
+// ClientInterface that extends client.Interface
+type ClientInterface interface {
 	client.Interface
-	FindBundleThatProvides(ctx context.Context, group, version, kind, pkgName string) (*registryapi.Bundle, error)
+	FindBundleThatProvides(ctx context.Context, group, version, kind, excludedPkgName string) (*registryapi.Bundle, error)
 	GetLatestChannelEntriesThatProvide(ctx context.Context, group, version, kind string) (*ChannelEntryIterator, error)
 }
 
+// ChannelEntryIterator struct
 type ChannelEntryIterator struct {
 	stream ChannelEntryStream
 	error  error
 }
 
+// NewChannelEntryIterator returns a new ChannelEntryIterator
 func NewChannelEntryIterator(stream ChannelEntryStream) *ChannelEntryIterator {
 	return &ChannelEntryIterator{stream: stream}
 }
 
+// Next returns the next Channel Entry in the grpc stream
 func (ceit *ChannelEntryIterator) Next() *registryapi.ChannelEntry {
 	if ceit.error != nil {
 		return nil
@@ -48,21 +55,23 @@ func (ceit *ChannelEntryIterator) Error() error {
 	return ceit.error
 }
 
-type RegistryClient struct {
-	client.Interface
-	Client *client.Client
+// Client struct with a registry client embedded
+type Client struct {
+	*client.Client
 }
 
-func NewRegistryClient(client *client.Client) *RegistryClient {
-	return &RegistryClient{client, client}
+// NewClientFromConn returns the next Channel Entry in the grpc stream
+func NewClientFromConn(conn *grpc.ClientConn) *Client {
+	return &Client{
+		Client: client.NewClientFromConn(conn),
+	}
 }
 
-var _ RegistryClientInterface = &RegistryClient{}
-var _ client.Interface = &RegistryClient{}
+var _ ClientInterface = &Client{}
 
 // GetLatestChannelEntriesThatProvide uses registry client to get a list of
 // latest channel entries that provide the requested API (via an iterator)
-func (rc *RegistryClient) GetLatestChannelEntriesThatProvide(ctx context.Context, group, version, kind string) (*ChannelEntryIterator, error) {
+func (rc *Client) GetLatestChannelEntriesThatProvide(ctx context.Context, group, version, kind string) (*ChannelEntryIterator, error) {
 	stream, err := rc.Client.Registry.GetLatestChannelEntriesThatProvide(ctx, &registryapi.GetLatestProvidersRequest{Group: group, Version: version, Kind: kind})
 	if err != nil {
 		return nil, err
@@ -72,14 +81,14 @@ func (rc *RegistryClient) GetLatestChannelEntriesThatProvide(ctx context.Context
 
 // FindBundleThatProvides returns a bundle that provides the request API and
 // doesn't belong to the provided package
-func (rc *RegistryClient) FindBundleThatProvides(ctx context.Context, group, version, kind, pkgName string) (*registryapi.Bundle, error) {
+func (rc *Client) FindBundleThatProvides(ctx context.Context, group, version, kind, excludedPkgName string) (*registryapi.Bundle, error) {
 	it, err := rc.GetLatestChannelEntriesThatProvide(ctx, group, version, kind)
 	if err != nil {
 		return nil, err
 	}
-	entry := rc.filterChannelEntries(it, pkgName)
+	entry := rc.filterChannelEntries(it, excludedPkgName)
 	if entry == nil {
-		return nil, fmt.Errorf("Unable to find a channel entry which doesn't belong to package %s", pkgName)
+		return nil, fmt.Errorf("Unable to find a channel entry which doesn't belong to package %s", excludedPkgName)
 	}
 	bundle, err := rc.Client.Registry.GetBundle(ctx, &registryapi.GetBundleRequest{PkgName: entry.PackageName, ChannelName: entry.ChannelName, CsvName: entry.BundleName})
 	if err != nil {
@@ -91,10 +100,10 @@ func (rc *RegistryClient) FindBundleThatProvides(ctx context.Context, group, ver
 // FilterChannelEntries filters out a channel entries that provide the requested
 // API and come from the same package with original operator and returns the
 // first entry on the list
-func (rc *RegistryClient) filterChannelEntries(it *ChannelEntryIterator, pkgName string) *opregistry.ChannelEntry {
+func (rc *Client) filterChannelEntries(it *ChannelEntryIterator, excludedPkgName string) *opregistry.ChannelEntry {
 	var entries []*opregistry.ChannelEntry
 	for e := it.Next(); e != nil; e = it.Next() {
-		if e.PackageName != pkgName {
+		if e.PackageName != excludedPkgName {
 			entry := &opregistry.ChannelEntry{
 				PackageName: e.PackageName,
 				ChannelName: e.ChannelName,