From 3f3066be97349677fd489703effb85f45c0cdd76 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Wed, 21 Jan 2015 19:17:20 +0300 Subject: [PATCH] Extend images CLI v2 with new sorting syntax This code enables new syntax for sorting output with multiple keys and directions based on API Working group sorting guidelines. It's a client code to consume API modified in change Ie4ccfefa0492a3ac94cc7e22201f2f2be5c1cdbb Example: glance --os-image-api-version 2 --sort name:desc,size:asc Implements-blueprint: glance-sorting-enhancements DocImpact Depends-On: Ie4ccfefa0492a3ac94cc7e22201f2f2be5c1cdbb Change-Id: I36a9fa9f0508fea1235de2ac3a0d6a093e1af635 --- glanceclient/v2/images.py | 57 +++++++++++++++++++++++++++++---------- glanceclient/v2/shell.py | 13 ++++++--- tests/v2/test_images.py | 48 +++++++++++++++++++++++++++++++++ tests/v2/test_shell_v2.py | 42 ++++++++++++++++++++++++++--- 4 files changed, 139 insertions(+), 21 deletions(-) diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index 5f515395..c9fb0a7c 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) + 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', [])) - for dir in sort_dir: - url = '%s&sort_dir=%s' % (url, dir) + 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 d45f3c72..26289198 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='[:]', default=None, + help=(("Comma-separated list of sort keys and directions in the " + "form of [:]. 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 ac21ba6d..06f225f7 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 = { @@ -678,6 +694,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'] @@ -687,6 +710,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 033585d8..fc8f2db5 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: