Check if the alarm has time constraints field before displaying

Fixes the bug that broke the alarms CLI if the alarm didn't have
a time constraints field.

Reduces code duplication of alarm printing code, so now all the alarm
printing code is actually tested.

Renames some auxiliary methods for more clarity.

Change-Id: Ib691b4a5a6cf5ae133cd0a5576f90e4d0d189a92
Closes-bug: #1316390

2 files changed, 64 insertions, 28 deletions

diff --git a/ceilometerclient/tests/v2/test_shell.py b/ceilometerclient/tests/v2/test_shell.py
index 69e6a50..ea362aa 100644
--- a/ceilometerclient/tests/v2/test_shell.py
+++ b/ceilometerclient/tests/v2/test_shell.py
@@ -516,6 +516,8 @@ class ShellQueryAlarmsCommandTest(utils.BaseTestCase):
               "state_timestamp": "2014-02-20T10:37:15.589860",
               "threshold_rule": None,
               "timestamp": "2014-02-20T10:37:15.589856",
+              "time_constraints": [{"name": "test", "start": "0 23 * * *",
+                                    "duration": 10800}],
               "type": "combination",
               "user_id": "c96c887c216949acbdfbd8b494863567"}]
 
@@ -543,19 +545,52 @@ class ShellQueryAlarmsCommandTest(utils.BaseTestCase):
         self.assertEqual('''\
 +--------------------------------------+------------------+-------+---------\
 +------------+--------------------------------------------------------------\
-----------------------------------------+
+----------------------------------------+--------------------------------+
 | Alarm ID                             | Name             | State | Enabled \
 | Continuous | Alarm condition                                              \
-                                        |
+                                        | Time constraints               |
 +--------------------------------------+------------------+-------+---------\
 +------------+--------------------------------------------------------------\
-----------------------------------------+
+----------------------------------------+--------------------------------+
 | 768ff714-8cfb-4db9-9753-d484cb33a1cc | SwiftObjectAlarm | ok    | True    \
 | False      | combinated states (OR) of 739e99cb-c2ec-4718-b900-332502355f3\
-8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 |
+8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 | test at 0 23 * * *  for 10800s |
 +--------------------------------------+------------------+-------+---------\
 +------------+--------------------------------------------------------------\
-----------------------------------------+
+----------------------------------------+--------------------------------+
+''', sys.stdout.getvalue())
+
+    @mock.patch('sys.stdout', new=six.StringIO())
+    def test_time_constraints_compatibility(self):
+        # client should be backwards compatible
+        alarm_without_tc = dict(self.ALARM[0])
+        del alarm_without_tc['time_constraints']
+
+        # NOTE(nsaje): Since we're accessing a nonexisting key in the resource,
+        # the resource is looking it up with the manager (which is a mock).
+        manager_mock = mock.Mock()
+        del manager_mock.get
+        ret_alarm = [alarms.Alarm(manager_mock, alarm_without_tc)]
+        self.cc.query_alarms.query.return_value = ret_alarm
+
+        ceilometer_shell.do_query_alarms(self.cc, self.args)
+
+        self.assertEqual('''\
++--------------------------------------+------------------+-------+---------\
++------------+--------------------------------------------------------------\
+----------------------------------------+------------------+
+| Alarm ID                             | Name             | State | Enabled \
+| Continuous | Alarm condition                                              \
+                                        | Time constraints |
++--------------------------------------+------------------+-------+---------\
++------------+--------------------------------------------------------------\
+----------------------------------------+------------------+
+| 768ff714-8cfb-4db9-9753-d484cb33a1cc | SwiftObjectAlarm | ok    | True    \
+| False      | combinated states (OR) of 739e99cb-c2ec-4718-b900-332502355f3\
+8, 153462d0-a9b8-4b5b-8175-9e4b05e9b856 | None             |
++--------------------------------------+------------------+-------+---------\
++------------+--------------------------------------------------------------\
+----------------------------------------+------------------+
 ''', sys.stdout.getvalue())
 
 
