Merge "Add the ability to specify the sort dir for each key"

4 files changed, 92 insertions, 19 deletions

diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py
index a1b0397..5f51539 100644
--- a/glanceclient/v2/images.py
+++ b/glanceclient/v2/images.py
@@ -41,6 +41,12 @@ class Controller(object):
         schema = self.schema_client.get('image')
         return warlock.model_factory(schema.raw(), schemas.SchemaBasedModel)
 
+    @staticmethod
+    def _wrap(value):
+        if isinstance(value, six.string_types):
+            return [value]
+        return value
+
     def list(self, **kwargs):
         """Retrieve a listing of Image objects
 
@@ -98,9 +104,15 @@ class Controller(object):
         # the page_size as Glance's limit.
         filters['limit'] = page_size
 
-        sort_dir = kwargs.get('sort_dir')
-        if sort_dir is not None:
-            filters['sort_dir'] = sort_dir
+        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 = []
@@ -118,12 +130,11 @@ class Controller(object):
         for param in tags_url_params:
             url = '%s&%s' % (url, parse.urlencode(param))
 
-        sort_key = kwargs.get('sort_key')
-        if sort_key is not None:
-            if isinstance(sort_key, six.string_types):
-                sort_key = [sort_key]
-            for key in sort_key:
-                url = '%s&sort_key=%s' % (url, key)
+        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 b598539..d45f3c7 100644
--- a/glanceclient/v2/shell.py
+++ b/glanceclient/v2/shell.py
@@ -128,9 +128,9 @@ def do_image_update(gc, args):
 @utils.arg('--sort-key', default=[], action='append',
            choices=images.SORT_KEY_VALUES,
            help='Sort image list by specified fields.')
-@utils.arg('--sort-dir', default='asc',
+@utils.arg('--sort-dir', default=[], action='append',
            choices=images.SORT_DIR_VALUES,
-           help='Sort image list in specified direction.')
+           help='Sort image list in specified directions.')
 def do_image_list(gc, args):
     """List images you can access."""
     filter_keys = ['visibility', 'member_status', 'owner', 'checksum', 'tag']
@@ -152,7 +152,10 @@ def do_image_list(gc, args):
         kwargs['sort_key'] = args.sort_key
     else:
         kwargs['sort_key'] = ['name']
-    kwargs['sort_dir'] = args.sort_dir
+    if args.sort_dir:
+        kwargs['sort_dir'] = args.sort_dir
+    else:
+        kwargs['sort_dir'] = ['asc']
 
     images = gc.images.list(**kwargs)
     columns = ['ID', 'Name']
diff --git a/tests/v2/test_images.py b/tests/v2/test_images.py
index ec48e3e..1082f26 100644
--- a/tests/v2/test_images.py
+++ b/tests/v2/test_images.py
@@ -440,7 +440,39 @@ data_fixtures = {
                 },
             ]},
         ),
-    }
+    },
+    '/v2/images?limit=%d&sort_dir=desc&sort_key=name&sort_key=id'
+    % images.DEFAULT_PAGE_SIZE: {
+        'GET': (
+            {},
+            {'images': [
+                {
+                    'id': '6f99bf80-2ee6-47cf-acfe-1f1fabb7e810',
+                    'name': 'image-2',
+                },
+                {
+                    'id': '2a4560b2-e585-443e-9b39-553b46ec92d1',
+                    'name': 'image-1',
+                },
+            ]},
+        ),
+    },
+    '/v2/images?limit=%d&sort_dir=desc&sort_dir=asc&sort_key=name&sort_key=id'
+    % 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 = {
@@ -629,6 +661,33 @@ class TestController(testtools.TestCase):
         self.assertEqual(2, len(images))
         self.assertEqual('%s' % img_id1, images[1].id)
 
+    def test_list_images_with_multiple_sort_keys_and_one_sort_dir(self):
+        img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1'
+        sort_key = ['name', 'id']
+        sort_dir = 'desc'
+        images = list(self.controller.list(sort_key=sort_key,
+                                           sort_dir=sort_dir))
+        self.assertEqual(2, len(images))
+        self.assertEqual('%s' % img_id1, images[1].id)
+
+    def test_list_images_with_multiple_sort_dirs(self):
+        img_id1 = '2a4560b2-e585-443e-9b39-553b46ec92d1'
+        sort_key = ['name', 'id']
+        sort_dir = ['desc', 'asc']
+        images = list(self.controller.list(sort_key=sort_key,
+                                           sort_dir=sort_dir))
+        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']
+        self.assertRaises(exc.HTTPBadRequest,
+                          list,
+                          self.controller.list(
+                              sort_key=sort_key,
+                              sort_dir=sort_dir))
+
     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 84137e9..033585d 100644
--- a/tests/v2/test_shell_v2.py
+++ b/tests/v2/test_shell_v2.py
@@ -70,7 +70,7 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': [],
             'sort_key': ['name', 'id'],
-            'sort_dir': 'desc'
+            'sort_dir': ['desc', 'asc']
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list:
@@ -87,7 +87,7 @@ class ShellV2Test(testtools.TestCase):
             }
             mocked_list.assert_called_once_with(page_size=18,
                                                 sort_key=['name', 'id'],
-                                                sort_dir='desc',
+                                                sort_dir=['desc', 'asc'],
                                                 filters=exp_img_filters)
             utils.print_list.assert_called_once_with({}, ['ID', 'Name'])
 
@@ -102,7 +102,7 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': [],
             'sort_key': ['name'],
-            'sort_dir': 'desc'
+            'sort_dir': ['desc']
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list:
@@ -119,7 +119,7 @@ class ShellV2Test(testtools.TestCase):
             }
             mocked_list.assert_called_once_with(page_size=18,
                                                 sort_key=['name'],
-                                                sort_dir='desc',
+                                                sort_dir=['desc'],
                                                 filters=exp_img_filters)
             utils.print_list.assert_called_once_with({}, ['ID', 'Name'])
 
@@ -134,7 +134,7 @@ class ShellV2Test(testtools.TestCase):
             'tag': 'fake tag',
             'properties': ['os_distro=NixOS', 'architecture=x86_64'],
             'sort_key': ['name'],
-            'sort_dir': 'desc'
+            'sort_dir': ['desc']
         }
         args = self._make_args(input)
         with mock.patch.object(self.gc.images, 'list') as mocked_list:
@@ -154,7 +154,7 @@ class ShellV2Test(testtools.TestCase):
 
             mocked_list.assert_called_once_with(page_size=1,
                                                 sort_key=['name'],
-                                                sort_dir='desc',
+                                                sort_dir=['desc'],
                                                 filters=exp_img_filters)
             utils.print_list.assert_called_once_with({}, ['ID', 'Name'])