diff --git a/openstack_dashboard/api/glance.py b/openstack_dashboard/api/glance.py index 9e0666fad..cedf0b705 100644 --- a/openstack_dashboard/api/glance.py +++ b/openstack_dashboard/api/glance.py @@ -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) diff --git a/openstack_dashboard/dashboards/admin/images/tests.py b/openstack_dashboard/dashboards/admin/images/tests.py index 0a601a4f4..0dee4e496 100644 --- a/openstack_dashboard/dashboards/admin/images/tests.py +++ b/openstack_dashboard/dashboards/admin/images/tests.py @@ -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 diff --git a/openstack_dashboard/dashboards/admin/images/views.py b/openstack_dashboard/dashboards/admin/images/views.py index f87705ce1..2f052507a 100644 --- a/openstack_dashboard/dashboards/admin/images/views.py +++ b/openstack_dashboard/dashboards/admin/images/views.py @@ -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.') diff --git a/openstack_dashboard/test/api_tests/glance_tests.py b/openstack_dashboard/test/api_tests/glance_tests.py index f5b8a0bdb..838f39ed6 100644 --- a/openstack_dashboard/test/api_tests/glance_tests.py +++ b/openstack_dashboard/test/api_tests/glance_tests.py @@ -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)