openstack/deb-heat
Code Issues Proposed changes

Utility function for int param checking

Browse Source 
This patch adds a utility function for checking the int type parameters
passed to the API layer. A test case is included for this utility
function. To exemplify how to use it, the events controller is revised
to make use of this function. The plan is to add this checking to all
int type parameters at API layer.

Change-Id: If4a1e2e5e7adbc272e2cfa5b1918cdf733926013
This commit is contained in:
tengqm 2015-03-03 11:06:30 +08:00
parent 7feaa881eb
commit 157a11550f
5 changed files with 103 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
heat/api/openstack/v1/events.py
View File

@ -18,6 +18,7 @@ from webob import exc
from heat.api.openstack.v1 import util
from heat.common.i18n import _
from heat.common import identifier
from heat.common import param_utils
from heat.common import serializers
from heat.common import wsgi
from heat.rpc import api as rpc_api
@ -117,6 +118,11 @@ class EventController(object):
if not filter_params:
filter_params = None
key = rpc_api.PARAM_LIMIT
if key in params:
limit = param_utils.extract_int(key, params[key], allow_zero=True)
params[key] = limit
if resource_name is None:
events = self._event_list(req, identity,
filters=filter_params, **params)

28
heat/common/param_utils.py
View File

@ -25,3 +25,31 @@ def extract_bool(subject):
raise ValueError(_('Unrecognized value "%(value)s", acceptable '
'values are: true, false.') % {'value': subject})
return strutils.bool_from_string(subject, strict=True)
def extract_int(name, value, allow_zero=True, allow_negative=False):
if value is None:
return None
if not strutils.is_int_like(value):
raise ValueError(_("Only integer is acceptable by "
"'%(name)s'.") % {'name': name})
if value in ('0', 0):
if allow_zero:
return int(value)
raise ValueError(_("Only non-zero integer is acceptable by "
"'%(name)s'.") % {'name': name})
try:
result = int(value)
except (TypeError, ValueError):
raise ValueError(_("Value '%(value)s' is invalid for '%(name)s' "
"which only accepts integer.") %
{'name': name, 'value': value})
if allow_negative is False and result < 0:
raise ValueError(_("Value '%(value)s' is invalid for '%(name)s' "
"which only accepts non-negative integer.") %
{'name': name, 'value': value})
return result

4
heat/rpc/api.py
View File

@ -16,11 +16,11 @@ ENGINE_TOPIC = 'engine'
PARAM_KEYS = (
PARAM_TIMEOUT, PARAM_DISABLE_ROLLBACK, PARAM_ADOPT_STACK_DATA,
PARAM_SHOW_DELETED, PARAM_SHOW_NESTED, PARAM_EXISTING,
PARAM_CLEAR_PARAMETERS, PARAM_GLOBAL_TENANT,
PARAM_CLEAR_PARAMETERS, PARAM_GLOBAL_TENANT, PARAM_LIMIT,
) = (
'timeout_mins', 'disable_rollback', 'adopt_stack_data',
'show_deleted', 'show_nested', 'existing',
'clear_parameters', 'global_tenant'
'clear_parameters', 'global_tenant', 'limit',
)
STACK_KEYS = (

4
heat/tests/test_api_openstack_v1.py
View File

@ -2835,7 +2835,7 @@ class EventControllerTest(ControllerTest, common.HeatTestCase):
def test_index_whitelists_pagination_params(self, mock_call, mock_enforce):
self._mock_enforce_setup(mock_enforce, 'index', True)
params = {
'limit': 'fake limit',
'limit': 10,
'sort_keys': 'fake sort keys',
'marker': 'fake marker',
'sort_dir': 'fake sort dir',
@ -2857,7 +2857,7 @@ class EventControllerTest(ControllerTest, common.HeatTestCase):
engine_args = rpc_call_args[1][1]
self.assertEqual(6, len(engine_args))
self.assertIn('limit', engine_args)
self.assertEqual('fake limit', engine_args['limit'])
self.assertEqual(10, engine_args['limit'])
self.assertIn('sort_keys', engine_args)
self.assertEqual(['fake sort keys'], engine_args['sort_keys'])
self.assertIn('marker', engine_args)

65
heat/tests/test_common_param_utils.py
View File

@ -23,3 +23,68 @@ class TestExtractBool(common.HeatTestCase):
self.assertFalse(param_utils.extract_bool(value))
for value in ('foo', 't', 'f', 'yes', 'no', 'y', 'n', '1', '0', None):
self.assertRaises(ValueError, param_utils.extract_bool, value)
class TestExtractInt(common.HeatTestCase):
def test_extract_int(self):
# None case
self.assertIsNone(param_utils.extract_int('num', None))
# 0 case
self.assertEqual(0, param_utils.extract_int('num', 0))
self.assertEqual(0, param_utils.extract_int('num', 0, allow_zero=True))
self.assertEqual(0, param_utils.extract_int('num', '0'))
self.assertEqual(0, param_utils.extract_int('num', '0',
allow_zero=True))
self.assertRaises(ValueError,
param_utils.extract_int,
'num', 0, allow_zero=False)
self.assertRaises(ValueError,
param_utils.extract_int,
'num', '0', allow_zero=False)
# positive values
self.assertEqual(1, param_utils.extract_int('num', 1))
self.assertEqual(1, param_utils.extract_int('num', '1'))
self.assertRaises(ValueError, param_utils.extract_int, 'num', '1.1')
self.assertRaises(ValueError, param_utils.extract_int, 'num', 1.1)
# negative values
self.assertEqual(-1, param_utils.extract_int('num', -1,
allow_negative=True))
self.assertEqual(-1, param_utils.extract_int('num', '-1',
allow_negative=True))
self.assertRaises(ValueError,
param_utils.extract_int, 'num', '-1.1',
allow_negative=True)
self.assertRaises(ValueError,
param_utils.extract_int, 'num', -1.1,
allow_negative=True)
self.assertRaises(ValueError, param_utils.extract_int, 'num', -1)
self.assertRaises(ValueError, param_utils.extract_int, 'num', '-1')
self.assertRaises(ValueError, param_utils.extract_int, 'num', '-1.1')
self.assertRaises(ValueError, param_utils.extract_int, 'num', -1.1)
self.assertRaises(ValueError,
param_utils.extract_int, 'num', -1,
allow_negative=False)
self.assertRaises(ValueError,
param_utils.extract_int, 'num', '-1',
allow_negative=False)
self.assertRaises(ValueError,
param_utils.extract_int, 'num', '-1.1',
allow_negative=False)
self.assertRaises(ValueError,
param_utils.extract_int, 'num', -1.1,
allow_negative=False)
# Non-int value
self.assertRaises(ValueError,
param_utils.extract_int, 'num', 'abc')
self.assertRaises(ValueError,
param_utils.extract_int, 'num', '')
self.assertRaises(ValueError,
param_utils.extract_int, 'num', 'true')
self.assertRaises(ValueError,
param_utils.extract_int, 'num', True)