Change interface to explicitly pass legacy_headers

Based on suggestion from @sdague

TODO: more tests, positive and negative

README.rst

@@ -8,22 +8,21 @@ A simple parser for OpenStack microversion headers::
     # headers is a dict of headers with folded (comma-separated
     # values) or a list of header, value tuples
     version = microversion_parse.get_version(
-        headers, service_type='compute', legacy_type='nova')
+        headers, service_type='compute',
+        legacy_headers=['x-openstack-nova-api-version'])
 
 It processes microversion headers with the standard form::
 
     OpenStack-API-Version: compute 2.1
 
-It also deals with several older formats, depending on the values of
+If provided with a ``legacy_headers`` argument, this is treated as
-the service_type and legacy_type arguments::
+a list of headers to check for microversions. Some examples of
+headers include:
 
-    OpenStack-compute-api-version: 2.1
+    OpenStack-telemetry-api-version: 2.1
     OpenStack-nova-api-version: 2.1
     X-OpenStack-nova-api-version: 2.1
 
-.. note:: The X prefixed version does not currently parse for
+If a version string cannot be found, ``None`` will be returned. If
-          service type named headers, only project named headers.
-
-If a version string cannot be found ``None`` will be returned. If
 the input is incorrect usual Python exceptions (ValueError,
 TypeError) are allowed to raise to the caller.

microversion_parse/__init__.py

@@ -17,58 +17,53 @@ import collections
 STANDARD_HEADER = 'openstack-api-version'
 
 
-def get_version(headers, service_type=None, legacy_type=None):
+def get_version(headers, service_type=None, legacy_headers=None):
     """Parse a microversion out of headers
 
     :param headers: The headers of a request, dict or list
     :param service_type: The service type being looked for in the headers
-    :param legacy_type: The project name to use when looking for fallback
+    :param legacy_headers: Other headers to look at for a version
-                        headers.
     :returns: a version string or "latest"
     :raises: ValueError
+
+    If headers is not a dict we assume is an iterator of
+    tuple-like headers, which we will fold into a dict.
+
+    The flow is that we first look for the new standard singular
+    header:
+
+    * openstack-api-version: <service> <version>
+
+    If that's not present we fall back to the headers listed in
+    legacy_headers. These often look like this:
+
+    * openstack-<service>-api-version: <version>
+    * openstack-<legacy>-api-version: <version>
+    * x-openstack-<legacy>-api-version: <version>
+
+    Folded headers are joined by ','.
     """
-    # If headers is not a dict we assume is an iterator of
+
-    # tuple-like headers, which we will fold into a dict.
+    assert service_type, 'service type required'
-    #
+
-    # The flow is that we first look for the new standard singular
-    # header:
-    # * openstack-api-version: <service> <version>
-    # If that's not present we fall back, in order, to:
-    # * openstack-<service>-api-version: <version>
-    # * openstack-<legacy>-api-version: <version>
-    # * x-openstack-<legacy>-api-version: <version>
-    #
-    # Folded headers are joined by ,
     folded_headers = fold_headers(headers)
 
     version = check_standard_header(folded_headers, service_type)
     if version:
         return version
 
-    extra_headers = build_headers(service_type, legacy_type)
+    if legacy_headers:
-    version = check_legacy_headers(folded_headers, extra_headers)
+        version = check_legacy_headers(folded_headers, legacy_headers)
         return version
 
-
+    return None
-def build_headers(service_type, legacy_type=None):
-    """Create the headers to be looked at."""
-    headers = [
-        'openstack-%s-api-version' % service_type
-    ]
-    if legacy_type:
-        legacy_headers = [
-            'openstack-%s-api-version' % legacy_type,
-            'x-openstack-%s-api-version' % legacy_type
-        ]
-        headers.extend(legacy_headers)
-    return headers
 
 
 def check_legacy_headers(headers, legacy_headers):
     """Gather values from old headers."""
     for legacy_header in legacy_headers:
         try:
-            value = headers[legacy_header]
+            value = headers[legacy_header.lower()]
             return value.split(',')[-1].strip()
         except KeyError:
             pass

microversion_parse/tests/test_get_version.py

@@ -16,23 +16,6 @@ import testtools
 import microversion_parse
 
 
