Merge pull request #374 from mq-cloudpak/amf-1831-version-length

Handle multi-digit portions of VRMF

internal/mqversion/mqversion.go

@@ -18,6 +18,7 @@ package mqversion
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/ibm-messaging/mq-container/internal/command"
@@ -38,14 +39,59 @@ func Compare(checkVersion string) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	// trim any suffix from MQ version x.x.x.x
+
-	currentVersion = currentVersion[0:7]
+	currentVRMF, err := parseVRMF(currentVersion)
-	if currentVersion < checkVersion {
+	if err != nil {
-		return -1, nil
+		return 0, err
-	} else if currentVersion == checkVersion {
-		return 0, nil
-	} else if currentVersion > checkVersion {
-		return 1, nil
 	}
-	return 0, fmt.Errorf("Failed to compare MQ versions")
+	compareVRMF, err := parseVRMF(checkVersion)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse compare version: %w", err)
+	}
+	return currentVRMF.compare(*compareVRMF), nil
+}
+
+type vrmf [4]int
+
+func (v vrmf) String() string {
+	return fmt.Sprintf("%d.%d.%d.%d", v[0], v[1], v[2], v[3])
+}
+
+func (v vrmf) compare(to vrmf) int {
+	for idx := 0; idx < 4; idx++ {
+		if v[idx] < to[idx] {
+			return -1
+		}
+		if v[idx] > to[idx] {
+			return 1
+		}
+	}
+	return 0
+}
+
+func parseVRMF(vrmfString string) (*vrmf, error) {
+	versionParts := strings.Split(vrmfString, ".")
+	if len(versionParts) != 4 {
+		return nil, fmt.Errorf("incorrect number of parts to version string: expected 4, got %d", len(versionParts))
+	}
+	vmrfPartNames := []string{"version", "release", "minor", "fix"}
+	parsed := vrmf{}
+	for idx, value := range versionParts {
+		partName := vmrfPartNames[idx]
+		if value == "" {
+			return nil, fmt.Errorf("empty %s found in VRMF", partName)
+		}
+		val, err := strconv.Atoi(value)
+		if err != nil {
+			return nil, fmt.Errorf("non-numeric %s found in VRMF", partName)
+		}
+		if val < 0 {
+			return nil, fmt.Errorf("negative %s found in VRMF", partName)
+		}
+		if idx == 0 && val == 0 {
+			return nil, fmt.Errorf("zero value for version not allowed")
+		}
+		parsed[idx] = val
+	}
+	return &parsed, nil
 }

internal/mqversion/mqversion_test.go

@@ -16,10 +16,13 @@ limitations under the License.
 
 package mqversion
 
-import "testing"
+import (
+	"fmt"
+	"testing"
+)
 
 func TestCompareLower(t *testing.T) {
-	checkVersion := "9.9.9.9"
+	checkVersion := "99.99.99.99"
 	mqVersionCheck, err := Compare(checkVersion)
 	if err != nil {
 		t.Fatalf("Failed to compare MQ versions: %v", err)
@@ -53,3 +56,92 @@ func TestCompareEqual(t *testing.T) {
 		t.Errorf("MQ version compare result failed. Expected 0, Got %v", mqVersionCheck)
 	}
 }
+
+func TestVersionValid(t *testing.T) {
+	checkVersion, err := Get()
+	if err != nil {
+		t.Fatalf("Failed to get current MQ version: %v", err)
+	}
+	_, err = parseVRMF(checkVersion)
+	if err != nil {
+		t.Fatalf("Validation of MQ version failed: %v", err)
+	}
+}
+
+func TestValidVRMF(t *testing.T) {
+	validVRMFs := map[string]vrmf{
+		"1.0.0.0":         {1, 0, 0, 0},
+		"10.0.0.0":        {10, 0, 0, 0},
+		"1.10.0.0":        {1, 10, 0, 0},
+		"1.0.10.0":        {1, 0, 10, 0},
+		"1.0.0.10":        {1, 0, 0, 10},
+		"999.998.997.996": {999, 998, 997, 996},
+	}
+	for test, expect := range validVRMFs {
+		t.Run(test, func(t *testing.T) {
+			parsed, err := parseVRMF(test)
+			if err != nil {
+				t.Fatalf("Unexpectedly failed to parse VRMF '%s': %s", test, err.Error())
+			}
+			if *parsed != expect {
+				t.Fatalf("VRMF not parsed as expected. Expected '%v', got '%v'", parsed, expect)
+			}
+		})
+	}
+}
+
+func TestInvalidVRMF(t *testing.T) {
+	invalidVRMFs := []string{
+		"not-a-number",
+		"9.8.7.string",
+		"0.1.2.3",
+		"1.0.0.-10",
+	}
+	for _, test := range invalidVRMFs {
+		t.Run(test, func(t *testing.T) {
+			parsed, err := parseVRMF(test)
+			if err == nil {
+				t.Fatalf("Expected error when parsing VRMF '%s', but got none.  VRMF returned: %v", test, parsed)
+			}
+		})
+	}
+}
+
+func TestCompare(t *testing.T) {
+	tests := []struct {
+		current string
+		compare string
+		expect  int
+	}{
+		{"1.0.0.1", "1.0.0.1", 0},
+		{"1.0.0.1", "1.0.0.0", 1},
+		{"1.0.0.1", "1.0.0.2", -1},
+		{"9.9.9.9", "10.0.0.0", -1},
+		{"9.9.9.9", "9.10.0.0", -1},
+		{"9.9.9.9", "9.9.10.0", -1},
+		{"9.9.9.9", "9.9.9.10", -1},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%s-%s", test.current, test.compare), func(t *testing.T) {
+			baseVRMF, err := parseVRMF(test.current)
+			if err != nil {
+				t.Fatalf("Could not parse base version '%s': %s", test.current, err.Error())
+			}
+			compareVRMF, err := parseVRMF(test.compare)
+			if err != nil {
+				t.Fatalf("Could not parse current version '%s': %s", test.current, err.Error())
+			}
+			result := baseVRMF.compare(*compareVRMF)
+			if result != test.expect {
+				t.Fatalf("Expected %d but got %d when comparing '%s' with '%s'", test.expect, result, test.current, test.compare)
+			}
+			if test.expect == 0 {
+				return
+			}
+			resultReversed := compareVRMF.compare(*baseVRMF)
+			if resultReversed != test.expect*-1 {
+				t.Fatalf("Expected %d but got %d when comparing '%s' with '%s'", test.expect*-1, resultReversed, test.compare, test.current)
+			}
+		})
+	}
+}