Wrap quota controller with resource.Resource

Fixes bug 1116137

Currently the format of error message returned by quota extension was
different with quantum other resource. Other resource will return as
json(eg, '{"QuantumError": "error message"}'). But quota extension only
return messages without any format.
'quantum.api.v2.resource.Resource' provider error messages processing.
So wrap quota controller with it.

By the way, fix some small stuff:
* Use specific exception 'QuotaTenantNotFound' instead of generic exception.
* Correct error message.
* Use attribute mapping checking the request body.

Change-Id: I71261198aa79e9ed8e0ae672de32552abdbf45c1

quantum/common/exceptions.py

@@ -228,6 +228,10 @@ class OverQuota(Conflict):
     message = _("Quota exceeded for resources: %(overs)s")
 
 
+class QuotaMissingTenant(BadRequest):
+    message = _("Tenant-id was missing from Quota request")
+
+
 class InvalidQuotaValue(Conflict):
     message = _("Change would make usage less than 0 for the following "
                 "resources: %(unders)s")

quantum/extensions/quotasv2.py

@@ -19,8 +19,10 @@ from oslo.config import cfg
 import webob
 
 from quantum.api import extensions
+from quantum.api.v2.attributes import convert_to_int
 from quantum.api.v2 import base
-from quantum.common import exceptions
+from quantum.api.v2 import resource
+from quantum.common import exceptions as q_exc
 from quantum.manager import QuantumManager
 from quantum.openstack.common import importutils
 from quantum import quota
@@ -49,21 +51,10 @@ class QuotaSetsController(wsgi.Controller):
             attr_dict = EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION]
             attr_dict[quota_resource] = {'allow_post': False,
                                          'allow_put': True,
-                                         'convert_to': int,
+                                         'convert_to': convert_to_int,
                                          'is_visible': True}
         self._update_extended_attributes = False
 
-    def _get_body(self, request):
-        body = self._deserialize(request.body,
-                                 request.best_match_content_type())
-        if self._update_extended_attributes:
-            self._update_attributes()
-
-        attr_info = EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION]
-        req_body = base.Controller.prepare_request_body(
-            request.context, body, False, self._resource_name, attr_info)
-        return req_body
-
     def _get_quotas(self, request, tenant_id):
         return self._driver.get_tenant_quotas(
             request.context, QUOTAS.resources, tenant_id)
@@ -73,8 +64,7 @@ class QuotaSetsController(wsgi.Controller):
 
     def index(self, request):
         context = request.context
-        if not context.is_admin:
+        self._check_admin(context)
-            raise webob.exc.HTTPForbidden()
         return {self._resource_name + "s":
                 self._driver.get_all_quotas(context, QUOTAS.resources)}
 
@@ -82,44 +72,35 @@ class QuotaSetsController(wsgi.Controller):
         """Retrieve the tenant info in context."""
         context = request.context
         if not context.tenant_id:
-            raise webob.exc.HTTPBadRequest('invalid tenant')
+            raise q_exc.QuotaMissingTenant()
         return {'tenant': {'tenant_id': context.tenant_id}}
 
     def show(self, request, id):
-        context = request.context
+        if id != request.context.tenant_id:
-        tenant_id = id
+            self._check_admin(request.context,
-        if not tenant_id:
+                              reason=_("Non-admin is not authorised "
-            raise webob.exc.HTTPBadRequest('invalid tenant')
+                                       "to access quotas for another tenant"))
-        if (tenant_id != context.tenant_id and
+        return {self._resource_name: self._get_quotas(request, id)}
-            not context.is_admin):
-            raise webob.exc.HTTPForbidden()
-        return {self._resource_name:
-                self._get_quotas(request, tenant_id)}
 
-    def _check_modification_delete_privilege(self, context, tenant_id):
+    def _check_admin(self, context,
-        if not tenant_id:
+                     reason=_("Only admin can view or configure quota")):
-            raise webob.exc.HTTPBadRequest('invalid tenant')
         if not context.is_admin:
-            raise webob.exc.HTTPForbidden()
+            raise q_exc.AdminRequired(reason=reason)
-        return tenant_id
 
     def delete(self, request, id):
-        tenant_id = self._check_modification_delete_privilege(request.context,
+        self._check_admin(request.context)
-                                                              id)
+        self._driver.delete_tenant_quota(request.context, id)
-        self._driver.delete_tenant_quota(request.context, tenant_id)
 
-    def update(self, request, id):
+    def update(self, request, id, body=None):
-        tenant_id = self._check_modification_delete_privilege(request.context,
+        self._check_admin(request.context)
-                                                              id)
+        if self._update_extended_attributes:
-        req_body = self._get_body(request)
+            self._update_attributes()
-        for key in req_body[self._resource_name].keys():
+        body = base.Controller.prepare_request_body(
-            if key in QUOTAS.resources:
+            request.context, body, False, self._resource_name,
-                value = int(req_body[self._resource_name][key])
+            EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION])
-                self._driver.update_quota_limit(request.context,
+        for key, value in body[self._resource_name].iteritems():
-                                                tenant_id,
+            self._driver.update_quota_limit(request.context, id, key, value)
-                                                key,
+        return {self._resource_name: self._get_quotas(request, id)}
-                                                value)
-        return {self._resource_name: self._get_quotas(request, tenant_id)}
 
 
 class Quotasv2(extensions.ExtensionDescriptor):
@@ -151,7 +132,9 @@ class Quotasv2(extensions.ExtensionDescriptor):
     @classmethod
     def get_resources(cls):
         """ Returns Ext Resources """
-        controller = QuotaSetsController(QuantumManager.get_plugin())
+        controller = resource.Resource(
+            QuotaSetsController(QuantumManager.get_plugin()),
+            faults=base.FAULT_MAP)
         return [extensions.ResourceExtension(
             Quotasv2.get_alias(),
             controller,

quantum/tests/unit/test_quota_ext.py

@@ -12,7 +12,6 @@ from quantum.db import api as db
 from quantum import manager
 from quantum.plugins.linuxbridge.db import l2network_db_v2
 from quantum import quota
-from quantum.tests import base
 from quantum.tests.unit import test_api_v2
 from quantum.tests.unit import test_extensions
 from quantum.tests.unit import testlib_api
@@ -24,7 +23,6 @@ _get_path = test_api_v2._get_path
 
 
 class QuotaExtensionTestCase(testlib_api.WebTestCase):
-    fmt = 'json'
 
     def setUp(self):
         super(QuotaExtensionTestCase, self).setUp()
@@ -47,10 +45,6 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
 
         # Update the plugin and extensions path
         cfg.CONF.set_override('core_plugin', TARGET_PLUGIN)
-        cfg.CONF.set_override(
-            'quota_driver',
-            'quantum.db.quota_db.DbQuotaDriver',
-            group='QUOTAS')
         cfg.CONF.set_override(
             'quota_items',
             ['network', 'subnet', 'port', 'extra1'],
@@ -68,7 +62,6 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
         app = config.load_paste_app('extensions_test_app')
         ext_middleware = extensions.ExtensionMiddleware(app, ext_mgr=ext_mgr)
         self.api = webtest.TestApp(ext_middleware)
-        super(QuotaExtensionTestCase, self).setUp()
 
     def tearDown(self):
         self._plugin_patcher.stop()
@@ -82,6 +75,17 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
         attributes.RESOURCE_ATTRIBUTE_MAP = self.saved_attr_map
         super(QuotaExtensionTestCase, self).tearDown()
 
+
+class QuotaExtensionDbTestCase(QuotaExtensionTestCase):
+    fmt = 'json'
+
+    def setUp(self):
+        cfg.CONF.set_override(
+            'quota_driver',
+            'quantum.db.quota_db.DbQuotaDriver',
+            group='QUOTAS')
+        super(QuotaExtensionDbTestCase, self).setUp()
+
     def test_quotas_loaded_right(self):
         res = self.api.get(_get_path('quotas', fmt=self.fmt))
         quota = self.deserialize(res)
@@ -106,8 +110,12 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
         res = self.api.get(_get_path('quotas', id=tenant_id, fmt=self.fmt),
                            extra_environ=env)
         self.assertEqual(200, res.status_int)
+        quota = self.deserialize(res)
+        self.assertEqual(10, quota['quota']['network'])
+        self.assertEqual(10, quota['quota']['subnet'])
+        self.assertEqual(50, quota['quota']['port'])
 
-    def test_show_quotas_without_admin_forbidden(self):
+    def test_show_quotas_without_admin_forbidden_returns_403(self):
         tenant_id = 'tenant_id1'
         env = {'quantum.context': context.Context('', tenant_id + '2',
                                                   is_admin=False)}
@@ -115,7 +123,37 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                            extra_environ=env, expect_errors=True)
         self.assertEqual(403, res.status_int)
 
-    def test_update_quotas_without_admin_forbidden(self):
+    def test_show_quotas_with_owner_tenant(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=False)}
+        res = self.api.get(_get_path('quotas', id=tenant_id, fmt=self.fmt),
+                           extra_environ=env)
+        self.assertEqual(200, res.status_int)
+        quota = self.deserialize(res)
+        self.assertEqual(10, quota['quota']['network'])
+        self.assertEqual(10, quota['quota']['subnet'])
+        self.assertEqual(50, quota['quota']['port'])
+
+    def test_list_quotas_with_admin(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=True)}
+        res = self.api.get(_get_path('quotas', fmt=self.fmt),
+                           extra_environ=env)
+        self.assertEqual(200, res.status_int)
+        quota = self.deserialize(res)
+        self.assertEqual([], quota['quotas'])
+
+    def test_list_quotas_without_admin_forbidden_returns_403(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=False)}
+        res = self.api.get(_get_path('quotas', fmt=self.fmt),
+                           extra_environ=env, expect_errors=True)
+        self.assertEqual(403, res.status_int)
+
+    def test_update_quotas_without_admin_forbidden_returns_403(self):
         tenant_id = 'tenant_id1'
         env = {'quantum.context': context.Context('', tenant_id,
                                                   is_admin=False)}
