updated methods in semver class

Author: mmelograno

splitio/models/grammar/matchers/semver.py

@@ -6,6 +6,13 @@
 
 _LOGGER = logging.getLogger(__name__)
 
+def build_semver_or_none(version):
+    try:
+        return Semver(version)
+    except (RuntimeError, ValueError):
+        _LOGGER.error("Invalid semver version: %s", version)
+        return None
+
 class Semver(object):
     """Semver class."""
 
@@ -29,38 +36,29 @@ def __init__(self, version):
         self._metadata = ""
         self._parse(version)
 
-    @classmethod
-    def build(cls, version):
-        try:
-            self = cls(version)
-        except RuntimeError as e:
-            _LOGGER.error("Failed to parse Semver data, incorrect data type:  %s", e)
-            return None
-
-        return self
 
     def _parse(self, version):
         """
         Parse the string in self.version to update the other internal variables
         """
-        without_metadata = self._remove_metadata_if_exists(version)
+        without_metadata = self._extract_metadata(version)
         index = without_metadata.find(self._PRE_RELEASE_DELIMITER)
         if index == -1:
             self._is_stable = True
         else:
             pre_release_data = without_metadata[index+1:]
             if pre_release_data == "":
-                raise RuntimeError("Pre-release is empty despite delimeter exists: " + version)
+                raise RuntimeError("Pre-release is empty despite delimiter exists: " + version)
 
             without_metadata = without_metadata[:index]
             for pre_digit in pre_release_data.split(self._VALUE_DELIMITER):
                 if pre_digit.isnumeric():
                     pre_digit = str(int(pre_digit))
                 self._pre_release.append(pre_digit)
 
-        self._set_major_minor_and_patch(without_metadata)
+        self._set_components(without_metadata)
 
-    def _remove_metadata_if_exists(self, version):
+    def _extract_metadata(self, version):
         """
         Check if there is any metadata characters in self.version.
 
@@ -73,11 +71,11 @@ def _remove_metadata_if_exists(self, version):
 
         self._metadata = version[index+1:]
         if self._metadata == "":
-            raise RuntimeError("Metadata is empty despite delimeter exists: " + version)
+            raise RuntimeError("Metadata is empty despite delimiter exists: " + version)
 
         return  version[:index]
 
-    def _set_major_minor_and_patch(self, version):
+    def _set_components(self, version):
         """
         Set the major, minor and patch internal variables based on string passed.
 
@@ -140,7 +138,7 @@ def compare(self, to_compare):
                 continue
 
             if self._pre_release[i].isnumeric() and to_compare._pre_release[i].isnumeric():
-                 return self._compare_vars(int(self._pre_release[i]), int(to_compare._pre_release[i]))
+                return self._compare_vars(int(self._pre_release[i]), int(to_compare._pre_release[i]))
 
             return self._compare_vars(self._pre_release[i], to_compare._pre_release[i])
 
@@ -179,7 +177,7 @@ def _build(self, raw_matcher):
         :type raw_matcher: dict
         """
         self._data = raw_matcher.get('stringMatcherData')
-        self._semver = Semver.build(self._data)
+        self._semver = build_semver_or_none(raw_matcher.get('stringMatcherData'))
 
     def _match(self, key, attributes=None, context=None):
         """
@@ -195,15 +193,15 @@ def _match(self, key, attributes=None, context=None):
         :returns: Wheter the match is successful.
         :rtype: bool
         """
-        if self._data is None or self._semver is None:
+        if self._semver is None:
             _LOGGER.error("stringMatcherData is required for EQUAL_TO_SEMVER matcher type")
             return False
 
         matching_data = Sanitizer.ensure_string(self._get_matcher_input(key, attributes))
         if matching_data is None:
             return False
 
-        matching_semver = Semver.build(matching_data)
+        matching_semver = build_semver_or_none(matching_data)
         if matching_semver is None:
             return False
 
@@ -228,7 +226,7 @@ def _build(self, raw_matcher):
         :type raw_matcher: dict
         """
         self._data = raw_matcher.get('stringMatcherData')
-        self._semver = Semver.build(self._data)
+        self._semver = build_semver_or_none(raw_matcher.get('stringMatcherData'))
 
     def _match(self, key, attributes=None, context=None):
         """
@@ -244,15 +242,15 @@ def _match(self, key, attributes=None, context=None):
         :returns: Wheter the match is successful.
         :rtype: bool
         """
-        if self._data is None or self._semver is None:
+        if self._semver is None:
             _LOGGER.error("stringMatcherData is required for GREATER_THAN_OR_EQUAL_TO_SEMVER matcher type")
             return False
 
         matching_data = Sanitizer.ensure_string(self._get_matcher_input(key, attributes))
         if matching_data is None:
             return False
 
-        matching_semver = Semver.build(matching_data)
+        matching_semver = build_semver_or_none(matching_data)
         if matching_semver is None:
             return False
 
@@ -277,7 +275,7 @@ def _build(self, raw_matcher):
         :type raw_matcher: dict
         """
         self._data = raw_matcher.get('stringMatcherData')
-        self._semver = Semver.build(self._data)
+        self._semver = build_semver_or_none(raw_matcher.get('stringMatcherData'))
 
     def _match(self, key, attributes=None, context=None):
         """
@@ -293,15 +291,15 @@ def _match(self, key, attributes=None, context=None):
         :returns: Wheter the match is successful.
         :rtype: bool
         """
-        if self._data is None or self._semver is None:
+        if self._semver is None:
             _LOGGER.error("stringMatcherData is required for LESS_THAN_OR_EQUAL_TO_SEMVER matcher type")
             return False
 
         matching_data = Sanitizer.ensure_string(self._get_matcher_input(key, attributes))
         if matching_data is None:
             return False
 
-        matching_semver = Semver.build(matching_data)
+        matching_semver = build_semver_or_none(matching_data)
         if matching_semver is None:
             return False
 
@@ -326,8 +324,8 @@ def _build(self, raw_matcher):
         :type raw_matcher: dict
         """
         self._data = raw_matcher.get('betweenStringMatcherData')
-        self._semver_start = Semver.build(self._data['start']) if self._data.get('start') is not None else None
-        self._semver_end = Semver.build(self._data['end']) if self._data.get('end') is not None else None
+        self._semver_start = build_semver_or_none(self._data['start'])
+        self._semver_end = build_semver_or_none(self._data['end'])
 
     def _match(self, key, attributes=None, context=None):
         """
@@ -343,15 +341,15 @@ def _match(self, key, attributes=None, context=None):
         :returns: Wheter the match is successful.
         :rtype: bool
         """
-        if self._data is None or self._semver_start is None or self._semver_end is None:
+        if self._semver_start is None or self._semver_end is None:
             _LOGGER.error("betweenStringMatcherData is required for BETWEEN_SEMVER matcher type")
             return False
 
         matching_data = Sanitizer.ensure_string(self._get_matcher_input(key, attributes))
         if matching_data is None:
             return False
 
-        matching_semver = Semver.build(matching_data)
+        matching_semver = build_semver_or_none(matching_data)
         if matching_semver is None:
             return False
 
@@ -375,11 +373,9 @@ def _build(self, raw_matcher):
         :param raw_matcher: raw matcher as fetched from splitChanges response.
         :type raw_matcher: dict
         """
-        self._data = raw_matcher.get('whitelistMatcherData')
-        if self._data is not None:
-            self._data = self._data.get('whitelist')
-
-        self._semver_list = [Semver.build(item) if item is not None else None for item in self._data] if self._data is not None else []
+        self._data = raw_matcher['whitelistMatcherData']['whitelist']
+        semver_list = [build_semver_or_none(item) if item is not None else None for item in self._data]
+        self._semver_list = frozenset([item.version for item in semver_list])
 
     def _match(self, key, attributes=None, context=None):
         """
