Refactor placement version check

raise_http_status_code_if_not_version are duplicate
we don't need keep this function, instead, use
the decorater to check version.

Also, fixed a typo in comment as well.

Change-Id: Ie7200dbfb863d27115e12347e0e8b4ad444489e1

nova/api/openstack/placement/handlers/aggregate.py

@@ -44,6 +44,7 @@ def _serialize_aggregates(aggregate_uuids):
 
 @wsgi_wrapper.PlacementWsgify
 @util.check_accept('application/json')
+@microversion.version_handler('1.1')
 def get_aggregates(req):
     """GET a list of aggregates associated with a resource provider.
 
@@ -52,7 +53,6 @@ def get_aggregates(req):
     On success return a 200 with an application/json body containing a
     list of aggregate uuids.
     """
-    microversion.raise_http_status_code_if_not_version(req, 404, (1, 1))
     context = req.environ['placement.context']
     uuid = util.wsgi_path_item(req.environ, 'uuid')
     resource_provider = rp_obj.ResourceProvider.get_by_uuid(
@@ -64,8 +64,8 @@ def get_aggregates(req):
 
 @wsgi_wrapper.PlacementWsgify
 @util.require_content('application/json')
+@microversion.version_handler('1.1')
 def set_aggregates(req):
-    microversion.raise_http_status_code_if_not_version(req, 404, (1, 1))
     context = req.environ['placement.context']
     uuid = util.wsgi_path_item(req.environ, 'uuid')
     resource_provider = rp_obj.ResourceProvider.get_by_uuid(

nova/api/openstack/placement/handlers/inventory.py

@@ -387,6 +387,7 @@ def set_inventories(req):
 
 
 @wsgi_wrapper.PlacementWsgify
+@microversion.version_handler('1.5', status_code=405)
 def delete_inventories(req):
     """DELETE all inventory for a resource provider.
 
@@ -395,7 +396,6 @@ def delete_inventories(req):
     On success return a 204 No content.
     Return 405 Method Not Allowed if the wanted microversion does not match.
     """
-    microversion.raise_http_status_code_if_not_version(req, 405, (1, 5))
     context = req.environ['placement.context']
     uuid = util.wsgi_path_item(req.environ, 'uuid')
     resource_provider = rp_obj.ResourceProvider.get_by_uuid(

nova/api/openstack/placement/microversion.py

@@ -87,28 +87,6 @@ def parse_version_string(version_string):
             version_string, exc))
 
 
-def raise_http_status_code_if_not_version(req, status_code, min_version,
-                                          max_version=None):
-    """Utility to raise a http status code if the wanted microversion does not
-       match.
-
-    :param req: The HTTP request for the placement api
-    :param status_code: HTTP status code (integer value) to be raised
-    :param min_version: Minimum placement microversion level
-    :param max_version: Maximum placement microversion level
-    :returns: None
-    :raises: HTTP status code if the specified microversion does not match
-    :raises: KeyError if status_code is not a valid HTTP status code
-    """
-    if not isinstance(min_version, tuple):
-        min_version = parse_version_string(min_version)
-    if max_version and not isinstance(max_version, tuple):
-        max_version = parse_version_string(max_version)
-    want_version = req.environ[MICROVERSION_ENVIRON]
-    if not want_version.matches(min_version, max_version):
-        raise webob.exc.status_map[status_code]
-
-
 class MicroversionMiddleware(object):
     """WSGI middleware for getting microversion info."""
 
@@ -228,10 +206,11 @@ def _fully_qualified_name(obj):
     return name
 
 
-def _find_method(f, version):
+def _find_method(f, version, status_code):
     """Look in VERSIONED_METHODS for method with right name matching version.
 
-    If no match is found raise a 404.
+    If no match is found a HTTPError corresponding to status_code will
+    be returned.
     """
     qualified_name = _fully_qualified_name(f)
     # A KeyError shouldn't be possible here, but let's be robust
@@ -241,10 +220,10 @@ def _find_method(f, version):
         if min_version <= version <= max_version:
             return func
 
-    raise webob.exc.HTTPNotFound()
+    raise webob.exc.status_map[status_code]
 
 
-def version_handler(min_ver, max_ver=None):
+def version_handler(min_ver, max_ver=None, status_code=404):
     """Decorator for versioning API methods.
 
     Add as a decorator to a placement API handler to constrain
@@ -256,8 +235,9 @@ def version_handler(min_ver, max_ver=None):
 
     :param min_ver: A string of two numerals, X.Y indicating the
                     minimum version allowed for the decorated method.
-    :param min_ver: A string of two numerals, X.Y, indicating the
+    :param max_ver: A string of two numerals, X.Y, indicating the
                     maximum version allowed for the decorated method.
+    :param status_code: A status code to indicate error, 404 by default
     """
     def decorator(f):
         min_version = parse_version_string(min_ver)
@@ -271,7 +251,7 @@ def version_handler(min_ver, max_ver=None):
 
         def decorated_func(req, *args, **kwargs):
             version = req.environ[MICROVERSION_ENVIRON]
-            return _find_method(f, version)(req, *args, **kwargs)
+            return _find_method(f, version, status_code)(req, *args, **kwargs)
 
         # Sort highest min version to beginning of list.
         VERSIONED_METHODS[qualified_name].sort(key=lambda x: x[0],

nova/tests/unit/api/openstack/placement/test_microversion.py

@@ -14,9 +14,9 @@
 
 import collections
 import operator
+import webob
 
 import mock
-import webob
 
 # import the handlers to load up handler decorators
 import nova.api.openstack.placement.handler  # noqa
@@ -28,6 +28,16 @@ def handler():
     return True
 
 
+class TestMicroversionFindMethod(test.NoDBTestCase):
+    def test_method_405(self):
+        self.assertRaises(webob.exc.HTTPMethodNotAllowed,
+                          microversion._find_method, handler, '1.1', 405)
+
+    def test_method_404(self):
+        self.assertRaises(webob.exc.HTTPNotFound,
+                          microversion._find_method, handler, '1.1', 404)
+
+
 class TestMicroversionDecoration(test.NoDBTestCase):
 
     @mock.patch('nova.api.openstack.placement.microversion.VERSIONED_METHODS',
@@ -74,7 +84,7 @@ class TestMicroversionIntersection(test.NoDBTestCase):
     # if you add two different versions of method 'foobar' the
     # number only goes up by one if no other version foobar yet
     # exists. This operates as a simple sanity check.
-    TOTAL_VERSIONED_METHODS = 16
+    TOTAL_VERSIONED_METHODS = 19
 
     def test_methods_versioned(self):
         methods_data = microversion.VERSIONED_METHODS
@@ -124,40 +134,6 @@ class TestMicroversionIntersection(test.NoDBTestCase):
                 'method %s has intersecting versioned handlers' % method_name)
 
 
-class TestMicroversionUtility(test.NoDBTestCase):
-
-    req = webob.Request.blank('/', method="GET")
-    req.accept = 'application/json'
-
-    def test_raise_405_out_of_date_version(self):
-        version_obj = microversion.parse_version_string('1.4')
-        self.req.environ['placement.microversion'] = version_obj
-        self.assertRaises(webob.exc.HTTPMethodNotAllowed,
-             microversion.raise_http_status_code_if_not_version,
-             self.req, 405, (1, 5))
-
-    def test_raise_405_out_of_date_version_max(self):
-        version_obj = microversion.parse_version_string('1.4')
-        self.req.environ['placement.microversion'] = version_obj
-        self.assertRaises(webob.exc.HTTPMethodNotAllowed,
-             microversion.raise_http_status_code_if_not_version,
-             self.req, 405, (1, 2), '1.3')
-
-    def test_raise_keyerror_out_of_date_version_tuple(self):
-        version_obj = microversion.parse_version_string('1.4')
-        self.req.environ['placement.microversion'] = version_obj
-        self.assertRaises(KeyError,
-             microversion.raise_http_status_code_if_not_version,
-             self.req, 999, (1, 5))
-
-    def test_raise_keyerror_out_of_date_version_string(self):
-        version_obj = microversion.parse_version_string('1.4')
-        self.req.environ['placement.microversion'] = version_obj
-        self.assertRaises(KeyError,
-             microversion.raise_http_status_code_if_not_version,
-             self.req, 999, '1.5')
-
-
 class MicroversionSequentialTest(test.NoDBTestCase):
 
     def test_microversion_sequential(self):