@@ -125,6 +163,26 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                            expect_errors=True)
         self.assertEqual(403, res.status_int)
 
+    def test_update_quotas_with_non_integer_returns_400(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=True)}
+        quotas = {'quota': {'network': 'abc'}}
+        res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt),
+                           self.serialize(quotas), extra_environ=env,
+                           expect_errors=True)
+        self.assertEqual(400, res.status_int)
+
+    def test_update_quotas_with_non_support_resource_returns_400(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=True)}
+        quotas = {'quota': {'abc': 100}}
+        res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt),
+                           self.serialize(quotas), extra_environ=env,
+                           expect_errors=True)
+        self.assertEqual(400, res.status_int)
+
     def test_update_quotas_with_admin(self):
         tenant_id = 'tenant_id1'
         env = {'quantum.context': context.Context('', tenant_id + '2',
@@ -138,6 +196,8 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                            extra_environ=env2)
         quota = self.deserialize(res)
         self.assertEqual(100, quota['quota']['network'])
+        self.assertEqual(10, quota['quota']['subnet'])
+        self.assertEqual(50, quota['quota']['port'])
 
     def test_update_attributes(self):
         tenant_id = 'tenant_id1'
@@ -161,7 +221,7 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                               extra_environ=env)
         self.assertEqual(204, res.status_int)
 
-    def test_delete_quotas_without_admin_forbidden(self):
+    def test_delete_quotas_without_admin_forbidden_returns_403(self):
         tenant_id = 'tenant_id1'
         env = {'quantum.context': context.Context('', tenant_id,
                                                   is_admin=False)}
@@ -169,7 +229,7 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                               extra_environ=env, expect_errors=True)
         self.assertEqual(403, res.status_int)
 
-    def test_quotas_loaded_bad(self):
+    def test_quotas_loaded_bad_returns_404(self):
         try:
             res = self.api.get(_get_path('quotas'), expect_errors=True)
             self.assertEqual(404, res.status_int)