diff --git a/ceilometerclient/v2/shell.py b/ceilometerclient/v2/shell.py
index b13ab20..d0d2f75 100644
--- a/ceilometerclient/v2/shell.py
+++ b/ceilometerclient/v2/shell.py
@@ -186,6 +186,20 @@ def do_meter_list(cc, args={}):
                      sortby=0)
 
 
+def _display_alarm_list(alarms, sortby=None):
+    # omit action initially to keep output width sane
+    # (can switch over to vertical formatting when available from CLIFF)
+    field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous',
+                    'Alarm condition', 'Time constraints']
+    fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions',
+              'rule', 'time_constraints']
+    utils.print_list(
+        alarms, fields, field_labels,
+        formatters={'rule': alarm_rule_formatter,
+                    'time_constraints': time_constraints_formatter_brief},
+        sortby=sortby)
+
+
 def _display_rule(type, rule):
     if type == 'threshold':
         return ('%(meter_name)s %(comparison_operator)s '
@@ -212,7 +226,7 @@ def alarm_rule_formatter(alarm):
     return _display_rule(alarm.type, alarm.rule)
 
 
-def _display_time_constraints(time_constraints):
+def _display_time_constraints_brief(time_constraints):
     if time_constraints:
         return ', '.join('%(name)s at %(start)s %(timezone)s for %(duration)ss'
                          % {
@@ -226,8 +240,10 @@ def _display_time_constraints(time_constraints):
         return 'None'
 
 
-def time_constraints_formatter(alarm):
-    return _display_time_constraints(alarm.time_constraints)
+def time_constraints_formatter_brief(alarm):
+    return _display_time_constraints_brief(getattr(alarm,
+                                                   'time_constraints',
+                                                   None))
 
 
 def _infer_type(detail):
@@ -255,7 +271,7 @@ def alarm_change_detail_formatter(change):
                 fields.append('%s: %s' % (k, detail[k]))
         if 'time_constraints' in detail:
             fields.append('time_constraints: %s' %
-                          _display_time_constraints(
+                          _display_time_constraints_brief(
                               detail['time_constraints']))
     elif change.type == 'rule change':
         for k, v in six.iteritems(detail):
@@ -273,16 +289,7 @@ def alarm_change_detail_formatter(change):
 def do_alarm_list(cc, args={}):
     '''List the user's alarms.'''
     alarms = cc.alarms.list(q=options.cli_to_array(args.query))
-    # omit action initially to keep output width sane
-    # (can switch over to vertical formatting when available from CLIFF)
-    field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous',
-                    'Alarm condition', 'Time constraints']
-    fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions',
-              'rule', 'time_constraints']
-    utils.print_list(
-        alarms, fields, field_labels,
-        formatters={'rule': alarm_rule_formatter,
-                    'time_constraints': time_constraints_formatter}, sortby=0)
+    _display_alarm_list(alarms, sortby=0)
 
 
 def alarm_query_formater(alarm):
@@ -293,7 +300,7 @@ def alarm_query_formater(alarm):
     return r' AND\n'.join(qs)
 
 
-def alarm_time_constraints_formatter(alarm):
+def time_constraints_formatter_full(alarm):
     time_constraints = []
     for tc in alarm.time_constraints:
         lines = []
@@ -314,7 +321,7 @@ def _display_alarm(alarm):
     if alarm.type == 'threshold':
         data['query'] = alarm_query_formater(alarm)
     if alarm.time_constraints:
-        data['time_constraints'] = alarm_time_constraints_formatter(alarm)
+        data['time_constraints'] = time_constraints_formatter_full(alarm)
     utils.print_dict(data, wrap=72)
 
 
@@ -793,13 +800,7 @@ def do_query_alarms(cc, args):
     except exc.HTTPNotFound:
         raise exc.CommandError('Alarms not found')
     else:
-        field_labels = ['Alarm ID', 'Name', 'State', 'Enabled', 'Continuous',
-                        'Alarm condition']
-        fields = ['alarm_id', 'name', 'state', 'enabled', 'repeat_actions',
-                  'rule']
-        utils.print_list(alarms, fields, field_labels,
-                         formatters={'rule': alarm_rule_formatter},
-                         sortby=None)
+        _display_alarm_list(alarms, sortby=None)
 
 
 @utils.arg('-f', '--filter', metavar='<FILTER>',