From 8143c80213ae53b87644b68b16303d582dacb227 Mon Sep 17 00:00:00 2001 From: Anderson Mesquita Date: Mon, 25 Nov 2013 19:16:16 -0500 Subject: [PATCH] Add collection count to stack list Add an optional parameter ``with_count`` to the stack_list API to allow the inclusion of the collection count in the body of the response When passing the parameter, the response looks like: { 'stacks': [...], 'count': 42 } blueprint: count-stacks Change-Id: I560cfff9d327db9b325e7cf4786bd2fcd71d47b2 --- heat/api/openstack/v1/stacks.py | 16 +++++++- heat/api/openstack/v1/views/stacks_view.py | 4 +- heat/db/api.py | 4 +- heat/db/sqlalchemy/api.py | 6 ++- heat/db/sqlalchemy/filters.py | 2 + heat/engine/service.py | 10 +++++ heat/rpc/client.py | 10 +++++ heat/tests/test_api_openstack_v1.py | 37 +++++++++++++++++++ ..._openstack_v1_views_stacks_view_builder.py | 22 +++++++++++ heat/tests/test_sqlalchemy_api.py | 21 ++++++++--- 10 files changed, 119 insertions(+), 13 deletions(-) diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py index 38bc2299e2..754515ef4a 100644 --- a/heat/api/openstack/v1/stacks.py +++ b/heat/api/openstack/v1/stacks.py @@ -167,10 +167,22 @@ class StackController(object): } params = util.get_allowed_params(req.params, whitelist) filter_params = util.get_allowed_params(req.params, filter_whitelist) - stacks = self.engine.list_stacks(req.context, filters=filter_params, + + stacks = self.engine.list_stacks(req.context, + filters=filter_params, **params) - return stacks_view.collection(req, stacks) + count = None + if req.params.get('with_count'): + try: + # Check if engine has been updated to a version with + # support to count_stacks before trying to use it. + count = self.engine.count_stacks(req.context, + filters=filter_params) + except AttributeError as exc: + logger.warning("Old Engine Version: %s" % str(exc)) + + return stacks_view.collection(req, stacks=stacks, count=count) @util.tenant_local def detail(self, req): diff --git a/heat/api/openstack/v1/views/stacks_view.py b/heat/api/openstack/v1/views/stacks_view.py index 32ca983ac6..3d83ca275d 100644 --- a/heat/api/openstack/v1/views/stacks_view.py +++ b/heat/api/openstack/v1/views/stacks_view.py @@ -57,12 +57,14 @@ def format_stack(req, stack, keys=None): transform(k, v) for k, v in stack.items())) -def collection(req, stacks): +def collection(req, stacks, count=None): formatted_stacks = [format_stack(req, s, basic_keys) for s in stacks] result = {'stacks': formatted_stacks} links = views_common.get_collection_links(req, formatted_stacks) if links: result['links'] = links + if count is not None: + result['count'] = count return result diff --git a/heat/db/api.py b/heat/db/api.py index f3f64007a6..1a9fb564e7 100644 --- a/heat/db/api.py +++ b/heat/db/api.py @@ -129,8 +129,8 @@ def stack_get_all_by_tenant(context, limit=None, sort_keys=None, marker, sort_dir, filters) -def stack_count_all_by_tenant(context): - return IMPL.stack_count_all_by_tenant(context) +def stack_count_all_by_tenant(context, filters=None): + return IMPL.stack_count_all_by_tenant(context, filters=filters) def stack_create(context, values): diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index 30604cbb8f..065fd987bc 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -336,8 +336,10 @@ def stack_get_all_by_tenant(context, limit=None, sort_keys=None, marker=None, marker, sort_dir).all() -def stack_count_all_by_tenant(context): - return _query_stack_get_all_by_tenant(context).count() +def stack_count_all_by_tenant(context, filters=None): + query = _query_stack_get_all_by_tenant(context) + query = db_filters.exact_filter(query, models.Stack, filters) + return query.count() def stack_create(context, values): diff --git a/heat/db/sqlalchemy/filters.py b/heat/db/sqlalchemy/filters.py index d8483ced63..7771c34d1e 100644 --- a/heat/db/sqlalchemy/filters.py +++ b/heat/db/sqlalchemy/filters.py @@ -30,6 +30,8 @@ def exact_filter(query, model, filters): """ filter_dict = {} + if filters is None: + filters = {} for key, value in filters.iteritems(): if isinstance(value, (list, tuple, set, frozenset)): diff --git a/heat/engine/service.py b/heat/engine/service.py index 05f2651afa..7e76b6cd11 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -235,6 +235,16 @@ class EngineService(service.Service): sort_dir, filters) or [] return list(format_stack_details(stacks)) + @request_context + def count_stacks(self, cnxt, filters=None): + """ + Return the number of stacks that match the given filters + :param ctxt: RPC context. + :param filters: a dict of ATTR:VALUE to match agains stacks + :returns: a integer representing the number of matched stacks + """ + return db_api.stack_count_all_by_tenant(cnxt, filters=filters) + def _validate_deferred_auth_context(self, cnxt, stack): if cfg.CONF.deferred_auth_method != 'password': return diff --git a/heat/rpc/client.py b/heat/rpc/client.py index 8909f3e235..7d97da0f87 100644 --- a/heat/rpc/client.py +++ b/heat/rpc/client.py @@ -69,6 +69,16 @@ class EngineClient(heat.openstack.common.rpc.proxy.RpcProxy): sort_keys=sort_keys, marker=marker, sort_dir=sort_dir, filters=filters)) + def count_stacks(self, ctxt, filters=None): + """ + Return the number of stacks that match the given filters + :param ctxt: RPC context. + :param filters: a dict of ATTR:VALUE to match agains stacks + :returns: a integer representing the number of matched stacks + """ + return self.call(ctxt, self.make_msg('count_stacks', + filters=filters)) + def show_stack(self, ctxt, stack_identity): """ Return detailed information about one or all stacks. diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index 3ce889751e..327f8862da 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -26,6 +26,7 @@ from heat.common.wsgi import Request from heat.common import urlfetch from heat.openstack.common.rpc import common as rpc_common from heat.rpc import api as rpc_api +from heat.rpc import client as rpc_client from heat.tests.common import HeatTestCase import heat.api.openstack.v1 as api_v1 @@ -371,6 +372,7 @@ class StackControllerTest(ControllerTest, HeatTestCase): self.assertIn('sort_keys', engine_args) self.assertIn('marker', engine_args) self.assertIn('sort_dir', engine_args) + self.assertIn('filters', engine_args) self.assertNotIn('balrog', engine_args) @mock.patch.object(rpc, 'call') @@ -395,6 +397,41 @@ class StackControllerTest(ControllerTest, HeatTestCase): self.assertIn('name', filters) self.assertNotIn('balrog', filters) + def test_index_returns_stack_count_if_with_count_is_truthy(self): + params = {'with_count': 'True'} + req = self._get('/stacks', params=params) + engine = self.controller.engine + + engine.list_stacks = mock.Mock(return_value=[]) + engine.count_stacks = mock.Mock(return_value=0) + + result = self.controller.index(req, tenant_id='fake_tenant_id') + self.assertEqual(0, result['count']) + + def test_index_doesnt_return_stack_count_if_with_count_is_falsy(self): + params = {'with_count': ''} + req = self._get('/stacks', params=params) + engine = self.controller.engine + + engine.list_stacks = mock.Mock(return_value=[]) + engine.count_stacks = mock.Mock() + + result = self.controller.index(req, tenant_id='fake_tenant_id') + self.assertNotIn('count', result) + assert not engine.count_stacks.called + + @mock.patch.object(rpc_client.EngineClient, 'count_stacks') + def test_index_doesnt_break_with_old_engine(self, mock_count_stacks): + params = {'with_count': 'Truthy'} + req = self._get('/stacks', params=params) + engine = self.controller.engine + + engine.list_stacks = mock.Mock(return_value=[]) + mock_count_stacks.side_effect = AttributeError("Should not exist") + + result = self.controller.index(req, tenant_id='fake_tenant_id') + self.assertNotIn('count', result) + @mock.patch.object(rpc, 'call') def test_detail(self, mock_call): req = self._get('/stacks/detail') diff --git a/heat/tests/test_api_openstack_v1_views_stacks_view_builder.py b/heat/tests/test_api_openstack_v1_views_stacks_view_builder.py index 1f57773f74..8809531371 100644 --- a/heat/tests/test_api_openstack_v1_views_stacks_view_builder.py +++ b/heat/tests/test_api_openstack_v1_views_stacks_view_builder.py @@ -134,3 +134,25 @@ class TestStacksViewBuilder(HeatTestCase): mock_get_collection_links.return_value = None stack_view = stacks_view.collection(self.request, stacks) self.assertNotIn('links', stack_view) + + @mock.patch.object(stacks_view.views_common, 'get_collection_links') + def test_append_collection_count(self, mock_get_collection_links): + stacks = [self.stack1] + count = 1 + stack_view = stacks_view.collection(self.request, stacks, count) + self.assertIn('count', stack_view) + self.assertEqual(1, stack_view['count']) + + @mock.patch.object(stacks_view.views_common, 'get_collection_links') + def test_doesnt_append_collection_count(self, mock_get_collection_links): + stacks = [self.stack1] + stack_view = stacks_view.collection(self.request, stacks) + self.assertNotIn('count', stack_view) + + @mock.patch.object(stacks_view.views_common, 'get_collection_links') + def test_appends_collection_count_of_zero(self, mock_get_collection_links): + stacks = [self.stack1] + count = 0 + stack_view = stacks_view.collection(self.request, stacks, count) + self.assertIn('count', stack_view) + self.assertEqual(0, stack_view['count']) diff --git a/heat/tests/test_sqlalchemy_api.py b/heat/tests/test_sqlalchemy_api.py index 902b04e685..3696e365c5 100644 --- a/heat/tests/test_sqlalchemy_api.py +++ b/heat/tests/test_sqlalchemy_api.py @@ -219,8 +219,8 @@ class SqlAlchemyTest(HeatTestCase): self.assertEqual(1, len(st_db)) def test_stack_get_all_by_tenant_and_filters(self): - stack1 = self._setup_test_stack('foo', UUIDs[0]) - stack2 = self._setup_test_stack('bar', UUIDs[1]) + stack1 = self._setup_test_stack('foo', UUID1) + stack2 = self._setup_test_stack('bar', UUID2) stacks = [stack1, stack2] filters = {'name': 'foo'} @@ -231,8 +231,8 @@ class SqlAlchemyTest(HeatTestCase): self.assertEqual('foo', results[0]['name']) def test_stack_get_all_by_tenant_filter_matches_in_list(self): - stack1 = self._setup_test_stack('foo', UUIDs[0]) - stack2 = self._setup_test_stack('bar', UUIDs[1]) + stack1 = self._setup_test_stack('foo', UUID1) + stack2 = self._setup_test_stack('bar', UUID2) stacks = [stack1, stack2] filters = {'name': ['bar', 'quux']} @@ -243,8 +243,8 @@ class SqlAlchemyTest(HeatTestCase): self.assertEqual('bar', results[0]['name']) def test_stack_get_all_by_tenant_returns_all_if_no_filters(self): - stack1 = self._setup_test_stack('foo', UUIDs[0]) - stack2 = self._setup_test_stack('bar', UUIDs[1]) + stack1 = self._setup_test_stack('foo', UUID1) + stack2 = self._setup_test_stack('bar', UUID2) stacks = [stack1, stack2] filters = None @@ -328,6 +328,15 @@ class SqlAlchemyTest(HeatTestCase): st_db = db_api.stack_count_all_by_tenant(self.ctx) self.assertEqual(1, st_db) + def test_stack_count_all_by_tenant_with_filters(self): + stack1 = self._setup_test_stack('foo', UUID1) + stack2 = self._setup_test_stack('bar', UUID2) + stack3 = self._setup_test_stack('bar', UUID3) + filters = {'name': 'bar'} + + st_db = db_api.stack_count_all_by_tenant(self.ctx, filters=filters) + self.assertEqual(2, st_db) + def test_event_get_all_by_stack(self): stack = self._setup_test_stack('stack', UUID1)[1]