@@ -210,66 +270,31 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase):
                                      tenant_id,
                                      network=-1)
 
+    def test_quotas_get_tenant_from_request_context(self):
+        tenant_id = 'tenant_id1'
+        env = {'quantum.context': context.Context('', tenant_id,
+                                                  is_admin=True)}
+        res = self.api.get(_get_path('quotas/tenant', fmt=self.fmt),
+                           extra_environ=env)
+        self.assertEqual(200, res.status_int)
+        quota = self.deserialize(res)
+        self.assertEqual(quota['tenant']['tenant_id'], tenant_id)
 
-class QuotaExtensionTestCaseXML(QuotaExtensionTestCase):
+    def test_quotas_get_tenant_from_empty_request_context_returns_400(self):
+        env = {'quantum.context': context.Context('', '',
+                                                  is_admin=True)}
+        res = self.api.get(_get_path('quotas/tenant', fmt=self.fmt),
+                           extra_environ=env, expect_errors=True)
+        self.assertEqual(400, res.status_int)
+
+
+class QuotaExtensionDbTestCaseXML(QuotaExtensionDbTestCase):
     fmt = 'xml'
 
 
-class QuotaExtensionCfgTestCase(testlib_api.WebTestCase):
+class QuotaExtensionCfgTestCase(QuotaExtensionTestCase):
     fmt = 'json'
 
-    def setUp(self):
-        super(QuotaExtensionCfgTestCase, self).setUp()
-        db._ENGINE = None
-        db._MAKER = None
-        # Ensure 'stale' patched copies of the plugin are never returned
-        manager.QuantumManager._instance = None
-
-        # Ensure existing ExtensionManager is not used
-        extensions.PluginAwareExtensionManager._instance = None
-
-        # Save the global RESOURCE_ATTRIBUTE_MAP
-        self.saved_attr_map = {}
-        for resource, attrs in attributes.RESOURCE_ATTRIBUTE_MAP.iteritems():
-            self.saved_attr_map[resource] = attrs.copy()
-
-        # Create the default configurations
-        args = ['--config-file', test_extensions.etcdir('quantum.conf.test')]
-        config.parse(args=args)
-
-        # Update the plugin and extensions path
-        cfg.CONF.set_override('core_plugin', TARGET_PLUGIN)
-        cfg.CONF.set_override(
-            'quota_items',
-            ['network', 'subnet', 'port', 'extra1'],
-            group='QUOTAS')
-        quota.QUOTAS = quota.QuotaEngine()
-        quota.register_resources_from_config()
-        self._plugin_patcher = mock.patch(TARGET_PLUGIN, autospec=True)
-        self.plugin = self._plugin_patcher.start()
-        self.plugin.return_value.supported_extension_aliases = ['quotas']
-        # QUOTAS will regester the items in conf when starting
-        # extra1 here is added later, so have to do it manually
-        quota.QUOTAS.register_resource_by_name('extra1')
-        ext_mgr = extensions.PluginAwareExtensionManager.get_instance()
-        l2network_db_v2.initialize()
-        app = config.load_paste_app('extensions_test_app')
-        ext_middleware = extensions.ExtensionMiddleware(app, ext_mgr=ext_mgr)
-        self.api = webtest.TestApp(ext_middleware)
-        super(QuotaExtensionCfgTestCase, self).setUp()
-
-    def tearDown(self):
-        self._plugin_patcher.stop()
-        self.api = None
-        self.plugin = None
-        db._ENGINE = None
-        db._MAKER = None
-        cfg.CONF.reset()
-
-        # Restore the global RESOURCE_ATTRIBUTE_MAP
-        attributes.RESOURCE_ATTRIBUTE_MAP = self.saved_attr_map
-        super(QuotaExtensionCfgTestCase, self).tearDown()
-
     def test_quotas_default_values(self):
         tenant_id = 'tenant_id1'
         env = {'quantum.context': context.Context('', tenant_id)}