Merge "Avoid empty entity field in uri path"

2 files changed, 105 insertions, 15 deletions

diff --git a/ceilometerclient/tests/v2/test_shell.py b/ceilometerclient/tests/v2/test_shell.py
index 96ab38f..4bcc1f4 100644
--- a/ceilometerclient/tests/v2/test_shell.py
+++ b/ceilometerclient/tests/v2/test_shell.py
@@ -22,6 +22,7 @@ import mock
 import six
 from testtools import matchers
 
+from ceilometerclient import exc
 from ceilometerclient import shell as base_shell
 from ceilometerclient.tests import utils
 from ceilometerclient.v2 import alarms
@@ -763,3 +764,83 @@ class ShellStatisticsTest(utils.BaseTestCase):
                 fields,
                 [self.displays.get(f, f) for f in fields],
             )
+
+
+class ShellEmptyIdTest(utils.BaseTestCase):
+    """Test empty field which will cause calling incorrect rest uri."""
+
+    def _test_entity_action_with_empty_values(self, entity, *args):
+        for value in ('', ' ', '   ', '\t'):
+            self._test_entity_action_with_empty_value(entity, value, *args)
+
+    def _test_entity_action_with_empty_value(self, entity, value, *args):
+        shell = base_shell.CeilometerShell()
+        argv = list(args) + ['--%s' % entity, value]
+
+        with mock.patch('ceilometerclient.exc.CommandError') as e:
+            e.return_value = exc.BaseException()
+            self.assertRaises(exc.BaseException, shell.parse_args, argv)
+            entity = entity.replace('-', '_')
+            e.assert_called_with('%s should not be empty' % entity)
+
+    def _test_alarm_action_with_empty_ids(self, method, *args):
+        args = [method] + list(args)
+        self._test_entity_action_with_empty_values('alarm_id', *args)
+
+    def test_alarm_show_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-show')
+
+    def test_alarm_update_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-update')
+
+    def test_alarm_threshold_update_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-threshold-update')
+
+    def test_alarm_combination_update_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-combination-update')
+
+    def test_alarm_delete_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-delete')
+
+    def test_alarm_state_get_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-state-get')
+
+    def test_alarm_state_set_with_empty_id(self):
+        args = ['alarm-state-set', '--state', 'ok']
+        self._test_alarm_action_with_empty_ids(*args)
+
+    def test_alarm_history_with_empty_id(self):
+        self._test_alarm_action_with_empty_ids('alarm-history')
+
+    def test_event_show_with_empty_message_id(self):
+        args = ['event-show']
+        self._test_entity_action_with_empty_values('message_id', *args)
+
+    def test_resource_show_with_empty_id(self):
+        args = ['resource-show']
+        self._test_entity_action_with_empty_values('resource_id', *args)
+
+    def test_sample_list_with_empty_meter(self):
+        args = ['sample-list']
+        self._test_entity_action_with_empty_values('meter', *args)
+
+    def test_sample_create_with_empty_meter(self):
+        args = ['sample-create', '-r', 'x', '--meter-type', 'gauge',
+                '--meter-unit', 'B', '--sample-volume', '1']
+        self._test_entity_action_with_empty_values('meter-name', *args)
+
+    def test_statistics_with_empty_meter(self):
+        args = ['statistics']
+        self._test_entity_action_with_empty_values('meter', *args)
+
+    def test_trait_description_list_with_empty_event_type(self):
+        args = ['trait-description-list']
+        self._test_entity_action_with_empty_values('event_type', *args)
+
+    def test_trait_list_with_empty_event_type(self):
+        args = ['trait-list', '--trait_name', 'x']
+        self._test_entity_action_with_empty_values('event_type', *args)
+
+    def test_trait_list_with_empty_trait_name(self):
+        args = ['trait-list', '--event_type', 'x']
+        self._test_entity_action_with_empty_values('trait_name', *args)
diff --git a/ceilometerclient/v2/shell.py b/ceilometerclient/v2/shell.py
index b13ab20..2565826 100644
--- a/ceilometerclient/v2/shell.py
+++ b/ceilometerclient/v2/shell.py
@@ -18,6 +18,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import argparse
 import functools
 import json
 
@@ -48,11 +49,18 @@ COMPLEX_OPERATORS = ['and', 'or']
 SIMPLE_OPERATORS = ["=", "!=", "<", "<=", '>', '>=']
 
 
+class NotEmptyAction(argparse.Action):
+    def __call__(self, parser, namespace, values, option_string=None):
+        if not values or values.isspace():
+            raise exc.CommandError('%s should not be empty' % self.dest)
+        setattr(namespace, self.dest, values)
+
+
 @utils.arg('-q', '--query', metavar='<QUERY>',
            help='key[op]data_type::value; list. data_type is optional, '
                 'but if supplied must be string, integer, float, or boolean.')
 @utils.arg('-m', '--meter', metavar='<NAME>', required=True,
-           help='Name of meter to show samples for.')
+           action=NotEmptyAction, help='Name of meter to show samples for.')
 @utils.arg('-p', '--period', metavar='<PERIOD>',
            help='Period in seconds over which to group samples.')
 @utils.arg('-g', '--groupby', metavar='<FIELD>', action='append',
@@ -106,7 +114,7 @@ def do_statistics(cc, args):
            help='key[op]data_type::value; list. data_type is optional, '
                 'but if supplied must be string, integer, float, or boolean.')
 @utils.arg('-m', '--meter', metavar='<NAME>', required=True,
-           help='Name of meter to show samples for.')
+           action=NotEmptyAction, help='Name of meter to show samples for.')
 @utils.arg('-l', '--limit', metavar='<NUMBER>',
            help='Maximum number of samples to return.')
 def do_sample_list(cc, args):
