Updates based on Copilot code review

Author: absurdfarce

cassandra/util.py

@@ -1692,17 +1692,21 @@ def __repr__(self):
             self.lower_bound, self.upper_bound, self.value
         )
 
-VERSION_REGEX = re.compile("(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?([~\\-]\\w[.\\w]*(?:-\\w[.\\w]*)*)?(\\+[.\\w]+)?")
+VERSION_REGEX = re.compile("^(\\d+)\\.(\\d+)(\\.\\d+)?(\\.\\d+)?([~\\-]\\w[.\\w]*(?:-\\w[.\\w]*)*)?(\\+[.\\w]+)?$")
 
 @total_ordering
 class Version(object):
+    """
+    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
+    """
 
     def __init__(self, version):
         self._version = version
 
         match = VERSION_REGEX.match(version)
         if not match:
-            raise ValueError("Version string did not match expected format")
+            raise ValueError("Version string {0} did not match expected format".format(version))
 
         self.major = int(match[1])
         self.minor = int(match[2])
@@ -1723,15 +1727,15 @@ def __init__(self, version):
             self.prerelease = 0
 
     # Trim off the leading '.' characters and convert the discovered value to an integer
-    def _cleanup_int(self, s):
-        return int(s[1:]) if s else 0
+    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, str):
-        return str[1:] if str else 0
+    def _cleanup_str(self, instr):
+        return instr[1:] if instr else 0
 
     def __hash__(self):
-        return self._version
+        return hash((self.major, self.minor, self.patch, self.build, self.prerelease))
 
     def __repr__(self):
         version_string = "Version({0}, {1}, {2}".format(self.major, self.minor, self.patch)

tests/unit/test_util_types.py

@@ -216,18 +216,9 @@ def test_version_parsing(self):
             ('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')),
-
-            # We also include a few test cases from the former impl of this class, mainly to note differences in behaviours
-
-            # Note that prerelease tags are expected to start with a hyphen or tilde so the expected tag is
-            # lost in all cases below
-            ('3.55.1.build12', (3, 55, 1, 0, 0)),
-            ('1.0.5.4.3', (1, 0, 5, 4, 0)),
-            ('2.1.hello', (2, 1, 0, 0, 0))
         ]
 
         for str_version, expected_result in versions:
-            print(str_version)
             v = Version(str_version)
             self.assertEqual(str_version, str(v))
             self.assertEqual(v.major, expected_result[0])
@@ -245,7 +236,6 @@ def test_version_parsing(self):
         ]
 
         for v in unsupported_versions:
-            print(v)
             with self.assertRaises(ValueError):
                 Version(v)
 
@@ -286,18 +276,29 @@ def test_version_compare(self):
         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.5-SNAPSHOT') == Version('4.0.0.5-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.6-SNAPSHOT') > Version('4.0.0.5-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'))
+
+        equalTuples = [
+            (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 equalTuples:
+            self.assertEqual(a, b)
+            self.assertEqual(hash(a), hash(b))
+
+        leftgreaterTuples = [
+            (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 leftgreaterTuples:
+            self.assertGreater(a, b)
+            self.assertNotEqual(hash(a), hash(b))
 
         # Test the version limit for v4 schema parsing in cassandra.metadata to make sure
         # all 4.0.x Cassandra servers are covered