Allow api_version_request.matches to accept a string

According to the Cinder devref you should be able to use the following
pattern:

    if req_version.matches("3.1", "3.5"):
        ....stuff....
    elif req_version.matches("3.6", "3.10"):
        ....other stuff....
    elif req_version > api_version_request.APIVersionRequest("3.10"):
        ....more stuff.....

However, the api_version_request.matches() function will not accept a
string, it requires an api_version_request object.

Fix this to accept a string, as well as None for object.

Closes-Bug: 1550337
Change-Id: Ic90f4a13cfad601a181fd3a91684ecbb2b2fba74

diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py
index 175cb2284515495fdc0befb1d8375b21e6a9a5a3..fd242dad5a48794b1e6886c62fd097170a08981d 100644 (file)
--- a/cinder/api/openstack/api_version_request.py
+++ b/cinder/api/openstack/api_version_request.py
@@ -143,11 +143,21 @@ class APIVersionRequest(utils.ComparableMixin):
 
         if self.is_null():
             raise ValueError
-        if max_version.is_null() and min_version.is_null():
+
+        if isinstance(min_version, str):
+            min_version = APIVersionRequest(version_string=min_version)
+        if isinstance(max_version, str):
+            max_version = APIVersionRequest(version_string=max_version)
+
+        if not min_version and not max_version:
             return True
-        elif max_version.is_null():
+        elif ((min_version and max_version) and
+              max_version.is_null() and min_version.is_null()):
+            return True
+
+        elif not max_version or max_version.is_null():
             return min_version <= self
-        elif min_version.is_null():
+        elif not min_version or min_version.is_null():
             return self <= max_version
         else:
             return min_version <= self <= max_version

diff --git a/cinder/tests/unit/api/test_versions.py b/cinder/tests/unit/api/test_versions.py
index 1c8fd8969d272e8953737e3eeca9fc95287fde59..276c5bc9b6d0ea075f68d919c4b9029ed042f8d4 100644 (file)
--- a/cinder/tests/unit/api/test_versions.py
+++ b/cinder/tests/unit/api/test_versions.py
@@ -16,6 +16,7 @@
 import ddt
 import mock
 from oslo_serialization import jsonutils
+from oslo_utils import encodeutils
 
 from cinder.api.openstack import api_version_request
 from cinder.api.openstack import wsgi
@@ -37,9 +38,12 @@ class VersionsControllerTestCase(test.TestCase):
         super(VersionsControllerTestCase, self).setUp()
         self.wsgi_apps = (versions.Versions(), router.APIRouter())
 
