From fac0cb2f07f43e6845818f3aa6cac59e2881a1eb Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Mon, 19 Jan 2015 21:00:57 +0300 Subject: [PATCH] Add the ability to specify the sort dir for each key Extend rest images api v2 with multiple sort directions support. Example: /v2/images/detail?sort_key=name&sort_dir=asc&sort_key=size&sort_dir=desc Changed database api which now can take sort_dir param as a list. python-glanceclient support will be added in separate commit. Implements-blueprint: glance-sorting-enhancements DocImpact APIImpact Change-Id: Ib43b53abfba7cb5789d916a014376cf38fc5245b --- glance/api/v2/images.py | 46 ++++- glance/common/exception.py | 4 + glance/db/__init__.py | 6 +- glance/db/registry/api.py | 4 +- glance/db/simple/api.py | 36 ++-- glance/db/sqlalchemy/api.py | 18 +- glance/registry/api/v1/images.py | 4 +- glance/tests/functional/db/base.py | 12 +- glance/tests/functional/db/test_sqlalchemy.py | 8 +- glance/tests/unit/test_db.py | 25 ++- glance/tests/unit/v2/test_images_resource.py | 54 ++++-- glance/tests/unit/v2/test_registry_api.py | 160 ++++++++++++++++-- glance/tests/unit/v2/test_registry_client.py | 72 ++++++-- 13 files changed, 366 insertions(+), 83 deletions(-) diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 7af52b6b79..fb05dc2d70 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -88,8 +88,12 @@ class ImagesController(object): return image - def index(self, req, marker=None, limit=None, sort_key=['created_at'], - sort_dir='desc', filters=None, member_status='accepted'): + def index(self, req, marker=None, limit=None, sort_key=None, + sort_dir=None, filters=None, member_status='accepted'): + sort_key = ['created_at'] if not sort_key else sort_key + + sort_dir = ['desc'] if not sort_dir else sort_dir + result = {} if filters is None: filters = {} @@ -313,6 +317,10 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): 'disk_format', 'size', 'id', 'created_at', 'updated_at') + _default_sort_key = 'created_at' + + _default_sort_dir = 'desc' + _path_depth_limits = {'locations': {'add': 2, 'remove': 2, 'replace': 1}} def __init__(self, schema=None): @@ -581,7 +589,6 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): params = request.params.copy() limit = params.pop('limit', None) marker = params.pop('marker', None) - sort_dir = params.pop('sort_dir', 'desc') member_status = params.pop('member_status', 'accepted') # NOTE (flwang) To avoid using comma or any predefined chars to split @@ -599,8 +606,12 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): sort_keys.append(self._validate_sort_key( params.pop('sort_key').strip())) + sort_dirs = [] + while 'sort_dir' in params: + sort_dirs.append(self._validate_sort_dir( + params.pop('sort_dir').strip())) + query_params = { - 'sort_dir': self._validate_sort_dir(sort_dir), 'filters': self._get_filters(params), 'member_status': self._validate_member_status(member_status), } @@ -614,12 +625,31 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer): if tags: query_params['filters']['tags'] = tags - # NOTE(mfedosin): param is still called sort_key, instead of sort_keys + # NOTE(mfedosin): params is still called sort_key and sort_dir, + # instead of sort_keys and sort_dirs respectively. # It's done because in v1 it's still a single value. - if sort_keys: - query_params['sort_key'] = sort_keys + query_params['sort_key'] = sort_keys if sort_keys \ + else [self._default_sort_key] + + # NOTE(mfedosin): we have 3 options here: + # 1. sort_dir wasn't passed: we use default one - 'desc'. + # 2. Only one sort_dir was passed: use it for every sort_key + # in the list. + # 3. Multiple sort_dirs were passed: consistently apply each one to + # the corresponding sort_key. + # If number of sort_dirs and sort_keys doesn't match then raise an + # exception. + if sort_dirs: + dir_len = len(sort_dirs) + key_len = len(sort_keys) + + if dir_len > 1 and dir_len != key_len: + msg = _('Number of sort dirs does not match the number ' + 'of sort keys') + raise webob.exc.HTTPBadRequest(explanation=msg) + query_params['sort_dir'] = sort_dirs else: - query_params['sort_key'] = ['created_at'] + query_params['sort_dir'] = [self._default_sort_dir] return query_params diff --git a/glance/common/exception.py b/glance/common/exception.py index 6dcd8aaf30..dc9b3de79d 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -164,6 +164,10 @@ class InvalidSortKey(Invalid): message = _("Sort key supplied was not valid.") +class InvalidSortDir(Invalid): + message = _("Sort direction supplied was not valid.") + + class InvalidPropertyProtectionConfiguration(Invalid): message = _("Invalid configuration in property protection file.") diff --git a/glance/db/__init__.py b/glance/db/__init__.py index c761c8266d..210923a24d 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -75,8 +75,10 @@ class ImageRepo(object): image = self._format_image_from_db(db_api_image, tags) return ImageProxy(image, self.context, self.db_api) - def list(self, marker=None, limit=None, sort_key=['created_at'], - sort_dir='desc', filters=None, member_status='accepted'): + def list(self, marker=None, limit=None, sort_key=None, + sort_dir=None, filters=None, member_status='accepted'): + sort_key = ['created_at'] if not sort_key else sort_key + sort_dir = ['desc'] if not sort_dir else sort_dir db_api_images = self.db_api.image_get_all( self.context, filters=filters, marker=marker, limit=limit, sort_key=sort_key, sort_dir=sort_dir, diff --git a/glance/db/registry/api.py b/glance/db/registry/api.py index fc094e7a35..7485761ae5 100644 --- a/glance/db/registry/api.py +++ b/glance/db/registry/api.py @@ -118,7 +118,7 @@ def is_image_visible(context, image, status=None): @_get_client def image_get_all(client, filters=None, marker=None, limit=None, - sort_key=['created_at'], sort_dir='desc', + sort_key=None, sort_dir=None, member_status='accepted', is_public=None, admin_as_user=False, return_tag=False): """ @@ -142,6 +142,8 @@ def image_get_all(client, filters=None, marker=None, limit=None, relevant tag entries. This could improve upper-layer query performance, to prevent using separated calls """ + sort_key = ['created_at'] if not sort_key else sort_key + sort_dir = ['desc'] if not sort_dir else sort_dir return client.image_get_all(filters=filters, marker=marker, limit=limit, sort_key=sort_key, sort_dir=sort_dir, member_status=member_status, diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index c683a12ad0..4acb98d9fe 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -15,8 +15,8 @@ # under the License. import copy -import datetime import functools +from operator import itemgetter import uuid from oslo_utils import timeutils @@ -340,29 +340,35 @@ def _do_pagination(context, images, marker, limit, show_deleted, def _sort_images(images, sort_key, sort_dir): - reverse = sort_dir == 'desc' + sort_key = ['created_at'] if not sort_key else sort_key + + default_sort_dir = 'desc' + + if not sort_dir: + sort_dir = [default_sort_dir] * len(sort_key) + elif len(sort_dir) == 1: + default_sort_dir = sort_dir[0] + sort_dir *= len(sort_key) for key in ['created_at', 'id']: if key not in sort_key: sort_key.append(key) + sort_dir.append(default_sort_dir) for key in sort_key: if images and not (key in images[0]): raise exception.InvalidSortKey() - def sort_func(element): - keys = [] - for key in sort_key: - if element[key] is not None: - keys.append(element[key]) - else: - if key in ['created_at', 'updated_at', 'deleted_at']: - keys.append(datetime.datetime.min) - else: - keys.append('') - return tuple(keys) + if any(dir for dir in sort_dir if dir not in ['asc', 'desc']): + raise exception.InvalidSortDir() - images.sort(key=sort_func, reverse=reverse) + if len(sort_key) != len(sort_dir): + raise exception.Invalid(message='Number of sort dirs does not match ' + 'the number of sort keys') + + for key, dir in reversed(zip(sort_key, sort_dir)): + reverse = dir == 'desc' + images.sort(key=itemgetter(key), reverse=reverse) return images @@ -395,7 +401,7 @@ def image_get(context, image_id, session=None, force_show_deleted=False): @log_call def image_get_all(context, filters=None, marker=None, limit=None, - sort_key=['created_at'], sort_dir='desc', + sort_key=None, sort_dir=None, member_status='accepted', is_public=None, admin_as_user=False, return_tag=False): filters = filters or {} diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 9e3d8554ff..ca933ff590 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -526,7 +526,7 @@ def _select_images_query(context, image_conditions, admin_as_user, def image_get_all(context, filters=None, marker=None, limit=None, - sort_key=['created_at'], sort_dir='desc', + sort_key=None, sort_dir=None, member_status='accepted', is_public=None, admin_as_user=False, return_tag=False): """ @@ -538,7 +538,7 @@ def image_get_all(context, filters=None, marker=None, limit=None, :param marker: image id after which to start page :param limit: maximum number of images to return :param sort_key: list of image attributes by which results should be sorted - :param sort_dir: direction in which results should be sorted (asc, desc) + :param sort_dir: directions in which results should be sorted (asc, desc) :param member_status: only return shared images that have this membership status :param is_public: If true, return only public images. If false, return @@ -550,6 +550,16 @@ def image_get_all(context, filters=None, marker=None, limit=None, relevant tag entries. This could improve upper-layer query performance, to prevent using separated calls """ + sort_key = ['created_at'] if not sort_key else sort_key + + default_sort_dir = 'desc' + + if not sort_dir: + sort_dir = [default_sort_dir] * len(sort_key) + elif len(sort_dir) == 1: + default_sort_dir = sort_dir[0] + sort_dir *= len(sort_key) + filters = filters or {} visibility = filters.pop('visibility', None) @@ -590,11 +600,13 @@ def image_get_all(context, filters=None, marker=None, limit=None, for key in ['created_at', 'id']: if key not in sort_key: sort_key.append(key) + sort_dir.append(default_sort_dir) query = _paginate_query(query, models.Image, limit, sort_key, marker=marker_image, - sort_dir=sort_dir) + sort_dir=None, + sort_dirs=sort_dir) query = query.options(sa_orm.joinedload( models.Image.properties)).options( diff --git a/glance/registry/api/v1/images.py b/glance/registry/api/v1/images.py index f6e89bc4bc..94c0a55040 100644 --- a/glance/registry/api/v1/images.py +++ b/glance/registry/api/v1/images.py @@ -196,7 +196,7 @@ class Controller(object): 'filters': self._get_filters(req), 'limit': self._get_limit(req), 'sort_key': [self._get_sort_key(req)], - 'sort_dir': self._get_sort_dir(req), + 'sort_dir': [self._get_sort_dir(req)], 'marker': self._get_marker(req), } @@ -292,7 +292,7 @@ class Controller(object): def _get_sort_dir(self, req): """Parse a sort direction query param from the request object.""" - sort_dir = req.params.get('sort_dir', None) + sort_dir = req.params.get('sort_dir', 'desc') if sort_dir is not None and sort_dir not in SUPPORTED_SORT_DIRS: _keys = ', '.join(SUPPORTED_SORT_DIRS) msg = _("Unsupported sort_dir. Acceptable values: %s") % (_keys,) diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 3618c25707..2e7a4782f6 100644 --- a/glance/tests/functional/db/base.py +++ b/glance/tests/functional/db/base.py @@ -549,7 +549,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['name'], - sort_dir='desc') + sort_dir=['desc']) image_ids = [image['id'] for image in images] expected = [] self.assertEqual(sorted(expected), sorted(image_ids)) @@ -571,7 +571,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['disk_format'], - sort_dir='desc') + sort_dir=['desc']) image_ids = [image['id'] for image in images] expected = [] self.assertEqual(sorted(expected), sorted(image_ids)) @@ -593,7 +593,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['container_format'], - sort_dir='desc') + sort_dir=['desc']) image_ids = [image['id'] for image in images] expected = [] self.assertEqual(sorted(expected), sorted(image_ids)) @@ -615,7 +615,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['name'], - sort_dir='asc') + sort_dir=['asc']) image_ids = [image['id'] for image in images] expected = [UUID3, UUID2, UUID1] self.assertEqual(sorted(expected), sorted(image_ids)) @@ -637,7 +637,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['disk_format'], - sort_dir='asc') + sort_dir=['asc']) image_ids = [image['id'] for image in images] expected = [UUID3, UUID2, UUID1] self.assertEqual(sorted(expected), sorted(image_ids)) @@ -659,7 +659,7 @@ class DriverTests(object): images = self.db_api.image_get_all(ctxt1, marker=UUIDX, sort_key=['container_format'], - sort_dir='asc') + sort_dir=['asc']) image_ids = [image['id'] for image in images] expected = [UUID3, UUID2, UUID1] self.assertEqual(sorted(expected), sorted(image_ids)) diff --git a/glance/tests/functional/db/test_sqlalchemy.py b/glance/tests/functional/db/test_sqlalchemy.py index 6fe0054346..b880785c2e 100644 --- a/glance/tests/functional/db/test_sqlalchemy.py +++ b/glance/tests/functional/db/test_sqlalchemy.py @@ -112,10 +112,10 @@ class TestSqlAlchemyDBDataIntegrity(base.TestDriver, original_method = self.db_api._paginate_query def fake_paginate_query(query, model, limit, - sort_keys, marker, sort_dir): + sort_keys, marker, sort_dir, sort_dirs): self.assertEqual(['created_at', 'id'], sort_keys) return original_method(query, model, limit, - sort_keys, marker, sort_dir) + sort_keys, marker, sort_dir, sort_dirs) self.stubs.Set(self.db_api, '_paginate_query', fake_paginate_query) @@ -125,10 +125,10 @@ class TestSqlAlchemyDBDataIntegrity(base.TestDriver, original_method = self.db_api._paginate_query def fake_paginate_query(query, model, limit, - sort_keys, marker, sort_dir): + sort_keys, marker, sort_dir, sort_dirs): self.assertEqual(['name', 'created_at', 'id'], sort_keys) return original_method(query, model, limit, - sort_keys, marker, sort_dir) + sort_keys, marker, sort_dir, sort_dirs) self.stubs.Set(self.db_api, '_paginate_query', fake_paginate_query) diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index ecfa2326b0..3eda43752b 100644 --- a/glance/tests/unit/test_db.py +++ b/glance/tests/unit/test_db.py @@ -302,7 +302,7 @@ class TestImageRepo(test_utils.BaseTestCase): self.assertEqual(set([UUID1, UUID3]), image_ids) def test_sorted_list(self): - images = self.image_repo.list(sort_key=['size'], sort_dir='asc') + images = self.image_repo.list(sort_key=['size'], sort_dir=['asc']) image_ids = [i.image_id for i in images] self.assertEqual([UUID1, UUID2, UUID3], image_ids) @@ -316,15 +316,34 @@ class TestImageRepo(test_utils.BaseTestCase): 'status': 'active'}]) self.db.image_create(None, image) images = self.image_repo.list(sort_key=['name', 'size'], - sort_dir='asc') + sort_dir=['asc']) image_ids = [i.image_id for i in images] self.assertEqual([UUID1, temp_id, UUID2, UUID3], image_ids) images = self.image_repo.list(sort_key=['size', 'name'], - sort_dir='asc') + sort_dir=['asc']) image_ids = [i.image_id for i in images] self.assertEqual([UUID1, UUID2, temp_id, UUID3], image_ids) + def test_sorted_list_with_multiple_dirs(self): + temp_id = 'd80a1a6c-bd1f-41c5-90ee-81afedb1d58d' + image = _db_fixture(temp_id, owner=TENANT1, checksum=CHECKSUM, + name='1', size=1024, + is_public=True, status='active', + locations=[{'url': UUID1_LOCATION, + 'metadata': UUID1_LOCATION_METADATA, + 'status': 'active'}]) + self.db.image_create(None, image) + images = self.image_repo.list(sort_key=['name', 'size'], + sort_dir=['asc', 'desc']) + image_ids = [i.image_id for i in images] + self.assertEqual([temp_id, UUID1, UUID2, UUID3], image_ids) + + images = self.image_repo.list(sort_key=['name', 'size'], + sort_dir=['desc', 'asc']) + image_ids = [i.image_id for i in images] + self.assertEqual([UUID3, UUID2, UUID1, temp_id], image_ids) + def test_add_image(self): image = self.image_factory.new_image(name='added image') self.assertEqual(image.updated_at, image.created_at) diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 0df24773cd..0c57b52f60 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -217,7 +217,7 @@ class TestImagesController(base.IsolatedUnitTest): request = unit_test_utils.get_fake_request() output = self.controller.index(request, marker=UUID3, limit=1, sort_key=['created_at'], - sort_dir='desc') + sort_dir=['desc']) self.assertEqual(1, len(output['images'])) actual = set([image.image_id for image in output['images']]) expected = set([UUID2]) @@ -416,7 +416,7 @@ class TestImagesController(base.IsolatedUnitTest): def test_index_with_sort_dir(self): path = '/images' request = unit_test_utils.get_fake_request(path) - output = self.controller.index(request, sort_dir='asc', limit=3) + output = self.controller.index(request, sort_dir=['asc'], limit=3) actual = [image.image_id for image in output['images']] self.assertEqual(3, len(actual)) self.assertEqual(UUID1, actual[0]) @@ -2492,7 +2492,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase): expected = {'limit': 1, 'marker': marker, 'sort_key': ['created_at'], - 'sort_dir': 'desc', + 'sort_dir': ['desc'], 'member_status': 'pending', 'filters': {}} output = self.deserializer.index(request) @@ -2541,7 +2541,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase): expected = {'limit': 0, 'sort_key': ['created_at'], 'member_status': 'accepted', - 'sort_dir': 'desc', + 'sort_dir': ['desc'], 'filters': {}} output = self.deserializer.index(request) self.assertEqual(expected, output) @@ -2584,7 +2584,7 @@ class TestImagesDeserializer(test_utils.BaseTestCase): output = self.deserializer.index(request) expected = { 'sort_key': ['id'], - 'sort_dir': 'desc', + 'sort_dir': ['desc'], 'member_status': 'accepted', 'filters': {} } @@ -2597,13 +2597,14 @@ class TestImagesDeserializer(test_utils.BaseTestCase): output = self.deserializer.index(request) expected = { 'sort_key': ['name', 'size'], - 'sort_dir': 'desc', + 'sort_dir': ['desc'], 'member_status': 'accepted', 'filters': {} } self.assertEqual(expected, output) def test_index_invalid_multiple_sort_keys(self): + # blah is an invalid sort key request = unit_test_utils.get_fake_request('/images?' 'sort_key=name&' 'sort_key=blah') @@ -2615,23 +2616,56 @@ class TestImagesDeserializer(test_utils.BaseTestCase): output = self.deserializer.index(request) expected = { 'sort_key': ['created_at'], - 'sort_dir': 'asc', + 'sort_dir': ['asc'], 'member_status': 'accepted', 'filters': {}} self.assertEqual(expected, output) + def test_index_multiple_sort_dirs(self): + req_string = ('/images?sort_key=name&sort_dir=asc&' + 'sort_key=id&sort_dir=desc') + request = unit_test_utils.get_fake_request(req_string) + output = self.deserializer.index(request) + expected = { + 'sort_key': ['name', 'id'], + 'sort_dir': ['asc', 'desc'], + 'member_status': 'accepted', + 'filters': {}} + self.assertEqual(expected, output) + + def test_index_sort_wrong_sort_dirs_number(self): + req_string = '/images?sort_key=name&sort_dir=asc&sort_dir=desc' + request = unit_test_utils.get_fake_request(req_string) + self.assertRaises(webob.exc.HTTPBadRequest, + self.deserializer.index, request) + + def test_index_sort_dirs_fewer_than_keys(self): + req_string = ('/images?sort_key=name&sort_dir=asc&sort_key=id&' + 'sort_dir=asc&sort_key=created_at') + request = unit_test_utils.get_fake_request(req_string) + self.assertRaises(webob.exc.HTTPBadRequest, + self.deserializer.index, request) + + def test_index_sort_wrong_sort_dirs_number_without_key(self): + req_string = '/images?sort_dir=asc&sort_dir=desc' + request = unit_test_utils.get_fake_request(req_string) + self.assertRaises(webob.exc.HTTPBadRequest, + self.deserializer.index, request) + def test_index_sort_private_key(self): request = unit_test_utils.get_fake_request('/images?sort_key=min_ram') self.assertRaises(webob.exc.HTTPBadRequest, self.deserializer.index, request) - def test_index_sort_key_bad_value(self): + def test_index_sort_key_invalid_value(self): + # blah is an invalid sort key request = unit_test_utils.get_fake_request('/images?sort_key=blah') self.assertRaises(webob.exc.HTTPBadRequest, self.deserializer.index, request) - def test_index_sort_dir_bad_value(self): - request = unit_test_utils.get_fake_request('/images?sort_dir=blah') + def test_index_sort_dir_invalid_value(self): + # foo is an invalid sort dir + request = unit_test_utils.get_fake_request('/images?sort_dir=foo') self.assertRaises(webob.exc.HTTPBadRequest, self.deserializer.index, request) diff --git a/glance/tests/unit/v2/test_registry_api.py b/glance/tests/unit/v2/test_registry_api.py index da8a63e2fe..f43a496aca 100644 --- a/glance/tests/unit/v2/test_registry_api.py +++ b/glance/tests/unit/v2/test_registry_api.py @@ -265,7 +265,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['name'], - 'sort_dir': 'asc'}, + 'sort_dir': ['asc']}, }] req.body = jsonutils.dumps(cmd) @@ -298,7 +298,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['name'], - 'sort_dir': 'desc'}, + 'sort_dir': ['desc']}, }] req.body = jsonutils.dumps(cmd) @@ -331,7 +331,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['disk_format'], - 'sort_dir': 'asc'}, + 'sort_dir': ['asc']}, }] req.body = jsonutils.dumps(cmd) @@ -364,7 +364,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['disk_format'], - 'sort_dir': 'desc'}, + 'sort_dir': ['desc']}, }] req.body = jsonutils.dumps(cmd) @@ -397,7 +397,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['container_format'], - 'sort_dir': 'asc'}, + 'sort_dir': ['asc']}, }] req.body = jsonutils.dumps(cmd) @@ -430,7 +430,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'marker': UUID3, 'sort_key': ['container_format'], - 'sort_dir': 'desc'}, + 'sort_dir': ['desc']}, }] req.body = jsonutils.dumps(cmd) @@ -831,7 +831,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): req.method = "POST" cmd = [{ 'command': 'image_get_all', - 'kwargs': {'sort_key': ['name'], 'sort_dir': 'asc'} + 'kwargs': {'sort_key': ['name'], 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -883,7 +883,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): req.method = "POST" cmd = [{ 'command': 'image_get_all', - 'kwargs': {'sort_key': ['status'], 'sort_dir': 'asc'} + 'kwargs': {'sort_key': ['status'], 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -934,7 +934,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): req.method = "POST" cmd = [{ 'command': 'image_get_all', - 'kwargs': {'sort_key': ['disk_format'], 'sort_dir': 'asc'} + 'kwargs': {'sort_key': ['disk_format'], 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -986,7 +986,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['container_format'], - 'sort_dir': 'desc'} + 'sort_dir': ['desc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1034,7 +1034,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['size'], - 'sort_dir': 'asc'} + 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1089,7 +1089,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['created_at'], - 'sort_dir': 'asc'} + 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1144,7 +1144,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['updated_at'], - 'sort_dir': 'desc'} + 'sort_dir': ['desc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1158,10 +1158,11 @@ class TestRegistryRPC(base.IsolatedUnitTest): self.assertEqual(UUID2, images[2]['id']) self.assertEqual(UUID1, images[3]['id']) - def test_get_index_sort_multiple_keys(self): + def test_get_index_sort_multiple_keys_one_sort_dir(self): """ Tests that the registry API returns list of - public images sorted by name-size and size-name. + public images sorted by name-size and size-name with ascending + sort direction. """ uuid4_time = timeutils.utcnow() + datetime.timedelta(seconds=10) uuid3_time = uuid4_time + datetime.timedelta(seconds=5) @@ -1213,7 +1214,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['name', 'size'], - 'sort_dir': 'asc'} + 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1231,7 +1232,7 @@ class TestRegistryRPC(base.IsolatedUnitTest): cmd = [{ 'command': 'image_get_all', 'kwargs': {'sort_key': ['size', 'name'], - 'sort_dir': 'asc'} + 'sort_dir': ['asc']} }] req.body = jsonutils.dumps(cmd) res = req.get_response(self.api) @@ -1246,6 +1247,131 @@ class TestRegistryRPC(base.IsolatedUnitTest): self.assertEqual(UUID5, images[3]['id']) self.assertEqual(UUID4, images[4]['id']) + def test_get_index_sort_multiple_keys_multiple_sort_dirs(self): + """ + Tests that the registry API returns list of + public images sorted by name-size and size-name + with ascending and descending directions. + """ + uuid4_time = timeutils.utcnow() + datetime.timedelta(seconds=10) + uuid3_time = uuid4_time + datetime.timedelta(seconds=5) + + UUID3 = _gen_uuid() + extra_fixture = {'id': UUID3, + 'status': 'active', + 'is_public': True, + 'disk_format': 'vhd', + 'container_format': 'ovf', + 'name': 'asdf', + 'size': 19, + 'checksum': None, + 'created_at': None, + 'updated_at': uuid3_time} + + db_api.image_create(self.context, extra_fixture) + + UUID4 = _gen_uuid() + extra_fixture = {'id': UUID4, + 'status': 'active', + 'is_public': True, + 'disk_format': 'vhd', + 'container_format': 'ovf', + 'name': 'xyz', + 'size': 20, + 'checksum': None, + 'created_at': None, + 'updated_at': uuid4_time} + + db_api.image_create(self.context, extra_fixture) + + UUID5 = _gen_uuid() + extra_fixture = {'id': UUID5, + 'status': 'active', + 'is_public': True, + 'disk_format': 'vhd', + 'container_format': 'ovf', + 'name': 'asdf', + 'size': 20, + 'checksum': None, + 'created_at': None, + 'updated_at': uuid4_time} + + db_api.image_create(self.context, extra_fixture) + + req = webob.Request.blank('/rpc') + req.method = "POST" + cmd = [{ + 'command': 'image_get_all', + 'kwargs': {'sort_key': ['name', 'size'], + 'sort_dir': ['desc', 'asc']} + }] + req.body = jsonutils.dumps(cmd) + res = req.get_response(self.api) + self.assertEqual(200, res.status_int) + res_dict = jsonutils.loads(res.body)[0] + + images = res_dict + self.assertEqual(5, len(images)) + self.assertEqual(UUID4, images[0]['id']) + self.assertEqual(UUID2, images[1]['id']) + self.assertEqual(UUID1, images[2]['id']) + self.assertEqual(UUID3, images[3]['id']) + self.assertEqual(UUID5, images[4]['id']) + + cmd = [{ + 'command': 'image_get_all', + 'kwargs': {'sort_key': ['size', 'name'], + 'sort_dir': ['desc', 'asc']} + }] + req.body = jsonutils.dumps(cmd) + res = req.get_response(self.api) + self.assertEqual(200, res.status_int) + res_dict = jsonutils.loads(res.body)[0] + + images = res_dict + self.assertEqual(5, len(images)) + self.assertEqual(UUID5, images[0]['id']) + self.assertEqual(UUID4, images[1]['id']) + self.assertEqual(UUID3, images[2]['id']) + self.assertEqual(UUID2, images[3]['id']) + self.assertEqual(UUID1, images[4]['id']) + + cmd = [{ + 'command': 'image_get_all', + 'kwargs': {'sort_key': ['name', 'size'], + 'sort_dir': ['asc', 'desc']} + }] + req.body = jsonutils.dumps(cmd) + res = req.get_response(self.api) + self.assertEqual(200, res.status_int) + res_dict = jsonutils.loads(res.body)[0] + + images = res_dict + self.assertEqual(5, len(images)) + self.assertEqual(UUID5, images[0]['id']) + self.assertEqual(UUID3, images[1]['id']) + self.assertEqual(UUID1, images[2]['id']) + self.assertEqual(UUID2, images[3]['id']) + self.assertEqual(UUID4, images[4]['id']) + + cmd = [{ + 'command': 'image_get_all', + 'kwargs': {'sort_key': ['size', 'name'], + 'sort_dir': ['asc', 'desc']} + }] + req.body = jsonutils.dumps(cmd) + res = req.get_response(self.api) + self.assertEqual(200, res.status_int) + res_dict = jsonutils.loads(res.body)[0] + + images = res_dict + self.assertEqual(5, len(images)) + self.assertEqual(UUID1, images[0]['id']) + self.assertEqual(UUID2, images[1]['id']) + self.assertEqual(UUID3, images[2]['id']) + self.assertEqual(UUID4, images[3]['id']) + self.assertEqual(UUID5, images[4]['id']) + def test_create_image(self): """Tests that the registry API creates the image""" fixture = {'name': 'fake public image', diff --git a/glance/tests/unit/v2/test_registry_client.py b/glance/tests/unit/v2/test_registry_client.py index 82c7ae77c3..f307bce982 100644 --- a/glance/tests/unit/v2/test_registry_client.py +++ b/glance/tests/unit/v2/test_registry_client.py @@ -117,7 +117,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['name'], - sort_dir='asc') + sort_dir=['asc']) self.assertEqualImages(images, (UUID3, UUID1, UUID2, UUID4), unjsonify=False) @@ -142,7 +142,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['status'], - sort_dir='desc') + sort_dir=['desc']) self.assertEqualImages(images, (UUID3, UUID4, UUID2, UUID1), unjsonify=False) @@ -166,7 +166,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['disk_format'], - sort_dir='asc') + sort_dir=['asc']) self.assertEqualImages(images, (UUID1, UUID3, UUID4, UUID2), unjsonify=False) @@ -191,7 +191,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['container_format'], - sort_dir='desc') + sort_dir=['desc']) self.assertEqualImages(images, (UUID2, UUID4, UUID3, UUID1), unjsonify=False) @@ -217,7 +217,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) - images = self.client.image_get_all(sort_key=['size'], sort_dir='asc') + images = self.client.image_get_all(sort_key=['size'], sort_dir=['asc']) self.assertEqualImages(images, (UUID4, UUID1, UUID2, UUID3), unjsonify=False) @@ -241,7 +241,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['created_at'], - sort_dir='asc') + sort_dir=['asc']) self.assertEqualImages(images, (UUID1, UUID2, UUID4, UUID3), unjsonify=False) @@ -267,7 +267,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['updated_at'], - sort_dir='desc') + sort_dir=['desc']) self.assertEqualImages(images, (UUID3, UUID4, UUID2, UUID1), unjsonify=False) @@ -275,7 +275,7 @@ class TestRegistryV2Client(base.IsolatedUnitTest, def test_get_image_details_sort_multiple_keys(self): """ Tests that a detailed call returns list of - public images sorted alphabetically by name-size and + public images sorted by name-size and size-name in ascending order. """ UUID3 = _gen_uuid() @@ -284,6 +284,42 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) + UUID4 = _gen_uuid() + extra_fixture = self.get_fixture(id=UUID4, name=u'xyz', + size=20) + + db_api.image_create(self.context, extra_fixture) + + UUID5 = _gen_uuid() + extra_fixture = self.get_fixture(id=UUID5, name=u'asdf', + size=20) + + db_api.image_create(self.context, extra_fixture) + + images = self.client.image_get_all(sort_key=['name', 'size'], + sort_dir=['asc']) + + self.assertEqualImages(images, (UUID3, UUID5, UUID1, UUID2, UUID4), + unjsonify=False) + + images = self.client.image_get_all(sort_key=['size', 'name'], + sort_dir=['asc']) + + self.assertEqualImages(images, (UUID1, UUID3, UUID2, UUID5, UUID4), + unjsonify=False) + + def test_get_image_details_sort_multiple_dirs(self): + """ + Tests that a detailed call returns list of + public images sorted by name-size and + size-name in ascending and descending orders. + """ + UUID3 = _gen_uuid() + extra_fixture = self.get_fixture(id=UUID3, name='asdf', + size=19) + + db_api.image_create(self.context, extra_fixture) + UUID4 = _gen_uuid() extra_fixture = self.get_fixture(id=UUID4, name='xyz', size=20) @@ -297,15 +333,27 @@ class TestRegistryV2Client(base.IsolatedUnitTest, db_api.image_create(self.context, extra_fixture) images = self.client.image_get_all(sort_key=['name', 'size'], - sort_dir='asc') + sort_dir=['asc', 'desc']) - self.assertEqualImages(images, (UUID3, UUID5, UUID1, UUID2, UUID4), + self.assertEqualImages(images, (UUID5, UUID3, UUID1, UUID2, UUID4), + unjsonify=False) + + images = self.client.image_get_all(sort_key=['name', 'size'], + sort_dir=['desc', 'asc']) + + self.assertEqualImages(images, (UUID4, UUID2, UUID1, UUID3, UUID5), unjsonify=False) images = self.client.image_get_all(sort_key=['size', 'name'], - sort_dir='asc') + sort_dir=['asc', 'desc']) - self.assertEqualImages(images, (UUID1, UUID3, UUID2, UUID5, UUID4), + self.assertEqualImages(images, (UUID1, UUID2, UUID3, UUID4, UUID5), + unjsonify=False) + + images = self.client.image_get_all(sort_key=['size', 'name'], + sort_dir=['desc', 'asc']) + + self.assertEqualImages(images, (UUID5, UUID4, UUID3, UUID2, UUID1), unjsonify=False) def test_image_get_index_marker(self):