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

@@ -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):

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,
+                                       **kwargs) \
-                                       filters=filters,
-                                       sort_dir='desc') \
             .AndReturn([images, True, True])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=None,
-                                       paginate=True,
+                                       **kwargs) \
-                                       filters=filters,
-                                       sort_dir='desc') \
             .AndReturn([images[:2], True, True])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=images[2].id,
-                                       paginate=True,
+                                       **kwargs) \
-                                       filters=filters,
-                                       sort_dir='desc') \
             .AndReturn([images[2:4], True, True])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=images[4].id,
-                                       paginate=True,
+                                       **kwargs) \
-                                       filters=filters,
-                                       sort_dir='desc') \
             .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,
+                                       reversed_order=False,
-                                       filters=filters,
+                                       **kwargs) \
-                                       sort_dir='desc') \
             .AndReturn([images, True, False])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=None,
-                                       paginate=True,
+                                       reversed_order=False,
-                                       filters=filters,
+                                       **kwargs) \
-                                       sort_dir='desc') \
             .AndReturn([images[:2], True, True])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=images[2].id,
-                                       paginate=True,
+                                       reversed_order=False,
-                                       filters=filters,
+                                       **kwargs) \
-                                       sort_dir='desc') \
             .AndReturn([images[2:], True, True])
         api.glance.image_list_detailed(IsA(http.HttpRequest),
                                        marker=images[2].id,
-                                       paginate=True,
+                                       reversed_order=True,
-                                       filters=filters,
+                                       **kwargs) \
-                                       sort_dir='asc') \
             .AndReturn([images[:2], True, True])
         # Test tenant list
         api.keystone.tenant_list(IsA(http.HttpRequest)).MultipleTimes().\

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)
+                sort_dir='asc',
-
+                sort_key='name',
-            if prev_marker is not None:
+                reversed_order=reversed_order)
-                images = sorted(images, key=lambda image:
-                                getattr(image, 'created_at'), reverse=True)
 
         except Exception:
             self._prev = False

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"

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,
+                                       marker=None, paginate=True,
-                                       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,
+                                       marker=None, paginate=True,
-                                       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,
+                                       marker=None, paginate=True,
-                                       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,
+                                       marker=None, paginate=True,
-                                       paginate=True) \
+                                       sort_dir='asc', sort_key='name',
+                                       reversed_order=False) \
             .AndReturn([snapshots, False, False])
         self.mox.ReplayAll()
 

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(
+        prev_marker = self.request.GET.get(
-            images_tables.ImagesTable._meta.pagination_param, None)
+            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