openstack
/
heat
Code Issues Proposed changes

Fix error count for stack-list while show deleted 

1.The count for stack-list is incorrect while show deleted.
  The count should include the deleted ones.
2.We should check the value of 'with_count', if the value
  is invalid, we should raise ValueError.

Change-Id: Ie1eef7a7ec63870ac28929bcadcd917bbc8d3203
Closes-bug: #1331383
This commit is contained in:
huangtianhua 2014-06-19 10:04:46 +08:00
parent 5cd7a547a4
commit 7434a93482
10 changed files with 78 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
heat/api/openstack/v1/stacks.py
View File

@ -167,9 +167,16 @@ class StackController(object):
params = util.get_allowed_params(req.params, whitelist)
filter_params = util.get_allowed_params(req.params, filter_whitelist)
show_deleted = False
if engine_api.PARAM_SHOW_DELETED in params:
params[engine_api.PARAM_SHOW_DELETED] = param_utils.extract_bool(
params[engine_api.PARAM_SHOW_DELETED])
show_deleted = params[engine_api.PARAM_SHOW_DELETED]
# get the with_count value, if invalid, raise ValueError
with_count = False
if req.params.get('with_count'):
with_count = param_utils.extract_bool(
req.params.get('with_count'))
if not filter_params:
filter_params = None
@ -180,13 +187,14 @@ class StackController(object):
**params)
count = None
if req.params.get('with_count'):
if with_count:
try:
# Check if engine has been updated to a version with
# support to count_stacks before trying to use it.
count = self.rpc_client.count_stacks(req.context,
filters=filter_params,
tenant_safe=tenant_safe)
tenant_safe=tenant_safe,
show_deleted=show_deleted)
except AttributeError as exc:
LOG.warning(_("Old Engine Version: %s") % exc)

4
heat/common/param_utils.py
View File