-    def build_request(self, base_url='http://localhost/v3',
+    def build_request(self, base_dir=None, base_url='http://localhost/v3',
                       header_version=None):
-        req = fakes.HTTPRequest.blank('/', base_url=base_url)
+        if base_dir:
+            req = fakes.HTTPRequest.blank(base_dir, base_url=base_url)
+        else:
+            req = fakes.HTTPRequest.blank('/', base_url=base_url)
         req.method = 'GET'
         req.content_type = 'application/json'
         if header_version:
@@ -49,8 +53,6 @@ class VersionsControllerTestCase(test.TestCase):
         return req
 
     def check_response(self, response, version):
-        self.assertEqual(VOLUME_SERVICE + version,
-                         response.headers[VERSION_HEADER_NAME])
         self.assertEqual(VOLUME_SERVICE + version,
                          response.headers[VERSION_HEADER_NAME])
         self.assertEqual(VERSION_HEADER_NAME, response.headers['Vary'])
@@ -155,6 +157,7 @@ class VersionsControllerTestCase(test.TestCase):
         response = req.get_response(router.APIRouter())
 
         self.assertEqual(200, response.status_int)
+        self.check_response(response, api_version_request._MAX_API_VERSION)
 
     def test_versions_version_invalid(self):
         req = self.build_request(header_version='2.0.1')
@@ -189,8 +192,6 @@ class VersionsControllerTestCase(test.TestCase):
         response = req.get_response(router.APIRouter())
 
         self.assertEqual(406, response.status_int)
-        self.assertEqual('4.0', response.headers[VERSION_HEADER_NAME])
-        self.assertEqual(VERSION_HEADER_NAME, response.headers['Vary'])
 
     @ddt.data(['volume 3.0, compute 2.22', True],
               ['volume 3.0, compute 2.22, identity 2.3', True],
@@ -221,3 +222,104 @@ class VersionsControllerTestCase(test.TestCase):
                          version_list[0].get('version'))
         self.assertEqual(api_version_request._MIN_API_VERSION,
                          version_list[0].get('min_version'))
+
+    @ddt.data(['3.5', 200], ['3.55', 404])
+    @ddt.unpack
+    def test_req_version_matches(self, version, HTTP_ret):
+        version_request = api_version_request.APIVersionRequest(version)
+        self.mock_object(api_version_request,
+                         'max_api_version',
+                         mock.Mock(return_value=version_request))
+
+        class Controller(wsgi.Controller):
+
+            @wsgi.Controller.api_version('3.0', '3.6')
+            def index(self, req):
+                return 'off'
+
+        req = self.build_request(base_dir='/tests', header_version=version)
+        app = fakes.TestRouter(Controller())
+
+        response = req.get_response(app)
+        resp = encodeutils.safe_decode(response.body, incoming='utf-8')
+
+        if HTTP_ret == 200:
+            self.assertEqual('off', resp)
+        elif HTTP_ret == 404:
+            self.assertNotEqual('off', resp)
+        self.assertEqual(HTTP_ret, response.status_int)
+
+    @ddt.data(['3.5', 'older'], ['3.37', 'newer'])
+    @ddt.unpack
+    def test_req_version_matches_with_if(self, version, ret_val):
+        version_request = api_version_request.APIVersionRequest(version)
+        self.mock_object(api_version_request,
+                         'max_api_version',
+                         mock.Mock(return_value=version_request))
+
+        class Controller(wsgi.Controller):
+
+            def index(self, req):
+                req_version = req.api_version_request
+                if req_version.matches('3.1', '3.8'):
+                    return 'older'
+                if req_version.matches('3.9', '8.8'):
+                    return 'newer'
+
+        req = self.build_request(base_dir='/tests', header_version=version)
+        app = fakes.TestRouter(Controller())
+
+        response = req.get_response(app)
+
+        resp = encodeutils.safe_decode(response.body, incoming='utf-8')
+        self.assertEqual(ret_val, resp)
+        self.assertEqual(200, response.status_int)
+
+    @ddt.data(['3.5', 'older'], ['3.37', 'newer'])
+    @ddt.unpack
+    def test_req_version_matches_with_None(self, version, ret_val):
+        version_request = api_version_request.APIVersionRequest(version)
+        self.mock_object(api_version_request,
+                         'max_api_version',
+                         mock.Mock(return_value=version_request))
+
+        class Controller(wsgi.Controller):
+
+            def index(self, req):
+                req_version = req.api_version_request
+                if req_version.matches(None, '3.8'):
+                    return 'older'
+                if req_version.matches('3.9', None):
+                    return 'newer'
+
+        req = self.build_request(base_dir='/tests', header_version=version)
+        app = fakes.TestRouter(Controller())
+
+        response = req.get_response(app)
+
+        resp = encodeutils.safe_decode(response.body, incoming='utf-8')
+        self.assertEqual(ret_val, resp)
+        self.assertEqual(200, response.status_int)
+
+    def test_req_version_matches_with_None_None(self):
+        version_request = api_version_request.APIVersionRequest('3.39')
+        self.mock_object(api_version_request,
+                         'max_api_version',
+                         mock.Mock(return_value=version_request))
+
+        class Controller(wsgi.Controller):
+
+            def index(self, req):
+                req_version = req.api_version_request
+                # This case is artificial, and will return True
+                if req_version.matches(None, None):
+                    return "Pass"
+
+        req = self.build_request(base_dir='/tests', header_version='3.39')
+        app = fakes.TestRouter(Controller())
+
+        response = req.get_response(app)
+
+        resp = encodeutils.safe_decode(response.body, incoming='utf-8')
+        self.assertEqual("Pass", resp)
+        self.assertEqual(200, response.status_int)