Fix images/snapshots table pagination
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
This commit is contained in:
parent
3392d492cd
commit
c822a9f16e
openstack_dashboard
@ -20,6 +20,7 @@
|
||||
|
||||
from __future__ import absolute_import
|
||||
|
||||
import itertools
|
||||
import logging
|
||||
import thread
|
||||
import urlparse
|
||||
@ -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):
|
||||
@ -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)
|
||||
|
@ -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
|
||||
@ -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()
|
||||
@ -46,24 +48,29 @@ class ImagesViewTest(test.BaseAdminViewTests):
|
||||
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()
|
||||
@ -75,9 +82,6 @@ class ImagesViewTest(test.BaseAdminViewTests):
|
||||
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),
|
||||
@ -98,9 +102,3 @@ class ImagesViewTest(test.BaseAdminViewTests):
|
||||
# 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
|
||||
|
@ -49,7 +49,8 @@ class IndexView(tables.DataTableView):
|
||||
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.')
|
||||
|
@ -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)
|
||||
|
||||
# Restore
|
||||
if temp_page_size:
|
||||
settings.API_RESULT_PAGE_SIZE = temp_page_size
|
||||
else:
|
||||
del settings.API_RESULT_PAGE_SIZE
|
||||
@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()
|
||||
|
||||
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)
|
||||
|
Loading…
x
Reference in New Issue
Block a user