more review updates

Signed-off-by: grokspawn <jordan@nimblewidget.com>

Author: grokspawn

alpha/declcfg/declcfg.go

@@ -5,15 +5,26 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 
+	"github.com/blang/semver/v4"
 	"golang.org/x/text/cases"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 
+	"github.com/operator-framework/operator-registry/alpha/model"
 	"github.com/operator-framework/operator-registry/alpha/property"
 	prettyunmarshaler "github.com/operator-framework/operator-registry/pkg/prettyunmarshaler"
 )
 
+// Re-export VersionRelease/Release types/functions from model package to make it possible for users to only include this package and avoid import cycles
+type (
+	Release        = model.Release
+	VersionRelease = model.VersionRelease
+)
+
+var NewRelease = model.NewRelease
+
 const (
 	SchemaPackage     = "olm.package"
 	SchemaChannel     = "olm.channel"
@@ -206,3 +217,76 @@ func (destination *DeclarativeConfig) Merge(src *DeclarativeConfig) {
 	destination.Others = append(destination.Others, src.Others...)
 	destination.Deprecations = append(destination.Deprecations, src.Deprecations...)
 }
+
+// order by version, then
+// release, if present
+func (b *Bundle) Compare(other *Bundle) int {
+	if b.Name == other.Name {
+		return 0
+	}
+	avr, err := b.VersionRelease()
+	if err != nil {
+		return 0
+	}
+	otherVr, err := other.VersionRelease()
+	if err != nil {
+		return 0
+	}
+	return avr.Compare(otherVr)
+}
+
+// constructs a VersionRelease from the olm.package property of the bundle
+// this handles the cases where the property is present, missing, or duplicated
+// if a release field is present in the property, it is used as-is
+// if it is NOT present in the property, but the version field contains build metadata,
+// we attempt to convert the build metadata into a release and strip the build metadata from the version.
+// This is to support bundles that use the legacy approach of encoding release information in the build metadata field of the version
+func (b *Bundle) VersionRelease() (*VersionRelease, error) {
+	var (
+		vr *VersionRelease
+	)
+	// loop over all properties, and do not break if we find a package property, in order to check for duplicates
+	for _, prop := range b.Properties {
+		switch prop.Type {
+		case property.TypePackage:
+			var p property.Package
+
+			// if we encounter more than one olm.package property, return an error
+			if vr != nil {
+				return nil, fmt.Errorf("must be exactly one property of type %q", SchemaPackage)
+			}
+
+			if err := json.Unmarshal(prop.Value, &p); err != nil {
+				return nil, fmt.Errorf("unable to unmarshal \"olm.package\" property for bundle %q: %v", b.Name, err)
+			}
+			pv, err := semver.Parse(p.Version)
+			if err != nil {
+				return nil, fmt.Errorf("invalid semver version %q in \"olm.package\" property for bundle %q: %v", p.Version, b.Name, err)
+			}
+			pr, err := NewRelease(p.Release)
+			if err != nil {
+				return nil, fmt.Errorf("invalid release %q in \"olm.package\" property for bundle %q: %v", p.Release, b.Name, err)
+			}
+			vr = &VersionRelease{
+				Version: pv,
+				Release: pr,
+			}
+		}
+	}
+	if vr == nil {
+		return nil, fmt.Errorf("no \"olm.package\" property found for bundle %q", b.Name)
+	}
+
+	// if the bundle's release isn't provided, see if we can use the legacy build metadata release approach to identify a release
+	// if successful, remove the build metadata from the version
+	if len(vr.Release) == 0 && vr.Version.Build != nil {
+		newrel, err := NewRelease(strings.Join(vr.Version.Build, "."))
+		if err != nil {
+			return nil, fmt.Errorf("unable to convert build metadata to release for bundle %q: %v", b.Name, err)
+		}
+		vr.Release = newrel
+		vr.Version.Build = nil
+	}
+
+	return vr, nil
+}

alpha/declcfg/declcfg_to_model.go

@@ -146,16 +146,12 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 		}
 
 		// Parse release version from the package property.
-		var relver semver.Version
+		var relver model.Release
 		if props.Packages[0].Release != "" {
-			relver, err = semver.Parse(fmt.Sprintf("0.0.0-%s", props.Packages[0].Release))
+			relver, err = model.NewRelease(props.Packages[0].Release)
 			if err != nil {
 				return nil, fmt.Errorf("error parsing bundle %q release version %q: %v", b.Name, props.Packages[0].Release, err)
 			}
-			// only need to check for build metadata since we are using explicit zero major, minor, and patch versions above
-			if len(relver.Build) != 0 {
-				return nil, fmt.Errorf("bundle %q release version %q cannot contain build metadata", b.Name, props.Packages[0].Release)
-			}
 		}
 
 		channelDefinedEntries[b.Package] = channelDefinedEntries[b.Package].Delete(b.Name)
@@ -170,7 +166,8 @@ func ConvertToModel(cfg DeclarativeConfig) (model.Model, error) {
 				mb.Objects = b.Objects
 				mb.PropertiesP = props
 				mb.Version = ver
-				mb.Release = relver
+				// TODO: Jordan: follow-up will evolve the internal types for more consistent use of VersionRelease
+				mb.Release = semver.Version{Pre: relver}
 			}
 		}
 		if !found {

alpha/declcfg/declcfg_to_model_test.go

@@ -444,7 +444,7 @@ func TestConvertToModel(t *testing.T) {
 		},
 		{
 			name:      "Error/InvalidReleaseVersion",
-			assertion: hasError(`error parsing bundle "foo.v0.1.0" release version "!!!": Invalid character(s) found in prerelease "!!!"`),
+			assertion: hasError(`error parsing bundle "foo.v0.1.0" release version "!!!": invalid release "!!!": segment 0: Invalid character(s) found in prerelease "!!!"`),
 			cfg: DeclarativeConfig{
 				Packages: []Package{newTestPackage("foo", "alpha", svgSmallCircle)},
 				Channels: []Channel{newTestChannel("foo", "alpha", ChannelEntry{Name: testBundleName("foo", "0.1.0")})},