@ -21,6 +21,6 @@ def extract_bool(subject):
regardless of case.
'''
if str(subject).lower() not in ('true', 'false'):
raise ValueError(_('Unrecognized value "%(value)s, acceptable values '
'are: true, false.') % {'value': subject})
raise ValueError(_('Unrecognized value "%(value)s", acceptable '
'values are: true, false.') % {'value': subject})
return strutils.bool_from_string(subject, strict=True)

6
heat/db/api.py
View File

@ -138,9 +138,11 @@ def stack_get_all_by_owner_id(context, owner_id):
return IMPL.stack_get_all_by_owner_id(context, owner_id)
def stack_count_all(context, filters=None, tenant_safe=True):
def stack_count_all(context, filters=None, tenant_safe=True,
show_deleted=False):
return IMPL.stack_count_all(context, filters=filters,
tenant_safe=tenant_safe)
tenant_safe=tenant_safe,
show_deleted=show_deleted)
def stack_create(context, values):

6
heat/db/sqlalchemy/api.py
View File

@ -393,8 +393,10 @@ def _filter_and_page_query(context, query, limit=None, sort_keys=None,
whitelisted_sort_keys, marker, sort_dir)
def stack_count_all(context, filters=None, tenant_safe=True):
query = _query_stack_get_all(context, tenant_safe=tenant_safe)
def stack_count_all(context, filters=None, tenant_safe=True,
show_deleted=False):
query = _query_stack_get_all(context, tenant_safe=tenant_safe,
show_deleted=show_deleted)
query = db_filters.exact_filter(query, models.Stack, filters)
return query.count()

8
heat/engine/service.py
View File

@ -458,15 +458,19 @@ class EngineService(service.Service):
return [api.format_stack(stack) for stack in stacks]
@request_context
def count_stacks(self, cnxt, filters=None, tenant_safe=True):
def count_stacks(self, cnxt, filters=None, tenant_safe=True,
show_deleted=False):
"""
Return the number of stacks that match the given filters
:param ctxt: RPC context.
:param filters: a dict of ATTR:VALUE to match against stacks
:param tenant_safe: if true, scope the request by the current tenant
:param show_deleted: if true, count will include the deleted stacks
:returns: a integer representing the number of matched stacks
"""
return db_api.stack_count_all(cnxt, filters=filters,
tenant_safe=tenant_safe)
tenant_safe=tenant_safe,
show_deleted=show_deleted)
def _validate_deferred_auth_context(self, cnxt, stack):
if cfg.CONF.deferred_auth_method != 'password':

7
heat/rpc/client.py
View File

@ -94,17 +94,20 @@ class EngineClient(object):
tenant_safe=tenant_safe,
show_deleted=show_deleted))
def count_stacks(self, ctxt, filters=None, tenant_safe=True):
def count_stacks(self, ctxt, filters=None, tenant_safe=True,
show_deleted=False):
"""
Return the number of stacks that match the given filters
:param ctxt: RPC context.
:param filters: a dict of ATTR:VALUE to match against stacks
:param tenant_safe: if true, scope the request by the current tenant
:param show_deleted: if true, count will include the deleted stacks
:returns: a integer representing the number of matched stacks
"""
return self.call(ctxt, self.make_msg('count_stacks',
filters=filters,
tenant_safe=tenant_safe))
tenant_safe=tenant_safe,
show_deleted=show_deleted))
def show_stack(self, ctxt, stack_identity):
"""

37
heat/tests/test_api_openstack_v1.py
View File

@ -16,6 +16,7 @@ import json
import mock
from oslo.config import cfg
from oslo.messaging._drivers import common as rpc_common
import six
import webob.exc
import heat.api.middleware.fault as fault
@ -441,10 +442,10 @@ class StackControllerTest(ControllerTest, HeatTestCase):
result = self.controller.index(req, tenant_id=self.tenant)
self.assertEqual(0, result['count'])
def test_index_doesnt_return_stack_count_if_with_count_is_falsy(
def test_index_doesnt_return_stack_count_if_with_count_is_false(
self, mock_enforce):
self._mock_enforce_setup(mock_enforce, 'index', True)
params = {'with_count': ''}
params = {'with_count': 'false'}
req = self._get('/stacks', params=params)
engine = self.controller.rpc_client
@ -455,11 +456,22 @@ class StackControllerTest(ControllerTest, HeatTestCase):
self.assertNotIn('count', result)
assert not engine.count_stacks.called
def test_index_with_count_is_invalid(self, mock_enforce):
self._mock_enforce_setup(mock_enforce, 'index', True)
params = {'with_count': 'invalid_value'}
req = self._get('/stacks', params=params)
exc = self.assertRaises(ValueError, self.controller.index,
req, tenant_id=self.tenant)
excepted = ('Unrecognized value "invalid_value", '
'acceptable values are: true, false')
self.assertIn(excepted, six.text_type(exc))
@mock.patch.object(rpc_client.EngineClient, 'count_stacks')
def test_index_doesnt_break_with_old_engine(self, mock_count_stacks,
mock_enforce):
self._mock_enforce_setup(mock_enforce, 'index', True)
params = {'with_count': 'Truthy'}
params = {'with_count': 'True'}
req = self._get('/stacks', params=params)
engine = self.controller.rpc_client
@ -520,6 +532,25 @@ class StackControllerTest(ControllerTest, HeatTestCase):
tenant_safe=True,
show_deleted=True)
def test_index_show_deleted_True_with_count_True(self, mock_enforce):
rpc_client = self.controller.rpc_client
rpc_client.list_stacks = mock.Mock(return_value=[])
rpc_client.count_stacks = mock.Mock(return_value=0)
params = {'show_deleted': 'True',
'with_count': 'True'}
req = self._get('/stacks', params=params)
result = self.controller.index(req, tenant_id=self.tenant)
self.assertEqual(0, result['count'])
rpc_client.list_stacks.assert_called_once_with(mock.ANY,
filters=mock.ANY,
tenant_safe=True,
show_deleted=True)
rpc_client.count_stacks.assert_called_once_with(mock.ANY,
filters=mock.ANY,
tenant_safe=True,
show_deleted=True)
@mock.patch.object(rpc_client.EngineClient, 'call')
def test_detail(self, mock_call, mock_enforce):
self._mock_enforce_setup(mock_enforce, 'detail', True)

9
heat/tests/test_engine_service.py
View File

@ -1768,21 +1768,24 @@ class StackServiceTest(HeatTestCase):
self.eng.count_stacks(self.ctx, filters={'foo': 'bar'})
mock_stack_count_all.assert_called_once_with(mock.ANY,
filters={'foo': 'bar'},
tenant_safe=mock.ANY)
tenant_safe=mock.ANY,
show_deleted=False)
@mock.patch.object(db_api, 'stack_count_all')
def test_count_stacks_tenant_safe_default_true(self, mock_stack_count_all):
self.eng.count_stacks(self.ctx)
mock_stack_count_all.assert_called_once_with(mock.ANY,
filters=mock.ANY,
tenant_safe=True)
tenant_safe=True,
show_deleted=False)
@mock.patch.object(db_api, 'stack_count_all')
def test_count_stacks_passes_tenant_safe_info(self, mock_stack_count_all):
self.eng.count_stacks(self.ctx, tenant_safe=False)
mock_stack_count_all.assert_called_once_with(mock.ANY,
filters=mock.ANY,
tenant_safe=False)
tenant_safe=False,
show_deleted=False)
@stack_context('service_abandon_stack')
def test_abandon_stack(self):

1
heat/tests/test_rpc_client.py
View File

@ -89,6 +89,7 @@ class EngineRpcAPITestCase(testtools.TestCase):
default_args = {
'filters': mock.ANY,
'tenant_safe': mock.ANY,
'show_deleted': mock.ANY,
}
self._test_engine_api('count_stacks', 'call', **default_args)

6
heat/tests/test_sqlalchemy_api.py
View File

@ -510,10 +510,16 @@ class SqlAlchemyTest(HeatTestCase):
stacks[0].delete()
st_db = db_api.stack_count_all(self.ctx)
self.assertEqual(2, st_db)
# show deleted
st_db = db_api.stack_count_all(self.ctx, show_deleted=True)
self.assertEqual(3, st_db)
stacks[1].delete()
st_db = db_api.stack_count_all(self.ctx)
self.assertEqual(1, st_db)
# show deleted
st_db = db_api.stack_count_all(self.ctx, show_deleted=True)
self.assertEqual(3, st_db)
def test_stack_count_all_with_filters(self):
self._setup_test_stack('foo', UUID1)