@@ -395,19 +391,19 @@ def _match(self, key, attributes=None, context=None):
         :returns: Wheter the match is successful.
         :rtype: bool
         """
-        if self._data is None:
+        if self._semver_list is None:
             _LOGGER.error("whitelistMatcherData is required for IN_LIST_SEMVER matcher type")
             return False
 
         matching_data = Sanitizer.ensure_string(self._get_matcher_input(key, attributes))
         if matching_data is None:
             return False
 
-        matching_semver = Semver.build(matching_data)
+        matching_semver = build_semver_or_none(matching_data)
         if matching_semver is None:
             return False
 
-        return any([item.version == matching_semver.version if item is not None else False for item in self._semver_list])
+        return matching_semver.version in self._semver_list
 
     def __str__(self):
         """Return string Representation."""

tests/models/grammar/test_matchers.py

@@ -899,7 +899,7 @@ def test_from_raw(self, mocker):
         """Test parsing from raw json/dict."""
         parsed = matchers.from_raw(self.raw)
         assert isinstance(parsed, matchers.EqualToSemverMatcher)
-        assert parsed._data == "2.1.8"
+        assert parsed._semver.version == "2.1.8"
         assert isinstance(parsed._semver, Semver)
         assert parsed._semver._major == 2
         assert parsed._semver._minor == 1
@@ -940,7 +940,7 @@ def test_from_raw(self, mocker):
         """Test parsing from raw json/dict."""
         parsed = matchers.from_raw(self.raw)
         assert isinstance(parsed, matchers.GreaterThanOrEqualToSemverMatcher)
-        assert parsed._data == "2.1.8"
+        assert parsed._semver.version == "2.1.8"
         assert isinstance(parsed._semver, Semver)
         assert parsed._semver._major == 2
         assert parsed._semver._minor == 1
@@ -982,7 +982,7 @@ def test_from_raw(self, mocker):
         """Test parsing from raw json/dict."""
         parsed = matchers.from_raw(self.raw)
         assert isinstance(parsed, matchers.LessThanOrEqualToSemverMatcher)
-        assert parsed._data == "2.1.8"
+        assert parsed._semver.version == "2.1.8"
         assert isinstance(parsed._semver, Semver)
         assert parsed._semver._major == 2
         assert parsed._semver._minor == 1
@@ -1024,14 +1024,15 @@ def test_from_raw(self, mocker):
         """Test parsing from raw json/dict."""
         parsed = matchers.from_raw(self.raw)
         assert isinstance(parsed, matchers.BetweenSemverMatcher)
-        assert parsed._data == {"start": "2.1.8", "end": "2.1.11"}
         assert isinstance(parsed._semver_start, Semver)
         assert isinstance(parsed._semver_end, Semver)
+        assert parsed._semver_start.version == "2.1.8"
         assert parsed._semver_start._major == 2
         assert parsed._semver_start._minor == 1
         assert parsed._semver_start._patch == 8
         assert parsed._semver_start._pre_release == []
 
+        assert parsed._semver_end.version == "2.1.11"
         assert parsed._semver_end._major == 2
         assert parsed._semver_end._minor == 1
         assert parsed._semver_end._patch == 11
@@ -1073,16 +1074,9 @@ def test_from_raw(self, mocker):
         parsed = matchers.from_raw(self.raw)
         assert isinstance(parsed, matchers.InListSemverMatcher)
         assert parsed._data == ["2.1.8", "2.1.11"]
-        assert [isinstance(item, Semver) for item in parsed._semver_list]
-        assert parsed._semver_list[0]._major == 2
-        assert parsed._semver_list[0]._minor == 1
-        assert parsed._semver_list[0]._patch == 8
-        assert parsed._semver_list[0]._pre_release == []
-
-        assert parsed._semver_list[1]._major == 2
-        assert parsed._semver_list[1]._minor == 1
-        assert parsed._semver_list[1]._patch == 11
-        assert parsed._semver_list[1]._pre_release == []
+        assert [isinstance(item, str) for item in parsed._semver_list]
+        assert "2.1.8" in parsed._semver_list
+        assert "2.1.11" in parsed._semver_list
 
     def test_matcher_behaviour(self, mocker):
         """Test if the matcher works properly."""

tests/models/grammar/test_semver.py

@@ -1,9 +1,8 @@
 """Condition model tests module."""
-import pytest
 import csv
 import os
 
-from splitio.models.grammar.matchers.semver import Semver
+from splitio.models.grammar.matchers.semver import build_semver_or_none
 
 valid_versions = os.path.join(os.path.dirname(__file__), 'files', 'valid-semantic-versions.csv')
 invalid_versions = os.path.join(os.path.dirname(__file__), 'files', 'invalid-semantic-versions.csv')
@@ -17,27 +16,27 @@ def test_valid_versions(self):
         with open(valid_versions) as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
-                assert Semver.build(row['higher']) is not None
-                assert Semver.build(row['lower']) is not None
+                assert build_semver_or_none(row['higher']) is not None
+                assert build_semver_or_none(row['lower']) is not None
 
     def test_invalid_versions(self):
         with open(invalid_versions) as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
-                assert Semver.build(row['invalid']) is None
+                assert build_semver_or_none(row['invalid']) is None
 
     def test_compare(self):
         with open(valid_versions) as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
-                assert Semver.build(row['higher']).compare(Semver.build(row['lower'])) == 1
-                assert Semver.build(row['lower']).compare(Semver.build(row['higher'])) == -1
+                assert build_semver_or_none(row['higher']).compare(build_semver_or_none(row['lower'])) == 1
+                assert build_semver_or_none(row['lower']).compare(build_semver_or_none(row['higher'])) == -1
 
         with open(equalto_versions) as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
-                version1 = Semver.build(row['version1'])
-                version2 = Semver.build(row['version2'])
+                version1 = build_semver_or_none(row['version1'])
+                version2 = build_semver_or_none(row['version2'])
                 if row['equals'] == "true":
                     assert version1.version == version2.version
                 else:
@@ -46,14 +45,14 @@ def test_compare(self):
         with open(between_versions) as csvfile:
             reader = csv.DictReader(csvfile)
             for row in reader:
-                version1 = Semver.build(row['version1'])
-                version2 = Semver.build(row['version2'])
-                version3 = Semver.build(row['version3'])
+                version1 = build_semver_or_none(row['version1'])
+                version2 = build_semver_or_none(row['version2'])
+                version3 = build_semver_or_none(row['version3'])
                 if row['expected'] == "true":
                     assert version2.compare(version1) >= 0 and version3.compare(version2) >= 0
                 else:
                     assert version2.compare(version1) < 0 or version3.compare(version2) < 0
 
     def test_leading_zeros(self):
-        assert Semver.build('1.01.2').version == '1.1.2'
-        assert Semver.build('1.01.2-rc.01').version == '1.1.2-rc.1'
+        assert build_semver_or_none('1.01.2').version == '1.1.2'
+        assert build_semver_or_none('1.01.2-rc.01').version == '1.1.2-rc.1'