From f8e595b0fa4d88bf764e13e7f871e0a0f05a8fde Mon Sep 17 00:00:00 2001 From: Timur Sufiev Date: Fri, 24 Oct 2014 20:14:46 +0400 Subject: [PATCH] Sort images list in ascending alphabetical order Move most of the pagination-logic to `api.glance.image_list_detailed`, thus making code in Admin/Project->Images->get_data() less confusing (and remove hard-coded 'asc'|'desc' values). Also prepare to get images both from glanceclient.v1 and glanceclient.v2 (which doesn't set `is_public` attr on images using `visibility` attr instead). Change-Id: Ibe6d3dd1e94a1d1fbf95382599a5f53c3559ce5a Closes-Bug: #1534670 Closes-Bug: #1336317 --- openstack_dashboard/api/glance.py | 74 ++++++++++++++++++- .../dashboards/admin/images/tests.py | 45 +++++------ .../dashboards/admin/images/views.py | 11 +-- .../project/images/images/tables.py | 5 +- .../dashboards/project/images/tests.py | 20 +++-- .../dashboards/project/images/views.py | 19 ++++- 6 files changed, 123 insertions(+), 51 deletions(-) diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index ac45a0d17b..d9c5616a34 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -80,8 +80,63 @@ def image_get(request, image_id): return image +def is_image_public(im): + is_public_v1 = getattr(im, 'is_public', None) + if is_public_v1 is not None: + return is_public_v1 + else: + return im.visibility == 'public' + + def image_list_detailed(request, marker=None, sort_dir='desc', - sort_key='created_at', filters=None, paginate=False): + sort_key='created_at', filters=None, paginate=False, + reversed_order=False): + """Thin layer above glanceclient, for handling pagination issues. + + It provides iterating both forward and backward on top of ascetic + OpenStack pagination API - which natively supports only iterating forward + through the entries. Thus in order to retrieve list of objects at previous + page, a request with the reverse entries order had to be made to Glance, + using the first object id on current page as the marker - restoring + the original items ordering before sending them back to the UI. + + .. param:: request + + The request object coming from browser to be passed further into + Glance service. + + .. param:: marker + + The id of an object which defines a starting point of a query sent to + Glance service. + + .. param:: sort_dir + + The direction by which the resulting image list throughout all pages + (if pagination is enabled) will be sorted. Could be either 'asc' + (ascending) or 'desc' (descending), defaults to 'desc'. + + .. param:: sort_key + + The name of key by by which the resulting image list throughout all + pages (if pagination is enabled) will be sorted. Defaults to + 'created_at'. + + .. param:: filters + + A dictionary of filters passed as is to Glance service. + + .. param:: paginate + + Whether the pagination is enabled. If it is, then the number of + entries on a single page of images table is limited to the specific + number stored in browser cookies. + + .. param:: reversed_order + + Set this flag to True when it's necessary to get a reversed list of + images from Glance (used for navigating the images list back in UI). + """ limit = getattr(settings, 'API_RESULT_LIMIT', 1000) page_size = utils.get_page_size(request) @@ -93,9 +148,13 @@ def image_list_detailed(request, marker=None, sort_dir='desc', kwargs = {'filters': filters or {}} if marker: kwargs['marker'] = marker - kwargs['sort_dir'] = sort_dir kwargs['sort_key'] = sort_key + if not reversed_order: + kwargs['sort_dir'] = sort_dir + else: + kwargs['sort_dir'] = 'desc' if sort_dir == 'asc' else 'asc' + images_iter = glanceclient(request).images.list(page_size=request_size, limit=limit, **kwargs) @@ -111,14 +170,21 @@ def image_list_detailed(request, marker=None, sort_dir='desc', if marker is not None: has_prev_data = True # first page condition when reached via prev back - elif sort_dir == 'asc' and marker is not None: + elif reversed_order and marker is not None: has_more_data = True # last page condition elif marker is not None: has_prev_data = True + + # restore the original ordering here + if reversed_order: + images = sorted(images, key=lambda image: + (getattr(image, sort_key) or '').lower(), + reverse=(sort_dir == 'desc')) else: images = list(images_iter) - return (images, has_more_data, has_prev_data) + + return images, has_more_data, has_prev_data def image_update(request, image_id, **kwargs): diff --git a/openstack_dashboard/dashboards/admin/images/tests.py b/openstack_dashboard/dashboards/admin/images/tests.py index d4298fb804..42c5e79782 100644 --- a/openstack_dashboard/dashboards/admin/images/tests.py +++ b/openstack_dashboard/dashboards/admin/images/tests.py @@ -51,7 +51,9 @@ class ImagesViewTest(test.BaseAdminViewTests): marker=None, paginate=True, filters=filters, - sort_dir='desc') \ + sort_dir='asc', + sort_key='name', + reversed_order=False) \ .AndReturn([self.images.list(), False, False]) # Test tenant list @@ -72,29 +74,24 @@ class ImagesViewTest(test.BaseAdminViewTests): def test_images_list_get_pagination(self): images = self.images.list()[:5] filters = {'is_public': None} + kwargs = {'paginate': True, 'filters': filters, + 'sort_dir': 'asc', 'sort_key': 'name', + 'reversed_order': False} api.glance.image_list_detailed(IsA(http.HttpRequest), marker=None, - paginate=True, - filters=filters, - sort_dir='desc') \ + **kwargs) \ .AndReturn([images, True, True]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=None, - paginate=True, - filters=filters, - sort_dir='desc') \ + **kwargs) \ .AndReturn([images[:2], True, True]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=images[2].id, - paginate=True, - filters=filters, - sort_dir='desc') \ + **kwargs) \ .AndReturn([images[2:4], True, True]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=images[4].id, - paginate=True, - filters=filters, - sort_dir='desc') \ + **kwargs) \ .AndReturn([images[4:], True, True]) # Test tenant list api.keystone.tenant_list(IsA(http.HttpRequest)).MultipleTimes().\ @@ -138,29 +135,27 @@ class ImagesViewTest(test.BaseAdminViewTests): def test_images_list_get_prev_pagination(self): images = self.images.list()[:3] filters = {'is_public': None} + kwargs = {'paginate': True, 'filters': filters, + 'sort_dir': 'asc', 'sort_key': 'name'} api.glance.image_list_detailed(IsA(http.HttpRequest), marker=None, - paginate=True, - filters=filters, - sort_dir='desc') \ + reversed_order=False, + **kwargs) \ .AndReturn([images, True, False]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=None, - paginate=True, - filters=filters, - sort_dir='desc') \ + reversed_order=False, + **kwargs) \ .AndReturn([images[:2], True, True]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=images[2].id, - paginate=True, - filters=filters, - sort_dir='desc') \ + reversed_order=False, + **kwargs) \ .AndReturn([images[2:], True, True]) api.glance.image_list_detailed(IsA(http.HttpRequest), marker=images[2].id, - paginate=True, - filters=filters, - sort_dir='asc') \ + reversed_order=True, + **kwargs) \ .AndReturn([images[:2], True, True]) # Test tenant list api.keystone.tenant_list(IsA(http.HttpRequest)).MultipleTimes().\ diff --git a/openstack_dashboard/dashboards/admin/images/views.py b/openstack_dashboard/dashboards/admin/images/views.py index 1eaa6a9de9..ce6a7cec6b 100644 --- a/openstack_dashboard/dashboards/admin/images/views.py +++ b/openstack_dashboard/dashboards/admin/images/views.py @@ -56,23 +56,20 @@ class IndexView(tables.DataTableView): project_tables.AdminImagesTable._meta.prev_pagination_param, None) if prev_marker is not None: - sort_dir = 'asc' marker = prev_marker else: - sort_dir = 'desc' marker = self.request.GET.get( project_tables.AdminImagesTable._meta.pagination_param, None) + reversed_order = prev_marker is not None try: images, self._more, self._prev = api.glance.image_list_detailed( self.request, marker=marker, paginate=True, filters=filters, - sort_dir=sort_dir) - - if prev_marker is not None: - images = sorted(images, key=lambda image: - getattr(image, 'created_at'), reverse=True) + sort_dir='asc', + sort_key='name', + reversed_order=reversed_order) except Exception: self._prev = False diff --git a/openstack_dashboard/dashboards/project/images/images/tables.py b/openstack_dashboard/dashboards/project/images/images/tables.py index e893b30822..94de39cab2 100644 --- a/openstack_dashboard/dashboards/project/images/images/tables.py +++ b/openstack_dashboard/dashboards/project/images/images/tables.py @@ -222,13 +222,13 @@ class OwnerFilter(tables.FixedFilterAction): def get_image_categories(im, user_tenant_id): categories = [] - if im.is_public: + if api.glance.is_image_public(im): categories.append('public') if im.owner == user_tenant_id: categories.append('project') elif im.owner in filter_tenant_ids(): categories.append(im.owner) - elif not im.is_public: + elif not api.glance.is_image_public(im): categories.append('shared') return categories @@ -339,4 +339,3 @@ class ImagesTable(tables.DataTable): row_actions = launch_actions + (CreateVolumeFromImage, EditImage, UpdateMetadata, DeleteImage,) - pagination_param = "image_marker" diff --git a/openstack_dashboard/dashboards/project/images/tests.py b/openstack_dashboard/dashboards/project/images/tests.py index df57e46ffb..e114ed099a 100644 --- a/openstack_dashboard/dashboards/project/images/tests.py +++ b/openstack_dashboard/dashboards/project/images/tests.py @@ -46,8 +46,9 @@ class ImagesAndSnapshotsTests(test.TestCase): def test_index(self): images = self.images.list() api.glance.image_list_detailed(IsA(http.HttpRequest), - marker=None, - paginate=True) \ + marker=None, paginate=True, + sort_dir='asc', sort_key='name', + reversed_order=False) \ .AndReturn([images, False, False]) self.mox.ReplayAll() @@ -72,8 +73,9 @@ class ImagesAndSnapshotsTests(test.TestCase): @test.create_stubs({api.glance: ('image_list_detailed',)}) def test_index_no_images(self): api.glance.image_list_detailed(IsA(http.HttpRequest), - marker=None, - paginate=True) \ + marker=None, paginate=True, + sort_dir='asc', sort_key='name', + reversed_order=False) \ .AndReturn([(), False, False]) self.mox.ReplayAll() @@ -84,8 +86,9 @@ class ImagesAndSnapshotsTests(test.TestCase): @test.create_stubs({api.glance: ('image_list_detailed',)}) def test_index_error(self): api.glance.image_list_detailed(IsA(http.HttpRequest), - marker=None, - paginate=True) \ + marker=None, paginate=True, + sort_dir='asc', sort_key='name', + reversed_order=False) \ .AndRaise(self.exceptions.glance) self.mox.ReplayAll() @@ -96,8 +99,9 @@ class ImagesAndSnapshotsTests(test.TestCase): def test_snapshot_actions(self): snapshots = self.snapshots.list() api.glance.image_list_detailed(IsA(http.HttpRequest), - marker=None, - paginate=True) \ + marker=None, paginate=True, + sort_dir='asc', sort_key='name', + reversed_order=False) \ .AndReturn([snapshots, False, False]) self.mox.ReplayAll() diff --git a/openstack_dashboard/dashboards/project/images/views.py b/openstack_dashboard/dashboards/project/images/views.py index bc27144593..5cc7f5f91a 100644 --- a/openstack_dashboard/dashboards/project/images/views.py +++ b/openstack_dashboard/dashboards/project/images/views.py @@ -44,14 +44,25 @@ class IndexView(tables.DataTableView): return getattr(self, "_more", False) def get_data(self): - marker = self.request.GET.get( - images_tables.ImagesTable._meta.pagination_param, None) + prev_marker = self.request.GET.get( + images_tables.ImagesTable._meta.prev_pagination_param, None) + + if prev_marker is not None: + marker = prev_marker + else: + marker = self.request.GET.get( + images_tables.ImagesTable._meta.pagination_param, None) + reversed_order = prev_marker is not None try: - (images, self._more, self._prev) = api.glance.image_list_detailed( + images, self._more, self._prev = api.glance.image_list_detailed( self.request, marker=marker, - paginate=True) + paginate=True, + sort_dir='asc', + sort_key='name', + reversed_order=reversed_order) except Exception: images = [] + self._prev = self._more = False exceptions.handle(self.request, _("Unable to retrieve images.")) return images