Skip to content

Commit

Permalink
Fix images/snapshots table pagination
Browse files Browse the repository at this point in the history 
For project images, disable pagination entirely as it's not compatible
with client-side filtering (at least without moving to asynch table
loading). This can be revisited when we move to glance v2, which
apparently supports filtering by owner.

For snapshots and admin images, fix pagination by making sure the
iterator returned by glanceclient isn't exhausted completely when
returning a page of images. Do this by slicing just the number of
images we require (+ 1 to check if there will be another page after
this one).

Fixes bug #1083034.

Change-Id: I692f57dec6dca16cf8909f5b234aadb002702fc8
  • Loading branch information
@spearki
spearki committed Feb 20, 2013
1 parent 3392d49 commit c822a9f
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 41 deletions.
28 changes: 20 additions & 8 deletions openstack_dashboard/api/glance.py
View file
Expand Up @@ -20,6 +20,7 @@

from __future__ import absolute_import

import itertools
import logging
import thread
import urlparse
Expand Down Expand Up @@ -56,20 +57,31 @@ def image_get(request, image_id):
return glanceclient(request).images.get(image_id)


def image_list_detailed(request, marker=None, filters=None):
def image_list_detailed(request, marker=None, filters=None, paginate=False):
limit = getattr(settings, 'API_RESULT_LIMIT', 1000)
page_size = getattr(settings, 'API_RESULT_PAGE_SIZE', 20)

if paginate:
request_size = page_size + 1
else:
request_size = limit

kwargs = {'filters': filters or {}}
if marker:
kwargs['marker'] = marker
images = list(glanceclient(request).images.list(page_size=page_size,

images_iter = glanceclient(request).images.list(page_size=request_size,
limit=limit,
**kwargs))
# Glance returns (page_size + 1) items if more items are available
if(len(images) > page_size):
return (images[0:-1], True)
**kwargs)
has_more_data = False
if paginate:
images = list(itertools.islice(images_iter, request_size))
if len(images) > page_size:
images.pop(-1)
has_more_data = True
else:
return (images, False)
images = list(images_iter)
return (images, has_more_data)


def image_update(request, image_id, **kwargs):
Expand All @@ -95,4 +107,4 @@ def image_create(request, **kwargs):
def snapshot_list_detailed(request, marker=None, extra_filters=None):
filters = {'property-image_type': 'snapshot'}
filters.update(extra_filters or {})
return image_list_detailed(request, marker, filters)
return image_list_detailed(request, marker, filters, paginate=True)
26 changes: 12 additions & 14 deletions openstack_dashboard/dashboards/admin/images/tests.py
View file
Expand Up @@ -17,6 +17,7 @@
from django import http
from django.conf import settings
from django.core.urlresolvers import reverse
from django.test.utils import override_settings
from mox import IsA

from openstack_dashboard import api
Expand All @@ -35,7 +36,8 @@ class ImagesViewTest(test.BaseAdminViewTests):
@test.create_stubs({api.glance: ('image_list_detailed',)})
def test_images_list(self):
api.glance.image_list_detailed(IsA(http.HttpRequest),
marker=None) \
marker=None,
paginate=True) \
.AndReturn([self.images.list(),
False])
self.mox.ReplayAll()
Expand All @@ -46,24 +48,29 @@ def test_images_list(self):
self.assertEqual(len(res.context['images_table'].data),
len(self.images.list()))

@override_settings(API_RESULT_PAGE_SIZE=2)
@test.create_stubs({api.glance: ('image_list_detailed',)})
def test_images_list_get_pagination(self):
images = self.images.list()[:5]

api.glance.image_list_detailed(IsA(http.HttpRequest),
marker=None) \
marker=None,
paginate=True) \
.AndReturn([images,
True])
api.glance.image_list_detailed(IsA(http.HttpRequest),
marker=None) \
marker=None,
paginate=True) \
.AndReturn([images[:2],
True])
api.glance.image_list_detailed(IsA(http.HttpRequest),
marker=images[2].id) \
marker=images[2].id,
paginate=True) \
.AndReturn([images[2:4],
True])
api.glance.image_list_detailed(IsA(http.HttpRequest),
marker=images[4].id) \
marker=images[4].id,
paginate=True) \
.AndReturn([images[4:],
True])
self.mox.ReplayAll()
Expand All @@ -75,9 +82,6 @@ def test_images_list_get_pagination(self):
len(images))
self.assertTemplateUsed(res, 'admin/images/index.html')

page_size = getattr(settings, "API_RESULT_PAGE_SIZE", None)
settings.API_RESULT_PAGE_SIZE = 2

