From b39e1469e824bc8bc79e1ecafa98825a94811c0b Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 23 Jun 2015 04:54:28 -0700 Subject: [PATCH] Add plural names for quota resources Introduce a 'plural_name' attribute in the BaseResource class in the neutron.quota.resource module. This will enable to specify the plural name of a resource when it's entered in the quota registry. This will come handy for managing reservations and also removes the need for passing the collection name when counting a resource. Also, the name of the 'resources' parameter for the _count_resource routine has ben changed to collection_name, as the previous one was misleading, since it led to believe it should be used for a collection, whereas it is simply the name of a collection. Change-Id: I0b61ba1856e9552cc14574be28fec6c77fb8783c Related-Blueprint: better-quotas --- neutron/api/v2/base.py | 3 +- neutron/quota/resource.py | 58 ++++++++++++++++++----- neutron/quota/resource_registry.py | 13 +++-- neutron/tests/unit/quota/test_resource.py | 46 +++++++++++++----- 4 files changed, 90 insertions(+), 30 deletions(-) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index a1841b8cb8d..cd591b4f9ea 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -431,8 +431,7 @@ class Controller(object): try: tenant_id = item[self._resource]['tenant_id'] count = quota.QUOTAS.count(request.context, self._resource, - self._plugin, self._collection, - tenant_id) + self._plugin, tenant_id) if bulk: delta = deltas.get(tenant_id, 0) + 1 deltas[tenant_id] = delta diff --git a/neutron/quota/resource.py b/neutron/quota/resource.py index 25bcba761bd..d9a716a5eb8 100644 --- a/neutron/quota/resource.py +++ b/neutron/quota/resource.py @@ -26,8 +26,8 @@ from neutron.i18n import _LE LOG = log.getLogger(__name__) -def _count_resource(context, plugin, resources, tenant_id): - count_getter_name = "get_%s_count" % resources +def _count_resource(context, plugin, collection_name, tenant_id): + count_getter_name = "get_%s_count" % collection_name # Some plugins support a count method for particular resources, # using a DB's optimized counting features. We try to use that one @@ -38,7 +38,7 @@ def _count_resource(context, plugin, resources, tenant_id): meh = obj_count_getter(context, filters={'tenant_id': [tenant_id]}) return meh except (NotImplementedError, AttributeError): - obj_getter = getattr(plugin, "get_%s" % resources) + obj_getter = getattr(plugin, "get_%s" % collection_name) obj_list = obj_getter(context, filters={'tenant_id': [tenant_id]}) return len(obj_list) if obj_list else 0 @@ -46,14 +46,33 @@ def _count_resource(context, plugin, resources, tenant_id): class BaseResource(object): """Describe a single resource for quota checking.""" - def __init__(self, name, flag): + def __init__(self, name, flag, plural_name=None): """Initializes a resource. :param name: The name of the resource, i.e., "instances". :param flag: The name of the flag or configuration option + :param plural_name: Plural form of the resource name. If not + specified, it is generated automatically by + appending an 's' to the resource name, unless + it ends with a 'y'. In that case the last + letter is removed, and 'ies' is appended. + Dashes are always converted to underscores. """ self.name = name + # If a plural name is not supplied, default to adding an 's' to + # the resource name, unless the resource name ends in 'y', in which + # case remove the 'y' and add 'ies'. Even if the code should not fiddle + # too much with English grammar, this is a rather common and easy to + # implement rule. + if plural_name: + self.plural_name = plural_name + elif self.name[-1] == 'y': + self.plural_name = "%sies" % self.name[:-1] + else: + self.plural_name = "%ss" % self.name + # always convert dashes to underscores + self.plural_name = self.plural_name.replace('-', '_') self.flag = flag @property @@ -79,7 +98,7 @@ class BaseResource(object): class CountableResource(BaseResource): """Describe a resource where the counts are determined by a function.""" - def __init__(self, name, count, flag=None): + def __init__(self, name, count, flag=None, plural_name=None): """Initializes a CountableResource. Countable resources are those resources which directly @@ -100,16 +119,26 @@ class CountableResource(BaseResource): :param flag: The name of the flag or configuration option which specifies the default value of the quota for this resource. + :param plural_name: Plural form of the resource name. If not + specified, it is generated automatically by + appending an 's' to the resource name, unless + it ends with a 'y'. In that case the last + letter is removed, and 'ies' is appended. + Dashes are always converted to underscores. """ - super(CountableResource, self).__init__(name, flag=flag) - self.count = count + super(CountableResource, self).__init__( + name, flag=flag, plural_name=plural_name) + self._count_func = count + + def count(self, context, plugin, tenant_id): + return self._count_func(context, plugin, self.plural_name, tenant_id) class TrackedResource(BaseResource): """Resource which keeps track of its usage data.""" - def __init__(self, name, model_class, flag): + def __init__(self, name, model_class, flag, plural_name=None): """Initializes an instance for a given resource. TrackedResource are directly mapped to data model classes. @@ -126,8 +155,16 @@ class TrackedResource(BaseResource): :param flag: The name of the flag or configuration option which specifies the default value of the quota for this resource. + :param plural_name: Plural form of the resource name. If not + specified, it is generated automatically by + appending an 's' to the resource name, unless + it ends with a 'y'. In that case the last + letter is removed, and 'ies' is appended. + Dashes are always converted to underscores. + """ - super(TrackedResource, self).__init__(name, flag) + super(TrackedResource, self).__init__( + name, flag=flag, plural_name=plural_name) # Register events for addition/removal of records in the model class # As tenant_id is immutable for all Neutron objects there is no need # to register a listener for update events @@ -198,8 +235,7 @@ class TrackedResource(BaseResource): # Update quota usage return self._resync(context, tenant_id, in_use) - def count(self, context, _plugin, _resources, tenant_id, - resync_usage=False): + def count(self, context, _plugin, tenant_id, resync_usage=False): """Return the current usage count for the resource.""" # Load current usage data usage_info = quota_api.get_quota_usage_by_resource_and_tenant( diff --git a/neutron/quota/resource_registry.py b/neutron/quota/resource_registry.py index a0dfeebe7dc..d0263e87614 100644 --- a/neutron/quota/resource_registry.py +++ b/neutron/quota/resource_registry.py @@ -27,8 +27,9 @@ def register_resource(resource): ResourceRegistry.get_instance().register_resource(resource) -def register_resource_by_name(resource_name): - ResourceRegistry.get_instance().register_resource_by_name(resource_name) +def register_resource_by_name(resource_name, plural_name=None): + ResourceRegistry.get_instance().register_resource_by_name( + resource_name, plural_name) def get_all_resources(): @@ -151,7 +152,7 @@ class ResourceRegistry(object): def __contains__(self, resource): return resource in self._resources - def _create_resource_instance(self, resource_name): + def _create_resource_instance(self, resource_name, plural_name): """Factory function for quota Resource. This routine returns a resource instance of the appropriate type @@ -220,9 +221,11 @@ class ResourceRegistry(object): for res in resources: self.register_resource(res) - def register_resource_by_name(self, resource_name): + def register_resource_by_name(self, resource_name, + plural_name=None): """Register a resource by name.""" - resource = self._create_resource_instance(resource_name) + resource = self._create_resource_instance( + resource_name, plural_name) self.register_resource(resource) def unregister_resources(self): diff --git a/neutron/tests/unit/quota/test_resource.py b/neutron/tests/unit/quota/test_resource.py index d1e890cd080..7f668539807 100644 --- a/neutron/tests/unit/quota/test_resource.py +++ b/neutron/tests/unit/quota/test_resource.py @@ -31,6 +31,33 @@ meh_quota_opts = [cfg.IntOpt(meh_quota_flag, default=99)] random.seed() +class TestResource(base.DietTestCase): + """Unit tests for neutron.quota.resource.BaseResource""" + + def test_create_resource_without_plural_name(self): + res = resource.BaseResource('foo', None) + self.assertEqual('foos', res.plural_name) + res = resource.BaseResource('foy', None) + self.assertEqual('foies', res.plural_name) + + def test_create_resource_with_plural_name(self): + res = resource.BaseResource('foo', None, + plural_name='foopsies') + self.assertEqual('foopsies', res.plural_name) + + def test_resource_default_value(self): + res = resource.BaseResource('foo', 'foo_quota') + with mock.patch('oslo_config.cfg.CONF') as mock_cfg: + mock_cfg.QUOTAS.foo_quota = 99 + self.assertEqual(99, res.default) + + def test_resource_negative_default_value(self): + res = resource.BaseResource('foo', 'foo_quota') + with mock.patch('oslo_config.cfg.CONF') as mock_cfg: + mock_cfg.QUOTAS.foo_quota = -99 + self.assertEqual(-1, res.default) + + class TestTrackedResource(testlib_api.SqlTestCaseLight): def _add_data(self, tenant_id=None): @@ -98,9 +125,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): self.context, self.resource, dirty=False) # Expect correct count to be returned anyway since the first call to # count() always resyncs with the db - self.assertEqual(2, res.count(self.context, - None, None, - self.tenant_id)) + self.assertEqual(2, res.count(self.context, None, self.tenant_id)) def _test_count(self): res = self._create_resource() @@ -111,14 +136,14 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): def test_count_with_dirty_false(self): res = self._test_count() - res.count(self.context, None, None, self.tenant_id) + res.count(self.context, None, self.tenant_id) # At this stage count has been invoked, and the dirty flag should be # false. Another invocation of count should not query the model class set_quota = 'neutron.db.quota.api.set_quota_usage' with mock.patch(set_quota) as mock_set_quota: self.assertEqual(0, mock_set_quota.call_count) self.assertEqual(2, res.count(self.context, - None, None, + None, self.tenant_id)) def test_count_with_dirty_true_resync(self): @@ -126,7 +151,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): # Expect correct count to be returned, which also implies # set_quota_usage has been invoked with the correct parameters self.assertEqual(2, res.count(self.context, - None, None, + None, self.tenant_id, resync_usage=True)) @@ -137,7 +162,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): quota_api.set_quota_usage_dirty(self.context, self.resource, self.tenant_id) - res.count(self.context, None, None, self.tenant_id, + res.count(self.context, None, self.tenant_id, resync_usage=True) mock_set_quota_usage.assert_called_once_with( self.context, self.resource, self.tenant_id, in_use=2) @@ -147,9 +172,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): self._add_data() # Invoke count without having usage info in DB - Expect correct # count to be returned - self.assertEqual(2, res.count(self.context, - None, None, - self.tenant_id)) + self.assertEqual(2, res.count(self.context, None, self.tenant_id)) def test_count_with_dirty_true_no_usage_info_calls_set_quota_usage(self): res = self._create_resource() @@ -159,8 +182,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): quota_api.set_quota_usage_dirty(self.context, self.resource, self.tenant_id) - res.count(self.context, None, None, self.tenant_id, - resync_usage=True) + res.count(self.context, None, self.tenant_id, resync_usage=True) mock_set_quota_usage.assert_called_once_with( self.context, self.resource, self.tenant_id, in_use=2)