CASSPYTHON-10 Update cassandra.util.Version to better support Cassandra version strings

patch by Bret McGuire; reviewed by Brad Schoening and Bret McGuire

Author: absurdfarce

cassandra/metadata.py

@@ -3290,7 +3290,7 @@ def get_schema_parser(connection, server_version, dse_version, timeout):
         elif v >= Version('6.0.0'):
             return SchemaParserDSE60(connection, timeout)
 
-    if version >= Version('4-a'):
+    if version >= Version('4.0-alpha'):
         return SchemaParserV4(connection, timeout)
     elif version >= Version('3.0.0'):
         return SchemaParserV3(connection, timeout)

cassandra/util.py

@@ -1692,57 +1692,56 @@ def __repr__(self):
             self.lower_bound, self.upper_bound, self.value
         )
 
+VERSION_REGEX = re.compile("^(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?([~\\-]\\w[.\\w]*(?:-\\w[.\\w]*)*)?(\\+[.\\w]+)?$")
 
 @total_ordering
 class Version(object):
     """
-    Internal minimalist class to compare versions.
-    A valid version is: <int>.<int>.<int>.<int or str>.
+    Representation of a Cassandra version.  Mostly follows the implementation of the same logic in the Java driver;
+    see https://github.com/apache/cassandra-java-driver/blob/4.19.2/core/src/main/java/com/datastax/oss/driver/api/core/Version.java.
 
-    TODO: when python2 support is removed, use packaging.version.
+    Cassandra versions are assumed to correspond to major.minor.patch with an optional additional numeric build field as well as a
+    string prerelease field.
     """
 
-    _version = None
-    major = None
-    minor = 0
-    patch = 0
-    build = 0
-    prerelease = 0
-
     def __init__(self, version):
         self._version = version
-        if '-' in version:
-            version_without_prerelease, self.prerelease = version.split('-', 1)
-        else:
-            version_without_prerelease = version
-        parts = list(reversed(version_without_prerelease.split('.')))
-        if len(parts) > 4:
-            prerelease_string = "-{}".format(self.prerelease) if self.prerelease else ""
-            log.warning("Unrecognized version: {}. Only 4 components plus prerelease are supported. "
-                        "Assuming version as {}{}".format(version, '.'.join(parts[:-5:-1]), prerelease_string))
+
+        match = VERSION_REGEX.match(version)
+        if not match:
+            raise ValueError("Version string {0} did not match expected format".format(version))
+
+        self.major = int(match[1])
+        self.minor = int(match[2])
 
         try:
-            self.major = int(parts.pop())
-        except ValueError as e:
-            raise ValueError(
-                "Couldn't parse version {}. Version should start with a number".format(version))\
-                .with_traceback(e.__traceback__)
+            self.patch = self._cleanup_int(match[3])
+        except:
+            self.patch = 0
+
         try:
-            self.minor = int(parts.pop()) if parts else 0
-            self.patch = int(parts.pop()) if parts else 0
+            self.build = self._cleanup_int(match[4])
+        except:
+            self.build = 0
 
-            if parts:  # we have a build version
-                build = parts.pop()
-                try:
-                    self.build = int(build)
-                except ValueError:
-                    self.build = build
-        except ValueError:
-            assumed_version = "{}.{}.{}.{}-{}".format(self.major, self.minor, self.patch, self.build, self.prerelease)
-            log.warning("Unrecognized version {}. Assuming version as {}".format(version, assumed_version))
+        try:
+            self.prerelease = self._cleanup_str(match[5])
+        except:
+            self.prerelease = ""
+
+        # This is used in a few places below so let's just build it now
+        self._tuple = (self.major, self.minor, self.patch, self.build, self.prerelease)
+
+    # Trim off the leading '.' characters and convert the discovered value to an integer
+    def _cleanup_int(self, instr):
+        return int(instr[1:]) if instr else 0
+
+    # Trim off the leading '.' or '~' characters and just return the string directly
+    def _cleanup_str(self, instr):
+        return instr[1:] if instr else ""
 
     def __hash__(self):
-        return self._version
+        return hash(self._tuple)
 
     def __repr__(self):
         version_string = "Version({0}, {1}, {2}".format(self.major, self.minor, self.patch)
@@ -1757,48 +1756,27 @@ def __repr__(self):
     def __str__(self):
         return self._version
 
-    @staticmethod
-    def _compare_version_part(version, other_version, cmp):
-        if not (isinstance(version, int) and
-                isinstance(other_version, int)):
-            version = str(version)
-            other_version = str(other_version)
-
-        return cmp(version, other_version)
-
+    # Methods below leverage left-to-right positional comparison of tuples
     def __eq__(self, other):
         if not isinstance(other, Version):
             return NotImplemented
 
-        return (self.major == other.major and
-                self.minor == other.minor and
-                self.patch == other.patch and
-                self._compare_version_part(self.build, other.build, lambda s, o: s == o) and
-                self._compare_version_part(self.prerelease, other.prerelease, lambda s, o: s == o)
-                )
+        return self._tuple == other._tuple
 
     def __gt__(self, other):
         if not isinstance(other, Version):
             return NotImplemented
 
-        is_major_ge = self.major >= other.major
-        is_minor_ge = self.minor >= other.minor
-        is_patch_ge = self.patch >= other.patch
-        is_build_gt = self._compare_version_part(self.build, other.build, lambda s, o: s > o)
-        is_build_ge = self._compare_version_part(self.build, other.build, lambda s, o: s >= o)
-
-        # By definition, a prerelease comes BEFORE the actual release, so if a version
-        # doesn't have a prerelease, it's automatically greater than anything that does
-        if self.prerelease and not other.prerelease:
-            is_prerelease_gt = False
+        # We start by comparing the first four fields directly
+        self_tuple = self._tuple[:4]
+        other_tuple = (other.major, other.minor, other.patch, other.build)
+        if self_tuple != other_tuple:
+            return self_tuple > other_tuple
+        # If we're still around we have to check prereleases... prereleases always come before
+        # the corresponding version
+        elif self.prerelease and not other.prerelease:
+            return False
         elif other.prerelease and not self.prerelease:
-            is_prerelease_gt = True
+            return True
         else:
-            is_prerelease_gt = self._compare_version_part(self.prerelease, other.prerelease, lambda s, o: s > o) \
-
-        return (self.major > other.major or
-                (is_major_ge and self.minor > other.minor) or
-                (is_major_ge and is_minor_ge and self.patch > other.patch) or
-                (is_major_ge and is_minor_ge and is_patch_ge and is_build_gt) or
-                (is_major_ge and is_minor_ge and is_patch_ge and is_build_ge and is_prerelease_gt)
-                )
+            return self.prerelease > other.prerelease

tests/unit/test_util_types.py

@@ -209,18 +209,13 @@ class VersionTests(unittest.TestCase):
 
     def test_version_parsing(self):
         versions = [
-            ('2.0.0', (2, 0, 0, 0, 0)),
-            ('3.1.0', (3, 1, 0, 0, 0)),
-            ('2.4.54', (2, 4, 54, 0, 0)),
-            ('3.1.1.12', (3, 1, 1, 12, 0)),
-            ('3.55.1.build12', (3, 55, 1, 'build12', 0)),
-            ('3.55.1.20190429-TEST', (3, 55, 1, 20190429, 'TEST')),
-            ('4.0-SNAPSHOT', (4, 0, 0, 0, 'SNAPSHOT')),
-            ('1.0.5.4.3', (1, 0, 5, 4, 0)),
-            ('1-SNAPSHOT', (1, 0, 0, 0, 'SNAPSHOT')),
-            ('4.0.1.2.3.4.5-ABC-123-SNAP-TEST.blah', (4, 0, 1, 2, 'ABC-123-SNAP-TEST.blah')),
-            ('2.1.hello', (2, 1, 0, 0, 0)),
-            ('2.test.1', (2, 0, 0, 0, 0)),
+            # Test cases here adapted from the Java driver cases 
+            # (https://github.com/apache/cassandra-java-driver/blob/4.19.2/core/src/test/java/com/datastax/oss/driver/api/core/VersionTest.java)
+            ('1.2.19', (1, 2, 19, 0, "")),
+            ('1.2', (1, 2, 0, 0, "")),
+            ('1.2-beta1-SNAPSHOT', (1, 2, 0, 0, 'beta1-SNAPSHOT')),
+            ('1.2~beta1-SNAPSHOT', (1, 2, 0, 0, 'beta1-SNAPSHOT')),
+            ('1.2.19.2-SNAPSHOT', (1, 2, 19, 2, 'SNAPSHOT')),
         ]
 
         for str_version, expected_result in versions:
@@ -232,9 +227,17 @@ def test_version_parsing(self):
             self.assertEqual(v.build, expected_result[3])
             self.assertEqual(v.prerelease, expected_result[4])
 
-        # not supported version formats
-        with self.assertRaises(ValueError):
-            Version('test.1.0')
+        # Note that a few of these formats used to be supported when this class was based on the Python versioning scheme.
+        # This has been updated to more directly correspond to the Cassandra versioning scheme.  See CASSPYTHON-10 for more
+        # detail.
+        unsupported_versions = [
+            "test.1.0",
+            '2.test.1'
+        ]
+
+        for v in unsupported_versions:
+            with self.assertRaises(ValueError):
+                Version(v)
 
     def test_version_compare(self):
         # just tests a bunch of versions
@@ -251,41 +254,53 @@ def test_version_compare(self):
 
         # patch wins
         self.assertTrue(Version('2.3.1') > Version('2.3.0'))