-class TestBuildHeaders(testtools.TestCase):
-
-    def test_build_header_list_service(self):
-        headers = microversion_parse.build_headers('alpha')
-
-        self.assertEqual(1, len(headers))
-        self.assertEqual('openstack-alpha-api-version', headers[0])
-
-    def test_build_header_list_legacy(self):
-        headers = microversion_parse.build_headers('alpha', 'beta')
-
-        self.assertEqual(3, len(headers))
-        self.assertEqual('openstack-alpha-api-version', headers[0])
-        self.assertEqual('openstack-beta-api-version', headers[1])
-        self.assertEqual('x-openstack-beta-api-version', headers[2])
-
-
 class TestFoldHeaders(testtools.TestCase):
 
     def test_dict_headers(self):
@@ -142,7 +125,8 @@ class TestLegacyHeaders(testtools.TestCase):
             'header-two': 'beta',
         }
         version = microversion_parse.get_version(
-            headers, service_type='compute')
+            headers, service_type='compute',
+            legacy_headers=['openstack-CoMpUte-api-version'])
         self.assertEqual('2.1', version)
 
     def test_legacy_headers_folded(self):
@@ -152,28 +136,21 @@ class TestLegacyHeaders(testtools.TestCase):
             'header-two': 'beta',
         }
         version = microversion_parse.get_version(
-            headers, service_type='compute')
+            headers, service_type='compute',
+            legacy_headers=['openstack-compute-api-version'])
         self.assertEqual('9.2', version)
 
-    def test_older_legacy_headers_with_service(self):
+    def test_older_legacy_headers(self):
-        headers = {
-            'header-one': 'alpha',
-            'x-openstack-compute-api-version': ' 2.1, 9.2 ',
-            'header-two': 'beta',
-        }
-        version = microversion_parse.get_version(
-            headers, service_type='compute')
-        # We don't do x- for service types.
-        self.assertEqual(None, version)
-
-    def test_legacy_headers_project(self):
         headers = {
             'header-one': 'alpha',
             'x-openstack-nova-api-version': ' 2.1, 9.2 ',
             'header-two': 'beta',
         }
         version = microversion_parse.get_version(
-            headers, service_type='compute', legacy_type='nova')
+            headers, service_type='compute',
+            legacy_headers=['openstack-nova-api-version',
+                            'x-openstack-nova-api-version'])
+        # We don't do x- for service types.
         self.assertEqual('9.2', version)
 
     def test_legacy_headers_prefer(self):
@@ -184,8 +161,15 @@ class TestLegacyHeaders(testtools.TestCase):
             'header-two': 'beta',
         }
         version = microversion_parse.get_version(
-            headers, service_type='compute', legacy_type='nova')
+            headers, service_type='compute',
+            legacy_headers=['openstack-compute-api-version',
+                            'x-openstack-nova-api-version'])
         self.assertEqual('3.7', version)
+        version = microversion_parse.get_version(
+            headers, service_type='compute',
+            legacy_headers=['x-openstack-nova-api-version',
+                            'openstack-compute-api-version'])
+        self.assertEqual('9.2', version)
 
 
 class TestGetHeaders(testtools.TestCase):
@@ -199,9 +183,21 @@ class TestGetHeaders(testtools.TestCase):
             'header-two': 'beta',
         }
         version = microversion_parse.get_version(
-            headers, service_type='compute', legacy_type='nova')
+            headers, service_type='compute',
+            legacy_headers=['openstack-compute-api-version',
+                            'x-openstack-nova-api-version'])
         self.assertEqual('11.12', version)
 
+    def test_no_headers(self):
+        headers = {}
+        version = microversion_parse.get_version(
+            headers, service_type='compute')
+        self.assertEqual(None, version)
+
+        self.assertRaises(AssertionError,
+                          microversion_parse.get_version,
+                          headers)
+
     def test_unfolded_service(self):
         headers = [
             ('header-one', 'alpha'),
@@ -210,16 +206,17 @@ class TestGetHeaders(testtools.TestCase):
             ('openstack-api-version', '3.0'),
         ]
         version = microversion_parse.get_version(
-            headers, service_type='compute', legacy_type='nova')
+            headers, service_type='compute')
         self.assertEqual('2.0', version)
 
     def test_unfolded_in_name(self):
         headers = [
             ('header-one', 'alpha'),
-            ('openstack-compute-api-version', '1.0'),
+            ('x-openstack-nova-api-version', '1.0'),
-            ('openstack-compute-api-version', '2.0'),
+            ('x-openstack-nova-api-version', '2.0'),
             ('openstack-telemetry-api-version', '3.0'),
         ]
         version = microversion_parse.get_version(
-            headers, service_type='compute', legacy_type='nova')
+            headers, service_type='compute',
+            legacy_headers=['x-openstack-nova-api-version'])
         self.assertEqual('2.0', version)