From e346558d5f67ae2a3a848f6d8ad6166c761b39d1 Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Tue, 6 Mar 2018 18:47:31 +0000 Subject: [PATCH] Improve Version.matches() method Use of Version within the placement service has made it clear that the Version object knowing the min and max versions is useful when making Version.matches comparisons. Therefore the matches method has been updated to refer to min_version and max_version attributes if they are not set in the matches() request. The extract_version function now sets those attributes based on the version_list that is passed to it. If min and max are not set on the Version object, they default to negative versions, which are not possible in microversions and mean matches() (without override args) will be false. The use of explicit values (instead of None) is so python3 is happy (and it is better anyway). Change-Id: I71d091d037abcd068601b32722a09094ae74e658 --- microversion_parse/__init__.py | 24 ++++++++++-- .../tests/test_extract_version.py | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/microversion_parse/__init__.py b/microversion_parse/__init__.py index eae6398..f033b73 100644 --- a/microversion_parse/__init__.py +++ b/microversion_parse/__init__.py @@ -26,11 +26,26 @@ class Version(collections.namedtuple('Version', 'major minor')): Since it is a tuple, it is automatically comparable. """ + def __new__(cls, major, minor): + """Add mix and max version attributes to the tuple.""" + self = super(Version, cls).__new__(cls, major, minor) + self.max_version = (-1, 0) + self.min_version = (-1, 0) + return self + def __str__(self): return '%s.%s' % (self.major, self.minor) - def matches(self, min_version, max_version): - """Is this version within min_version and max_version.""" + def matches(self, min_version=None, max_version=None): + """Is this version within min_version and max_version. + """ + # NOTE(cdent): min_version and max_version are expected + # to be set by the code that is creating the Version, if + # they are known. + if min_version is None: + min_version = self.min_version + if max_version is None: + max_version = self.max_version return min_version <= self <= max_version @@ -178,7 +193,8 @@ def extract_version(headers, service_type, versions_list): :param service_type: The service_type as a string :param versions_list: List of all possible microversions as strings, sorted from earliest to latest version. - :returns: a Version + :returns: a Version with the optional min_version and max_version + attributes set. :raises: ValueError """ found_version = get_version(headers, service_type=service_type) @@ -191,6 +207,8 @@ def extract_version(headers, service_type, versions_list): if version_string == 'latest': version_string = max_version_string request_version = parse_version_string(version_string) + request_version.max_version = parse_version_string(max_version_string) + request_version.min_version = parse_version_string(min_version_string) # We need a version that is in versions_list. This gives us the option # to administratively disable a version if we really need to. if str(request_version) in versions_list: diff --git a/microversion_parse/tests/test_extract_version.py b/microversion_parse/tests/test_extract_version.py index af63ffc..c8fd1c8 100644 --- a/microversion_parse/tests/test_extract_version.py +++ b/microversion_parse/tests/test_extract_version.py @@ -42,6 +42,26 @@ class TestVersion(testtools.TestCase): self.assertTrue(self.version.matches(min_version, max_version)) + def test_version_matches_no_extremes(self): + """If no extremes are present, never match.""" + self.assertFalse(self.version.matches()) + + def test_version_zero_can_match(self): + """If a version is '0.0' we want to it be able to match.""" + version = microversion_parse.Version(0, 0) + min_version = microversion_parse.Version(0, 0) + max_version = microversion_parse.Version(0, 0) + version.min_version = min_version + version.max_version = max_version + + self.assertTrue(version.matches()) + + def test_version_zero_no_defaults(self): + """If a version is '0.0' we want to it be able to match.""" + version = microversion_parse.Version(0, 0) + + self.assertFalse(version.matches()) + def test_version_init_failure(self): self.assertRaises(TypeError, microversion_parse.Version, 1, 2, 3) @@ -107,6 +127,24 @@ class TestExtractVersion(testtools.TestCase): self.headers, 'service3', self.version_list) self.assertEqual((2, 4), version) + def test_min_max_extract(self): + version = microversion_parse.extract_version( + self.headers, 'service1', self.version_list) + + # below min + self.assertFalse(version.matches((1, 3))) + # at min + self.assertTrue(version.matches((1, 2))) + # within extremes + self.assertTrue(version.matches()) + # explicit max + self.assertTrue(version.matches(max_version=(2, 3))) + # explicit min + self.assertFalse(version.matches(min_version=(2, 3))) + # explicit both + self.assertTrue(version.matches(min_version=(0, 3), + max_version=(1, 5))) + def test_version_disabled(self): self.assertRaises(ValueError, microversion_parse.extract_version, self.headers, 'service2', self.version_list)