@@ -136,7 +144,7 @@ def do_sample_list(cc, args):
 @utils.arg('-r', '--resource-id', metavar='<RESOURCE_ID>', required=True,
            help='ID of the resource.')
 @utils.arg('-m', '--meter-name', metavar='<METER_NAME>', required=True,
-           help='The meter name.')
+           action=NotEmptyAction, help='The meter name.')
 @utils.arg('--meter-type', metavar='<METER_TYPE>', required=True,
            help='The meter type.')
 @utils.arg('--meter-unit', metavar='<METER_UNIT>', required=True,
@@ -319,7 +327,7 @@ def _display_alarm(alarm):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm to show.')
+           action=NotEmptyAction, help='ID of the alarm to show.')
 def do_alarm_show(cc, args={}):
     '''Show an alarm.'''
     alarm = cc.alarms.get(args.alarm_id)
@@ -476,7 +484,7 @@ def do_alarm_combination_create(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm to update.')
+           action=NotEmptyAction, help='ID of the alarm to update.')
 @common_alarm_arguments()
 @utils.arg('--remove-time-constraint', action='append',
            metavar='<Constraint names>',
@@ -517,7 +525,7 @@ def do_alarm_update(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm to update.')
+           action=NotEmptyAction, help='ID of the alarm to update.')
 @common_alarm_arguments()
 @utils.arg('--remove-time-constraint', action='append',
            metavar='<Constraint names>',
@@ -569,7 +577,7 @@ def do_alarm_threshold_update(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm to update.')
+           action=NotEmptyAction, help='ID of the alarm to update.')
 @common_alarm_arguments()
 @utils.arg('--remove-time-constraint', action='append',
            metavar='<Constraint names>',
@@ -601,7 +609,7 @@ def do_alarm_combination_update(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm to delete.')
+           action=NotEmptyAction, help='ID of the alarm to delete.')
 def do_alarm_delete(cc, args={}):
     '''Delete an alarm.'''
     try:
@@ -611,7 +619,7 @@ def do_alarm_delete(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm state to set.')
+           action=NotEmptyAction, help='ID of the alarm state to set.')
 @utils.arg('--state', metavar='<STATE>', required=True,
            help='State of the alarm, one of: ' + str(ALARM_STATES) +
            '.')
@@ -625,7 +633,7 @@ def do_alarm_state_set(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
-           help='ID of the alarm state to show.')
+           action=NotEmptyAction, help='ID of the alarm state to show.')
 def do_alarm_state_get(cc, args={}):
     '''Get the state of an alarm.'''
     try:
@@ -636,6 +644,7 @@ def do_alarm_state_get(cc, args={}):
 
 
 @utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True,
+           action=NotEmptyAction,
            help='ID of the alarm for which history is shown.')
 @utils.arg('-q', '--query', metavar='<QUERY>',
            help='key[op]data_type::value; list. data_type is optional, '
@@ -673,7 +682,7 @@ def do_resource_list(cc, args={}):
 
 
 @utils.arg('-r', '--resource_id', metavar='<RESOURCE_ID>', required=True,
-           help='ID of the resource to show.')
+           action=NotEmptyAction, help='ID of the resource to show.')
 def do_resource_show(cc, args={}):
     '''Show the resource.'''
     try:
@@ -705,7 +714,7 @@ def do_event_list(cc, args={}):
 
 @utils.arg('-m', '--message_id', metavar='<message_id>',
            help='The ID of the event. Should be a UUID.',
-           required=True)
+           required=True, action=NotEmptyAction)
 def do_event_show(cc, args={}):
     '''Show a particular event.'''
     event = cc.events.get(args.message_id)
@@ -722,7 +731,7 @@ def do_event_type_list(cc, args={}):
 
 @utils.arg('-e', '--event_type', metavar='<EVENT_TYPE>',
            help='Type of the event for which traits will be shown.',
-           required=True)
+           required=True, action=NotEmptyAction)
 def do_trait_description_list(cc, args={}):
     '''List trait info for an event type.'''
     trait_descriptions = cc.trait_descriptions.list(args.event_type)
@@ -733,10 +742,10 @@ def do_trait_description_list(cc, args={}):
 
 @utils.arg('-e', '--event_type', metavar='<EVENT_TYPE>',
            help='Type of the event for which traits will listed.',
-           required=True)
+           required=True, action=NotEmptyAction)
 @utils.arg('-t', '--trait_name', metavar='<TRAIT_NAME>',
            help='The name of the trait to list.',
-           required=True)
+           required=True, action=NotEmptyAction)
 def do_trait_list(cc, args={}):
     '''List trait all traits with name <trait_name> for Event Type
     <event_type>.