Merge "Add sort support to image list"

6 files changed, 178 insertions, 0 deletions

diff --git a/openstackclient/common/utils.py b/openstackclient/common/utils.py
index 9ad3823c..01a40e74 100644
--- a/openstackclient/common/utils.py
+++ b/openstackclient/common/utils.py
@@ -122,6 +122,17 @@ def format_list(data):
     return ', '.join(sorted(data))
 
 
+def get_field(item, field):
+    try:
+        if isinstance(item, dict):
+            return item[field]
+        else:
+            return getattr(item, field)
+    except Exception:
+        msg = "Resource doesn't have field %s" % field
+        raise exceptions.CommandError(msg)
+
+
 def get_item_properties(item, fields, mixed_case_fields=[], formatters={}):
     """Return a tuple containing the item properties.
 
@@ -170,6 +181,35 @@ def get_dict_properties(item, fields, mixed_case_fields=[], formatters={}):
     return tuple(row)
 
 
+def sort_items(items, sort_str):
+    """Sort items based on sort keys and sort directions given by sort_str.
+
+    :param items: a list or generator object of items
+    :param sort_str: a string defining the sort rules, the format is
+    '<key1>:[direction1],<key2>:[direction2]...', direction can be 'asc'
+    for ascending or 'desc' for descending, if direction is not given,
+    it's ascending by default
+    :return: sorted items
+    """
+    if not sort_str:
+        return items
+    # items may be a generator object, transform it to a list
+    items = list(items)
+    sort_keys = sort_str.strip().split(',')
+    for sort_key in reversed(sort_keys):
+        reverse = False
+        if ':' in sort_key:
+            sort_key, direction = sort_key.split(':', 1)
+            if direction not in ['asc', 'desc']:
+                msg = "Specify sort direction by asc or desc"
+                raise exceptions.CommandError(msg)
+            if direction == 'desc':
+                reverse = True
+        items.sort(key=lambda item: get_field(item, sort_key),
+                   reverse=reverse)
+    return items
+
+
 def string_to_bool(arg):
     return arg.strip().lower() in ('t', 'true', 'yes', '1')
 
diff --git a/openstackclient/image/v1/image.py b/openstackclient/image/v1/image.py
index 2490d2a0..127a7735 100644
--- a/openstackclient/image/v1/image.py
+++ b/openstackclient/image/v1/image.py
@@ -355,6 +355,13 @@ class ListImage(lister.Lister):
             metavar="<size>",
             help=argparse.SUPPRESS,
         )
+        parser.add_argument(
+            '--sort',
+            metavar="<key>[:<direction>]",
+            help="Sort output by selected keys and directions(asc or desc) "
+                 "(default: asc), multiple keys and directions can be "
+                 "specified separated by comma",
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -409,6 +416,9 @@ class ListImage(lister.Lister):
                 value=value,
                 property_field='properties',
             )
+
+        data = utils.sort_items(data, parsed_args.sort)
+
         return (
             column_headers,
             (utils.get_dict_properties(
diff --git a/openstackclient/image/v2/image.py b/openstackclient/image/v2/image.py
index 4eda506c..afc99e85 100644
--- a/openstackclient/image/v2/image.py
+++ b/openstackclient/image/v2/image.py
@@ -105,6 +105,13 @@ class ListImage(lister.Lister):
             metavar="<size>",
             help=argparse.SUPPRESS,
         )
+        parser.add_argument(
+            '--sort',
+            metavar="<key>[:<direction>]",
+            help="Sort output by selected keys and directions(asc or desc) "
+                 "(default: asc), multiple keys and directions can be "
+                 "specified separated by comma",
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -160,6 +167,9 @@ class ListImage(lister.Lister):
                 value=value,
                 property_field='properties',
             )
+
+        data = utils.sort_items(data, parsed_args.sort)
+
         return (
             column_headers,
             (utils.get_dict_properties(
diff --git a/openstackclient/tests/common/test_utils.py b/openstackclient/tests/common/test_utils.py
index 583ab99c..cda0b135 100644
--- a/openstackclient/tests/common/test_utils.py
+++ b/openstackclient/tests/common/test_utils.py
@@ -58,6 +58,68 @@ class TestUtils(test_utils.TestCase):
                               utils.get_password,
                               mock_stdin)
 
+    def get_test_items(self):
+        item1 = {'a': 1, 'b': 2}
+        item2 = {'a': 1, 'b': 3}
+        item3 = {'a': 2, 'b': 2}
+        item4 = {'a': 2, 'b': 1}
+        return [item1, item2, item3, item4]
+
+    def test_sort_items_with_one_key(self):
+        items = self.get_test_items()
+        sort_str = 'b'
+        expect_items = [items[3], items[0], items[2], items[1]]
+        self.assertEqual(expect_items, utils.sort_items(items, sort_str))
+
+    def test_sort_items_with_multiple_keys(self):
+        items = self.get_test_items()
+        sort_str = 'a,b'
+        expect_items = [items[0], items[1], items[3], items[2]]
+        self.assertEqual(expect_items, utils.sort_items(items, sort_str))
+
+    def test_sort_items_all_with_direction(self):
+        items = self.get_test_items()
+        sort_str = 'a:desc,b:desc'
+        expect_items = [items[2], items[3], items[1], items[0]]
+        self.assertEqual(expect_items, utils.sort_items(items, sort_str))
+
+    def test_sort_items_some_with_direction(self):
+        items = self.get_test_items()
+        sort_str = 'a,b:desc'
+        expect_items = [items[1], items[0], items[2], items[3]]
+        self.assertEqual(expect_items, utils.sort_items(items, sort_str))
+
+    def test_sort_items_with_object(self):
+        item1 = mock.Mock(a=1, b=2)
+        item2 = mock.Mock(a=1, b=3)
+        item3 = mock.Mock(a=2, b=2)
+        item4 = mock.Mock(a=2, b=1)
+        items = [item1, item2, item3, item4]
+        sort_str = 'b,a'
+        expect_items = [item4, item1, item3, item2]
+        self.assertEqual(expect_items, utils.sort_items(items, sort_str))
+
+    def test_sort_items_with_empty_key(self):
+        items = self.get_test_items()
+        sort_srt = ''
+        self.assertEqual(items, utils.sort_items(items, sort_srt))
+        sort_srt = None
+        self.assertEqual(items, utils.sort_items(items, sort_srt))
+
+    def test_sort_items_with_invalid_key(self):
+        items = self.get_test_items()
+        sort_str = 'c'
+        self.assertRaises(exceptions.CommandError,
+                          utils.sort_items,
+                          items, sort_str)
+
+    def test_sort_items_with_invalid_direction(self):
+        items = self.get_test_items()
+        sort_str = 'a:bad_dir'
+        self.assertRaises(exceptions.CommandError,
+                          utils.sort_items,
+                          items, sort_str)
+
 
 class NoUniqueMatch(Exception):
     pass
diff --git a/openstackclient/tests/image/v1/test_image.py b/openstackclient/tests/image/v1/test_image.py
index 355f8c82..2776e744 100644
--- a/openstackclient/tests/image/v1/test_image.py
+++ b/openstackclient/tests/image/v1/test_image.py
@@ -470,6 +470,35 @@ class TestImageList(TestImage):
         ), )
         self.assertEqual(datalist, tuple(data))
 
+    @mock.patch('openstackclient.common.utils.sort_items')
+    def test_image_list_sort_option(self, si_mock):
+        si_mock.return_value = [
+            copy.deepcopy(image_fakes.IMAGE)
+        ]
+
+        arglist = ['--sort', 'name:asc']
+        verifylist = [('sort', 'name:asc')]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+        self.api_mock.image_list.assert_called_with(
+            detailed=False
+        )
+        si_mock.assert_called_with(
+            [image_fakes.IMAGE],
+            'name:asc'
+        )
+
+        collist = ('ID', 'Name')
+
+        self.assertEqual(collist, columns)
+        datalist = ((
+            image_fakes.image_id,
+            image_fakes.image_name
+        ), )
+        self.assertEqual(datalist, tuple(data))
+
 
 class TestImageSet(TestImage):
 
diff --git a/openstackclient/tests/image/v2/test_image.py b/openstackclient/tests/image/v2/test_image.py
index db3c32df..6a28b1ec 100644
--- a/openstackclient/tests/image/v2/test_image.py
+++ b/openstackclient/tests/image/v2/test_image.py
@@ -255,3 +255,30 @@ class TestImageList(TestImage):
             image_fakes.image_name,
         ), )
         self.assertEqual(datalist, tuple(data))
+
+    @mock.patch('openstackclient.common.utils.sort_items')
+    def test_image_list_sort_option(self, si_mock):
+        si_mock.return_value = [
+            copy.deepcopy(image_fakes.IMAGE)
+        ]
+
+        arglist = ['--sort', 'name:asc']
+        verifylist = [('sort', 'name:asc')]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # DisplayCommandBase.take_action() returns two tuples
+        columns, data = self.cmd.take_action(parsed_args)
+        self.api_mock.image_list.assert_called_with()
+        si_mock.assert_called_with(
+            [image_fakes.IMAGE],
+            'name:asc'
+        )
+
+        collist = ('ID', 'Name')
+
+        self.assertEqual(collist, columns)
+        datalist = ((
+            image_fakes.image_id,
+            image_fakes.image_name
+        ), )
+        self.assertEqual(datalist, tuple(data))