-        self.assertTrue(Version('2.3.1') > Version('2.3.0.4post0'))
+        self.assertTrue(Version('2.3.1') > Version('2.3.0-4post0'))
         self.assertTrue(Version('2.3.1') > Version('2.3.0.44'))
 
         # various
         self.assertTrue(Version('2.3.0.1') > Version('2.3.0.0'))
         self.assertTrue(Version('2.3.0.680') > Version('2.3.0.670'))
         self.assertTrue(Version('2.3.0.681') > Version('2.3.0.680'))
-        self.assertTrue(Version('2.3.0.1build0') > Version('2.3.0.1'))  # 4th part fallback to str cmp
-        self.assertTrue(Version('2.3.0.build0') > Version('2.3.0.1'))  # 4th part fallback to str cmp
-        self.assertTrue(Version('2.3.0') < Version('2.3.0.build'))
-
-        self.assertTrue(Version('4-a') <= Version('4.0.0'))
-        self.assertTrue(Version('4-a') <= Version('4.0-alpha1'))
-        self.assertTrue(Version('4-a') <= Version('4.0-beta1'))
-        self.assertTrue(Version('4.0.0') >= Version('4.0.0'))
-        self.assertTrue(Version('4.0.0.421') >= Version('4.0.0'))
-        self.assertTrue(Version('4.0.1') >= Version('4.0.0'))
+
+        # If builds are equal then a prerelease always comes before
+        self.assertTrue(Version('2.3.0.1-SNAPSHOT') < Version('2.3.0.1'))
+
+        # If both have prereleases we fall back to a string compare
+        self.assertTrue(Version('2.3.0.1-SNAPSHOT') < Version('2.3.0.1-ZNAPSHOT'))
+
         self.assertTrue(Version('2.3.0') == Version('2.3.0'))
         self.assertTrue(Version('2.3.32') == Version('2.3.32'))
         self.assertTrue(Version('2.3.32') == Version('2.3.32.0'))
-        self.assertTrue(Version('2.3.0.build') == Version('2.3.0.build'))
+        self.assertTrue(Version('2.3.0-SNAPSHOT') == Version('2.3.0-SNAPSHOT'))
 
-        self.assertTrue(Version('4') == Version('4.0.0'))
         self.assertTrue(Version('4.0') == Version('4.0.0.0'))
         self.assertTrue(Version('4.0') > Version('3.9.3'))
 
-        self.assertTrue(Version('4.0') > Version('4.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0-SNAPSHOT') == Version('4.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0.0-SNAPSHOT') == Version('4.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0.0-SNAPSHOT') == Version('4.0.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0.0.build5-SNAPSHOT') == Version('4.0.0.build5-SNAPSHOT'))
-        self.assertTrue(Version('4.1-SNAPSHOT') > Version('4.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0.1-SNAPSHOT') > Version('4.0.0-SNAPSHOT'))
-        self.assertTrue(Version('4.0.0.build6-SNAPSHOT') > Version('4.0.0.build5-SNAPSHOT'))
-        self.assertTrue(Version('4.0-SNAPSHOT2') > Version('4.0-SNAPSHOT1'))
-        self.assertTrue(Version('4.0-SNAPSHOT2') > Version('4.0.0-SNAPSHOT1'))
-
-        self.assertTrue(Version('4.0.0-alpha1-SNAPSHOT') > Version('4.0.0-SNAPSHOT'))
+
+        equal_tuples = [
+            (Version('4.0-SNAPSHOT'), Version('4.0-SNAPSHOT')),
+            (Version('4.0.0-SNAPSHOT'), Version('4.0-SNAPSHOT')),
+            (Version('4.0.0-SNAPSHOT'), Version('4.0.0-SNAPSHOT')),
+            (Version('4.0.0.5-SNAPSHOT'), Version('4.0.0.5-SNAPSHOT'))
+        ]
+        for (a,b) in equal_tuples:
+            self.assertEqual(a, b)
+            self.assertEqual(hash(a), hash(b))
+
+        left_greater_tuples = [
+            (Version('4.0'), Version('4.0-SNAPSHOT')),
+            (Version('4.1-SNAPSHOT'), Version('4.0-SNAPSHOT')),
+            (Version('4.0.1-SNAPSHOT'), Version('4.0.0-SNAPSHOT')),
+            (Version('4.0.0.6-SNAPSHOT'), Version('4.0.0.5-SNAPSHOT')),
+            (Version('4.0-SNAPSHOT2'), Version('4.0-SNAPSHOT1')),
+            (Version('4.0-SNAPSHOT2'), Version('4.0.0-SNAPSHOT1')),
+            (Version('4.0.0-alpha1-SNAPSHOT'), Version('4.0.0-SNAPSHOT'))
+        ]
+        for (a,b) in left_greater_tuples:
+            self.assertGreater(a, b)
+
+        # Test the version limit for v4 schema parsing in cassandra.metadata to make sure
+        # all 4.0.x Cassandra servers are covered
+        self.assertTrue(Version('4.0-alpha') <= Version('4.0.0'))
+        self.assertTrue(Version('4.0-alpha') <= Version('4.0-alpha1'))
+        self.assertTrue(Version('4.0-alpha') <= Version('4.0-beta1'))