Merge "Extend images CLI v2 with new sorting syntax"

4 files changed, 140 insertions, 22 deletions

diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py
index 5f51539..c9fb0a7 100644
--- a/glanceclient/v2/images.py
+++ b/glanceclient/v2/images.py
@@ -47,6 +47,29 @@ class Controller(object):
             return [value]
         return value
 
+    @staticmethod
+    def _validate_sort_param(sort):
+        """Validates sorting argument for invalid keys and directions values.
+
+        :param sort: comma-separated list of sort keys with optional <:dir>
+        after each key
+        """
+        for sort_param in sort.strip().split(','):
+            key, _sep, dir = sort_param.partition(':')
+            if dir and dir not in SORT_DIR_VALUES:
+                msg = ('Invalid sort direction: %(sort_dir)s.'
+                       ' It must be one of the following: %(available)s.'
+                       ) % {'sort_dir': dir,
+                            'available': ', '.join(SORT_DIR_VALUES)}
+                raise exc.HTTPBadRequest(msg)
+            if key not in SORT_KEY_VALUES:
+                msg = ('Invalid sort key: %(sort_key)s.'
+                       ' It must be one of the following: %(available)s.'
+                       ) % {'sort_key': key,
+                            'available': ', '.join(SORT_KEY_VALUES)}
+                raise exc.HTTPBadRequest(msg)
+        return sort
+
     def list(self, **kwargs):
         """Retrieve a listing of Image objects
 
@@ -104,16 +127,6 @@ class Controller(object):
         # the page_size as Glance's limit.
         filters['limit'] = page_size
 
-        sort_dir = self._wrap(kwargs.get('sort_dir', []))
-        sort_key = self._wrap(kwargs.get('sort_key', []))
-
-        if len(sort_key) != len(sort_dir) and len(sort_dir) > 1:
-            raise exc.HTTPBadRequest("Unexpected number of sort directions: "
-                                     "provide only one default sorting "
-                                     "direction for each key or make sure "
-                                     "that sorting keys number matches with a "
-                                     "number of sorting directions.")
-
         tags = filters.pop('tag', [])
         tags_url_params = []
 
@@ -130,11 +143,27 @@ class Controller(object):
         for param in tags_url_params:
             url = '%s&%s' % (url, parse.urlencode(param))
 
-        for key in sort_key:
-            url = '%s&sort_key=%s' % (url, key)
-
-        for dir in sort_dir:
-            url = '%s&sort_dir=%s' % (url, dir)
+        if 'sort' in kwargs:
+            if 'sort_key' in kwargs or 'sort_dir' in kwargs:
+                raise exc.HTTPBadRequest("The 'sort' argument is not supported"
+                                         " with 'sort_key' or 'sort_dir'.")
+            url = '%s&sort=%s' % (url,
+                                  self._validate_sort_param(
+                                      kwargs['sort']))
+        else:
+            sort_dir = self._wrap(kwargs.get('sort_dir', []))
+            sort_key = self._wrap(kwargs.get('sort_key', []))
+
+            if len(sort_key) != len(sort_dir) and len(sort_dir) > 1:
+                raise exc.HTTPBadRequest(
+                    "Unexpected number of sort directions: "
+                    "either provide a single sort direction or an equal "
+                    "number of sort keys and sort directions.")
+            for key in sort_key:
+                url = '%s&sort_key=%s' % (url, key)
+
+            for dir in sort_dir:
+                url = '%s&sort_dir=%s' % (url, dir)
 
         for image in paginate(url, page_size, limit):
             yield image
diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py
index d45f3c7..2628919 100644
--- a/glanceclient/v2/shell.py
+++ b/glanceclient/v2/shell.py
@@ -131,6 +131,10 @@ def do_image_update(gc, args):
 @utils.arg('--sort-dir', default=[], action='append',
            choices=images.SORT_DIR_VALUES,
            help='Sort image list in specified directions.')
+@utils.arg('--sort', metavar='<key>[:<direction>]', default=None,
+           help=(("Comma-separated list of sort keys and directions in the "
+                  "form of <key>[:<asc|desc>]. Valid keys: %s. OPTIONAL: "
+                  "Default='name:asc'.") % ', '.join(images.SORT_KEY_VALUES)))
 def do_image_list(gc, args):
     """List images you can access."""
     filter_keys = ['visibility', 'member_status', 'owner', 'checksum', 'tag']
@@ -148,14 +152,15 @@ def do_image_list(gc, args):
         kwargs['limit'] = args.limit
     if args.page_size is not None:
         kwargs['page_size'] = args.page_size
+
     if args.sort_key:
         kwargs['sort_key'] = args.sort_key
-    else:
-        kwargs['sort_key'] = ['name']
     if args.sort_dir:
         kwargs['sort_dir'] = args.sort_dir
-    else:
-        kwargs['sort_dir'] = ['asc']
+    if args.sort is not None:
+        kwargs['sort'] = args.sort
+    elif not args.sort_dir and not args.sort_key:
+        kwargs['sort'] = 'name:asc'
 
     images = gc.images.list(**kwargs)
     columns = ['ID', 'Name']
diff --git a/tests/v2/test_images.py b/tests/v2/test_images.py
index 1082f26..00b06e3 100644
--- a/tests/v2/test_images.py
+++ b/tests/v2/test_images.py
@@ -473,6 +473,22 @@ data_fixtures = {
             ]},
         ),
     },
+    '/v2/images?limit=%d&sort=name%%3Adesc%%2Csize%%3Aasc'
+    % images.DEFAULT_PAGE_SIZE: {
+        'GET': (
+            {},
+            {'images': [
+                {
+                    'id': '6f99bf80-2ee6-47cf-acfe-1f1fabb7e810',
+                    'name': 'image-2',
+                },
+                {
+                    'id': '2a4560b2-e585-443e-9b39-553b46ec92d1',
+                    'name': 'image-1',
+                },
+            ]},
+        ),
+    },
 }
 
 schema_fixtures = {
@@ -679,6 +695,13 @@ class TestController(testtools.TestCase):
         self.assertEqual(2, len(images))
         self.assertEqual('%s' % img_id1, images[1].id)
 
+    def test_list_images_with_new_sorting_syntax(self):
+        img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1'
+        sort = 'name:desc,size:asc'
+        images = list(self.controller.list(sort=sort))
+        self.assertEqual(2, len(images))
+        self.assertEqual('%s' % img_id1, images[1].id)
+
     def test_list_images_sort_dirs_fewer_than_keys(self):
         sort_key = ['name', 'id', 'created_at']
         sort_dir = ['desc', 'asc']
@@ -688,6 +711,31 @@ class TestController(testtools.TestCase):
                               sort_key=sort_key,
                               sort_dir=sort_dir))
 
+    def test_list_images_combined_syntax(self):
+        sort_key = ['name', 'id']
+        sort_dir = ['desc', 'asc']
+        sort = 'name:asc'
+        self.assertRaises(exc.HTTPBadRequest,
+                          list,
+                          self.controller.list(
+                              sort=sort,
+                              sort_key=sort_key,
+                              sort_dir=sort_dir))
+
+    def test_list_images_new_sorting_syntax_invalid_key(self):
+        sort = 'INVALID:asc'
+        self.assertRaises(exc.HTTPBadRequest,
+                          list,
+                          self.controller.list(
+                              sort=sort))
+
+    def test_list_images_new_sorting_syntax_invalid_direction(self):
+        sort = 'name:INVALID'
+        self.assertRaises(exc.HTTPBadRequest,
+                          list,
+                          self.controller.list(
+                              sort=sort))
+
     def test_list_images_for_property(self):
         filters = {'filters': dict([('os_distro', 'NixOS')])}
         images = list(self.controller.list(**filters))
diff --git a/tests/v2/test_shell_v2.py b/tests/v2/test_shell_v2.py
index 033585d..fc8f2db 100644
--- a/tests/v2/test_shell_v2.py
+++ b/tests/v2/test_shell_v2.py
@@ -70,7 +70,8 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': [],
             'sort_key': ['name', 'id'],
-            'sort_dir': ['desc', 'asc']
+            'sort_dir': ['desc', 'asc'],
+            'sort': None
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list:
@@ -102,7 +103,8 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': [],
             'sort_key': ['name'],
-            'sort_dir': ['desc']
+            'sort_dir': ['desc'],
+            'sort': None
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list:
@@ -123,6 +125,39 @@ class ShellV2Test(testtools.TestCase):
                                                 filters=exp_img_filters)
             utils.print_list.assert_called_once_with({}, ['ID', 'Name'])
 
+    def test_do_image_list_new_sorting_syntax(self):
+        input = {
+            'limit': None,
+            'page_size': 18,
+            'visibility': True,
+            'member_status': 'Fake',
+            'owner': 'test',
+            'checksum': 'fake_checksum',
+            'tag': 'fake tag',
+            'properties': [],
+            'sort': 'name:desc,size:asc',
+            'sort_key': [],
+            'sort_dir': []
+        }
+        args = self._make_args(input)
+        with mock.patch.object(self.gc.images, 'list') as mocked_list:
+            mocked_list.return_value = {}
+
+            test_shell.do_image_list(self.gc, args)
+
+            exp_img_filters = {
+                'owner': 'test',
+                'member_status': 'Fake',
+                'visibility': True,
+                'checksum': 'fake_checksum',
+                'tag': 'fake tag'
+            }
+            mocked_list.assert_called_once_with(
+                page_size=18,
+                sort='name:desc,size:asc',
+                filters=exp_img_filters)
+            utils.print_list.assert_called_once_with({}, ['ID', 'Name'])
+
     def test_do_image_list_with_property_filter(self):
         input = {
             'limit': None,
@@ -134,7 +169,8 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': ['os_distro=NixOS', 'architecture=x86_64'],
             'sort_key': ['name'],
-            'sort_dir': ['desc']
+            'sort_dir': ['desc'],
+            'sort': None
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list: