Merge "api: Allow min/max_version arguments to expected_errors"

nova/api/openstack/wsgi.py

@@ -15,6 +15,7 @@
 #    under the License.
 
 import functools
+import typing as ty
 
 import microversion_parse
 from oslo_log import log as logging
@@ -664,7 +665,11 @@ def removed(version: str, reason: str):
     return decorator
 
 
-def expected_errors(errors):
+def expected_errors(
+    errors: ty.Union[int, tuple[int, ...]],
+    min_version: ty.Optional[str] = None,
+    max_version: ty.Optional[str] = None,
+):
     """Decorator for v2.1 API methods which specifies expected exceptions.
 
     Specify which exceptions may occur when an API method is called. If an
@@ -674,9 +679,27 @@ def expected_errors(errors):
     def decorator(f):
         @functools.wraps(f)
         def wrapped(*args, **kwargs):
+            min_ver = api_version.APIVersionRequest(min_version)
+            max_ver = api_version.APIVersionRequest(max_version)
+
+            # The request object is always the second argument.
+            # However numerous unittests pass in the request object
+            # via kwargs instead so we handle that as well.
+            # TODO(cyeoh): cleanup unittests so we don't have to
+            # to do this
+            if 'req' in kwargs:
+                ver = kwargs['req'].api_version_request
+            else:
+                ver = args[1].api_version_request
+
             try:
                 return f(*args, **kwargs)
             except Exception as exc:
+                # if this instance of the decorator is intended for other
+                # versions, let the exception bubble up as-is
+                if not ver.matches(min_ver, max_ver):
+                    raise
+
                 if isinstance(exc, webob.exc.WSGIHTTPException):
                     if isinstance(errors, int):
                         t_errors = (errors,)

nova/tests/unit/api/openstack/test_wsgi.py

@@ -930,36 +930,81 @@ class TestController(test.NoDBTestCase):
 class ExpectedErrorTestCase(test.NoDBTestCase):
 
     def test_expected_error(self):
-        @wsgi.expected_errors(404)
-        def fake_func():
-            raise webob.exc.HTTPNotFound()
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors(404)
+            def fake_func(self, req):
+                raise webob.exc.HTTPNotFound()
 
-        self.assertRaises(webob.exc.HTTPNotFound, fake_func)
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('')
+        self.assertRaises(webob.exc.HTTPNotFound, controller.fake_func, req)
 
     def test_expected_error_from_list(self):
-        @wsgi.expected_errors((404, 403))
-        def fake_func():
-            raise webob.exc.HTTPNotFound()
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors((404, 403))
+            def fake_func(self, req):
+                raise webob.exc.HTTPNotFound()
 
-        self.assertRaises(webob.exc.HTTPNotFound, fake_func)
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('')
+        self.assertRaises(webob.exc.HTTPNotFound, controller.fake_func, req)
+
+    def test_expected_error_with_microversion(self):
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors(404, '2.1', '2.5')
+            @wsgi.expected_errors((400, 404), '2.6')
+            def fake_func(self, req):
+                raise webob.exc.HTTPBadRequest()
+
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('', version='2.7')
+        self.assertRaises(webob.exc.HTTPBadRequest, controller.fake_func, req)
 
     def test_unexpected_error(self):
-        @wsgi.expected_errors(404)
-        def fake_func():
-            raise webob.exc.HTTPConflict()
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors(404)
+            def fake_func(self, req):
+                raise webob.exc.HTTPConflict()
 
-        self.assertRaises(webob.exc.HTTPInternalServerError, fake_func)
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('')
+        self.assertRaises(
+            webob.exc.HTTPInternalServerError, controller.fake_func, req
+        )
 
     def test_unexpected_error_from_list(self):
-        @wsgi.expected_errors((404, 413))
-        def fake_func():
-            raise webob.exc.HTTPConflict()
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors((404, 413))
+            def fake_func(self, req):
+                raise webob.exc.HTTPConflict()
 
-        self.assertRaises(webob.exc.HTTPInternalServerError, fake_func)
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('')
+        self.assertRaises(
+            webob.exc.HTTPInternalServerError, controller.fake_func, req
+        )
+
+    def test_unexpected_error_with_microversion(self):
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors(404, '2.1', '2.5')
+            @wsgi.expected_errors((400, 404), '2.6')
+            def fake_func(self, req):
+                raise webob.exc.HTTPBadRequest()
+
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('', version='2.5')
+        self.assertRaises(
+            webob.exc.HTTPInternalServerError, controller.fake_func, req
+        )
 
     def test_unexpected_policy_not_authorized_error(self):
-        @wsgi.expected_errors(404)
-        def fake_func():
-            raise exception.PolicyNotAuthorized(action="foo")
+        class FakeController(wsgi.Controller):
+            @wsgi.expected_errors(404)
+            def fake_func(self, req):
+                raise exception.PolicyNotAuthorized(action="foo")
 
-        self.assertRaises(exception.PolicyNotAuthorized, fake_func)
+        controller = FakeController()
+        req = fakes.HTTPRequest.blank('')
+        self.assertRaises(
+            exception.PolicyNotAuthorized, controller.fake_func, req
+        )