res = self.client.get(url)
# get first page with 2 items
self.assertEqual(len(res.context['images_table'].data),
Expand All @@ -98,9 +102,3 @@ def test_images_list_get_pagination(self):
# get third page (item 5)
self.assertEqual(len(res.context['images_table'].data),
1)

# restore API_RESULT_PAGE_SIZE
if page_size:
settings.API_RESULT_PAGE_SIZE = page_size
else:
del settings.API_RESULT_PAGE_SIZE
3 changes: 2 additions & 1 deletion openstack_dashboard/dashboards/admin/images/views.py
View file
Expand Up @@ -49,7 +49,8 @@ def get_data(self):
None)
try:
images, self._more = api.glance.image_list_detailed(self.request,
marker=marker)
marker=marker,
paginate=True)
except:
self._more = False
msg = _('Unable to retrieve image list.')
Expand Down
90 changes: 72 additions & 18 deletions openstack_dashboard/test/api_tests/glance_tests.py
View file
Expand Up @@ -19,49 +19,103 @@
# under the License.

from django.conf import settings
from django.test.utils import override_settings

from openstack_dashboard import api
from openstack_dashboard.test import helpers as test


class GlanceApiTests(test.APITestCase):
@override_settings(API_RESULT_PAGE_SIZE=2)
def test_image_list_detailed_no_pagination(self):
# Verify that all images are returned even with a small page size
api_images = self.images.list()
filters = {}
limit = getattr(settings, 'API_RESULT_LIMIT', 1000)

glanceclient = self.stub_glanceclient()
glanceclient.images = self.mox.CreateMockAnything()
glanceclient.images.list(page_size=limit,
limit=limit,
filters=filters,).AndReturn(iter(api_images))
self.mox.ReplayAll()

images, has_more = api.glance.image_list_detailed(self.request)
self.assertItemsEqual(images, api_images)
self.assertFalse(has_more)

def test_snapshot_list_detailed(self):
images = self.images.list()
# The total image count is under page size, should return all images.
api_images = self.images.list()
filters = {'property-image_type': 'snapshot'}
limit = getattr(settings, 'API_RESULT_LIMIT', 1000)
page_size = getattr(settings, 'API_RESULT_PAGE_SIZE', 20)

glanceclient = self.stub_glanceclient()
glanceclient.images = self.mox.CreateMockAnything()
glanceclient.images.list(page_size=page_size,
glanceclient.images.list(page_size=page_size + 1,
limit=limit,
filters=filters,).AndReturn(images)
filters=filters,).AndReturn(iter(api_images))
self.mox.ReplayAll()

# No assertions are necessary. Verification is handled by mox.
api.glance.snapshot_list_detailed(self.request)
images, has_more = api.glance.snapshot_list_detailed(self.request)
self.assertItemsEqual(images, api_images)
self.assertFalse(has_more)

@override_settings(API_RESULT_PAGE_SIZE=2)
def test_snapshot_list_detailed_pagination(self):
images = self.images.list()
# The total snapshot count is over page size, should return
# page_size images.
filters = {'property-image_type': 'snapshot'}
page_size = 2
temp_page_size = getattr(settings, 'API_RESULT_PAGE_SIZE', None)
settings.API_RESULT_PAGE_SIZE = page_size
page_size = settings.API_RESULT_PAGE_SIZE
limit = getattr(settings, 'API_RESULT_LIMIT', 1000)

api_images = self.images.list()
images_iter = iter(api_images)

glanceclient = self.stub_glanceclient()
glanceclient.images = self.mox.CreateMockAnything()
# Pass back all images, ignoring filters
glanceclient.images.list(limit=limit,
page_size=page_size,
page_size=page_size + 1,
filters=filters,) \
.AndReturn(images[0:page_size])
.AndReturn(images_iter)
self.mox.ReplayAll()

# No assertions are necessary. Verification is handled by mox.
api.glance.snapshot_list_detailed(self.request)
images, has_more = api.glance.snapshot_list_detailed(self.request)
expected_images = api_images[:page_size]
self.assertItemsEqual(images, expected_images)
self.assertTrue(has_more)
# Ensure that only the needed number of images are consumed
# from the iterator (page_size + 1).
self.assertEqual(len(list(images_iter)),
len(api_images) - len(expected_images) - 1)

@override_settings(API_RESULT_PAGE_SIZE=2)
def test_snapshot_list_detailed_pagination_marker(self):
# Tests getting a second page with a marker.
filters = {'property-image_type': 'snapshot'}
page_size = settings.API_RESULT_PAGE_SIZE
limit = getattr(settings, 'API_RESULT_LIMIT', 1000)
marker = 'nonsense'

api_images = self.images.list()[page_size:]
images_iter = iter(api_images)

glanceclient = self.stub_glanceclient()
glanceclient.images = self.mox.CreateMockAnything()
# Pass back all images, ignoring filters
glanceclient.images.list(limit=limit,
page_size=page_size + 1,
filters=filters,
marker=marker) \
.AndReturn(images_iter)
self.mox.ReplayAll()

# Restore
if temp_page_size:
settings.API_RESULT_PAGE_SIZE = temp_page_size
else:
del settings.API_RESULT_PAGE_SIZE
images, has_more = api.glance.snapshot_list_detailed(self.request,
marker=marker)
expected_images = api_images[:page_size]
self.assertItemsEqual(images, expected_images)
self.assertTrue(has_more)
self.assertEqual(len(list(images_iter)),
len(api_images) - len(expected_images) - 1)

0 comments on commit c822a9f

Please sign in to comment.