From 8dc6ba39f3d6ad56894ebb82810a1458daff41ae Mon Sep 17 00:00:00 2001 From: Richard Lee Date: Wed, 18 Dec 2013 15:53:21 -0600 Subject: [PATCH] Add filter and pagination to stack_get_all This change prepares the ground for calls that are not tenant-scoped to make use of the filter and paging capabilities. Implements: blueprint management-api Change-Id: I4862752999629666b46da20c2713edc950c5d52e --- heat/db/api.py | 6 +- heat/db/sqlalchemy/api.py | 37 ++++++---- heat/tests/test_sqlalchemy_api.py | 108 ++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 14 deletions(-) diff --git a/heat/db/api.py b/heat/db/api.py index 75dce186cc..c1519f4b04 100644 --- a/heat/db/api.py +++ b/heat/db/api.py @@ -120,8 +120,10 @@ def stack_get_by_name(context, stack_name): return IMPL.stack_get_by_name(context, stack_name) -def stack_get_all(context): - return IMPL.stack_get_all(context) +def stack_get_all(context, limit=None, sort_keys=None, marker=None, + sort_dir=None, filters=None): + return IMPL.stack_get_all(context, limit, sort_keys, + marker, sort_dir, filters) def stack_get_all_by_owner_id(context, owner_id): diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index ffac46cda6..d2a114c45a 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -276,12 +276,6 @@ def stack_get(context, stack_id, show_deleted=False, tenant_safe=True): return result -def stack_get_all(context): - results = soft_delete_aware_query(context, models.Stack).\ - filter_by(owner_id=None).all() - return results - - def stack_get_all_by_owner_id(context, owner_id): results = soft_delete_aware_query(context, models.Stack).\ filter_by(owner_id=owner_id).all() @@ -329,15 +323,35 @@ def _paginate_query(context, query, model, limit=None, sort_keys=None, def _query_stack_get_all_by_tenant(context): - query = soft_delete_aware_query(context, models.Stack).\ - filter_by(owner_id=None).\ + query = _query_stack_get_all(context).\ filter_by(tenant=context.tenant_id) return query +def _query_stack_get_all(context): + query = soft_delete_aware_query(context, models.Stack).\ + filter_by(owner_id=None) + + return query + + +def stack_get_all(context, limit=None, sort_keys=None, marker=None, + sort_dir=None, filters=None): + query = _query_stack_get_all(context) + return _filter_and_page_query(context, query, limit, sort_keys, + marker, sort_dir, filters).all() + + def stack_get_all_by_tenant(context, limit=None, sort_keys=None, marker=None, sort_dir=None, filters=None): + query = _query_stack_get_all_by_tenant(context) + return _filter_and_page_query(context, query, limit, sort_keys, + marker, sort_dir, filters).all() + + +def _filter_and_page_query(context, query, limit=None, sort_keys=None, + marker=None, sort_dir=None, filters=None): if filters is None: filters = {} @@ -345,12 +359,11 @@ def stack_get_all_by_tenant(context, limit=None, sort_keys=None, marker=None, models.Stack.status.key, models.Stack.created_at.key, models.Stack.updated_at.key] - filtered_keys = _filter_sort_keys(sort_keys, allowed_sort_keys) + whitelisted_sort_keys = _filter_sort_keys(sort_keys, allowed_sort_keys) - query = _query_stack_get_all_by_tenant(context) query = db_filters.exact_filter(query, models.Stack, filters) - return _paginate_query(context, query, models.Stack, limit, filtered_keys, - marker, sort_dir).all() + return _paginate_query(context, query, models.Stack, limit, + whitelisted_sort_keys, marker, sort_dir) def stack_count_all_by_tenant(context, filters=None): diff --git a/heat/tests/test_sqlalchemy_api.py b/heat/tests/test_sqlalchemy_api.py index 9bcba0abdb..be6056ecac 100644 --- a/heat/tests/test_sqlalchemy_api.py +++ b/heat/tests/test_sqlalchemy_api.py @@ -130,6 +130,114 @@ class SqlAlchemyTest(HeatTestCase): get = fc.client.get_servers_9999 get().MultipleTimes().AndRaise(novaclient.exceptions.NotFound(404)) + @mock.patch.object(db_api, '_paginate_query') + def test_filter_and_page_query_paginates_query(self, mock_paginate_query): + query = mock.Mock() + db_api._filter_and_page_query(self.ctx, query) + + assert mock_paginate_query.called + + @mock.patch.object(db_api.db_filters, 'exact_filter') + def test_filter_and_page_query_handles_no_filters(self, mock_db_filter): + query = mock.Mock() + db_api._filter_and_page_query(self.ctx, query) + + mock_db_filter.assert_called_once_with(mock.ANY, mock.ANY, {}) + + @mock.patch.object(db_api.db_filters, 'exact_filter') + def test_filter_and_page_query_applies_filters(self, mock_db_filter): + query = mock.Mock() + filters = {'foo': 'bar'} + db_api._filter_and_page_query(self.ctx, query, filters=filters) + + assert mock_db_filter.called + + @mock.patch.object(db_api, '_paginate_query') + def test_filter_and_page_query_whitelists_sort_keys(self, + mock_paginate_query): + query = mock.Mock() + sort_keys = ['name', 'foo'] + db_api._filter_and_page_query(self.ctx, query, sort_keys=sort_keys) + + args, _ = mock_paginate_query.call_args + self.assertIn(['name'], args) + + @mock.patch.object(db_api.utils, 'paginate_query') + def test_paginate_query_default_sorts_by_created_at_and_id( + self, mock_paginate_query): + query = mock.Mock() + model = mock.Mock() + db_api._paginate_query(self.ctx, query, model, sort_keys=None) + args, _ = mock_paginate_query.call_args + self.assertIn(['created_at', 'id'], args) + + @mock.patch.object(db_api.utils, 'paginate_query') + def test_paginate_query_default_sorts_dir_by_desc(self, + mock_paginate_query): + query = mock.Mock() + model = mock.Mock() + db_api._paginate_query(self.ctx, query, model, sort_dir=None) + args, _ = mock_paginate_query.call_args + self.assertIn('desc', args) + + @mock.patch.object(db_api.utils, 'paginate_query') + def test_paginate_query_uses_given_sort_plus_id(self, + mock_paginate_query): + query = mock.Mock() + model = mock.Mock() + db_api._paginate_query(self.ctx, query, model, sort_keys=['name']) + args, _ = mock_paginate_query.call_args + self.assertIn(['name', 'id'], args) + + @mock.patch.object(db_api.utils, 'paginate_query') + @mock.patch.object(db_api, 'model_query') + def test_paginate_query_gets_model_marker(self, mock_query, + mock_paginate_query): + query = mock.Mock() + model = mock.Mock() + marker = mock.Mock() + + mock_query_object = mock.Mock() + mock_query_object.get.return_value = 'real_marker' + mock_query.return_value = mock_query_object + + db_api._paginate_query(self.ctx, query, model, marker=marker) + mock_query_object.get.assert_called_once_with(marker) + args, _ = mock_paginate_query.call_args + self.assertIn('real_marker', args) + + @mock.patch.object(db_api.utils, 'paginate_query') + def test_paginate_query_raises_invalid_sort_key(self, mock_paginate_query): + query = mock.Mock() + model = mock.Mock() + + mock_paginate_query.side_effect = db_api.utils.InvalidSortKey() + self.assertRaises(exception.Invalid, db_api._paginate_query, + self.ctx, query, model, sort_keys=['foo']) + + def test_filter_sort_keys_returns_empty_list_if_no_keys(self): + sort_keys = None + whitelist = None + + filtered_keys = db_api._filter_sort_keys(sort_keys, whitelist) + self.assertEqual([], filtered_keys) + + def test_filter_sort_keys_whitelists_single_key(self): + sort_key = 'foo' + whitelist = ['foo'] + + filtered_keys = db_api._filter_sort_keys(sort_key, whitelist) + self.assertEqual(['foo'], filtered_keys) + + def test_filter_sort_keys_whitelists_multiple_keys(self): + sort_keys = ['foo', 'bar', 'nope'] + whitelist = ['foo', 'bar'] + + filtered_keys = db_api._filter_sort_keys(sort_keys, whitelist) + self.assertIn('foo', filtered_keys) + self.assertIn('bar', filtered_keys) + self.assertNotIn('nope', filtered_keys) + def test_encryption(self): stack_name = 'test_encryption' (t, stack) = self._setup_test_stack(stack_name)