Updating based on feedback

* Cleaning up imports
* Updating names to be more clear
* code optimizations.

Author: Shawn Hurley

cmd/opm/main.go

@@ -5,13 +5,13 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 
 	"github.com/operator-framework/operator-registry/cmd/opm/alpha"
 	"github.com/operator-framework/operator-registry/cmd/opm/index"
 	"github.com/operator-framework/operator-registry/cmd/opm/registry"
 	"github.com/operator-framework/operator-registry/cmd/opm/version"
 	registrylib "github.com/operator-framework/operator-registry/pkg/registry"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
 
 func main() {
@@ -37,14 +37,16 @@ func main() {
 	}
 
 	if err := rootCmd.Execute(); err != nil {
-		if agg, ok := err.(utilerrors.Aggregate); ok {
-			for _, e := range agg.Errors() {
-				if _, ok := e.(registrylib.BundleAlreadyAddedError); ok {
-					os.Exit(2)
-				}
-				if _, ok := e.(registrylib.BundleAlreadyInDatabaseError); ok {
-					os.Exit(3)
-				}
+		agg, ok := err.(utilerrors.Aggregate)
+		if !ok {
+			os.Exit(1)
+		}
+		for _, e := range agg.Errors() {
+			if _, ok := e.(registrylib.BundleImageAlreadyAddedErr); ok {
+				os.Exit(2)
+			}
+			if _, ok := e.(registrylib.PackageVersionAlreadyAddedErr); ok {
+				os.Exit(3)
 			}
 		}
 		os.Exit(1)

pkg/lib/registry/registry.go

@@ -8,14 +8,13 @@ import (
 	"io/ioutil"
 	"os"
 
-	"github.com/operator-framework/operator-registry/pkg/containertools"
-	"github.com/operator-framework/operator-registry/pkg/image/execregistry"
-
 	"github.com/sirupsen/logrus"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 
+	"github.com/operator-framework/operator-registry/pkg/containertools"
 	"github.com/operator-framework/operator-registry/pkg/image"
 	"github.com/operator-framework/operator-registry/pkg/image/containerdregistry"
+	"github.com/operator-framework/operator-registry/pkg/image/execregistry"
 	"github.com/operator-framework/operator-registry/pkg/registry"
 	"github.com/operator-framework/operator-registry/pkg/sqlite"
 )

pkg/registry/populator.go

@@ -65,9 +65,9 @@ func (i *DirectoryPopulator) Populate(mode Mode) error {
 
 func (i *DirectoryPopulator) globalSanityCheck(imagesToAdd []*ImageInput) error {
 	var errs []error
-	images := make(map[string]bool)
+	images := make(map[string]struct{})
 	for _, image := range imagesToAdd {
-		images[image.bundle.BundleImage] = true
+		images[image.bundle.BundleImage] = struct{}{}
 	}
 
 	for _, image := range imagesToAdd {
@@ -77,10 +77,10 @@ func (i *DirectoryPopulator) globalSanityCheck(imagesToAdd []*ImageInput) error
 			// Or that this is the first time the package is loaded.
 			return nil
 		}
-		isExactImage := false
 		for _, bundlePath := range bundlePaths {
 			if _, ok := images[bundlePath]; ok {
-				isExactImage = true
+				errs = append(errs, BundleImageAlreadyAddedErr{ErrorString: fmt.Sprintf("Bundle %s already exists", image.bundle.BundleImage)})
+				continue
 			}
 		}
 		for _, channel := range image.bundle.Channels {
@@ -89,15 +89,10 @@ func (i *DirectoryPopulator) globalSanityCheck(imagesToAdd []*ImageInput) error
 				// Assume that if we can not find a bundle for the package, channel and or CSV Name that this is safe to add
 				continue
 			}
-			if bundle != nil && !isExactImage {
+			if bundle != nil {
 				// raise error that this package + channel + csv combo is already in the db
-				errs = append(errs, BundleAlreadyInDatabaseError{ErrorString: "Bundle already added that provides package and csv"})
-				continue
-			}
-			if bundle != nil && isExactImage {
-				// raise error that this bundle and channel is already in the db
-				errs = append(errs, BundleAlreadyAddedError{ErrorString: fmt.Sprintf("Bundle %s already exists", image.bundle.BundleImage)})
-				continue
+				errs = append(errs, PackageVersionAlreadyAddedErr{ErrorString: "Bundle already added that provides package and csv"})
+				break
 			}
 		}
 	}

pkg/registry/populator_test.go

@@ -423,7 +423,7 @@ func TestImageLoading(t *testing.T) {
 				},
 			},
 			wantErr: true,
-			err:     registry.BundleAlreadyInDatabaseError{},
+			err:     registry.PackageVersionAlreadyAddedErr{},
 		},
 		{
 			name: "AddExactBundleAlreadyExists",
@@ -458,7 +458,7 @@ func TestImageLoading(t *testing.T) {
 				},
 			},
 			wantErr: true,
-			err:     registry.BundleAlreadyAddedError{},
+			err:     registry.BundleImageAlreadyAddedErr{},
 		},
 	}
 	for _, tt := range tests {

pkg/registry/types.go

@@ -14,21 +14,21 @@ var (
 	ErrPackageNotInDatabase = errors.New("Package not in database")
 )
 
-// BundleAlreadyAddedError is an error that describes a bundle is already added
-type BundleAlreadyAddedError struct {
+// BundleImageAlreadyAddedErr is an error that describes a bundle is already added
+type BundleImageAlreadyAddedErr struct {
 	ErrorString string
 }
 
-func (e BundleAlreadyAddedError) Error() string {
+func (e BundleImageAlreadyAddedErr) Error() string {
 	return e.ErrorString
 }
 
-// BundleAlreadyInDatabseError is an error that describes that a bundle that is already in the databse that provides this CSV
-type BundleAlreadyInDatabaseError struct {
+// PackageVersionAlreadyAddedErr is an error that describes that a bundle that is already in the databse that provides this package and version
+type PackageVersionAlreadyAddedErr struct {
 	ErrorString string
 }
 
-func (e BundleAlreadyInDatabaseError) Error() string {
+func (e PackageVersionAlreadyAddedErr) Error() string {
 	return e.ErrorString
 }