From 1af4c1ee47ebcf84e63590c27a4d1f4c94c073cb Mon Sep 17 00:00:00 2001 From: William Wolf Date: Tue, 13 Sep 2011 18:16:28 -0400 Subject: Add next links to images requests This adds next links to list images responses in JSON and xml. It keeps all other filters and query parameters in tact to ensure pagination works correctly when using the next links. Change-Id: If61e34589a91f528093c0d05522600d3eee8d89e --- nova/api/openstack/images.py | 29 ++- nova/api/openstack/schemas/v1.1/images_index.rng | 3 + nova/api/openstack/views/images.py | 36 +++ nova/tests/api/openstack/test_images.py | 276 ++++++++++++++++++++++- 4 files changed, 327 insertions(+), 17 deletions(-) diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py index 7795a3fc4..53150ad42 100644 --- a/nova/api/openstack/images.py +++ b/nova/api/openstack/images.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import urlparse import os.path from lxml import etree @@ -149,8 +148,7 @@ class ControllerV10(Controller): filters = self._get_filters(req) images = self._image_service.index(context, filters=filters) images = common.limited(images, req) - builder = self.get_builder(req).build - return dict(images=[builder(image, detail=False) for image in images]) + return self.get_builder(req).build_list(images) def detail(self, req): """Return a detailed index listing of images available to the request. @@ -183,11 +181,14 @@ class ControllerV11(Controller): """ context = req.environ['nova.context'] filters = self._get_filters(req) + params = req.GET.copy() page_params = common.get_pagination_params(req) + for key, val in page_params.iteritems(): + params[key] = val + images = self._image_service.index(context, filters=filters, **page_params) - builder = self.get_builder(req).build - return dict(images=[builder(image, detail=False) for image in images]) + return self.get_builder(req).build_list(images, **params) def detail(self, req): """Return a detailed index listing of images available to the request. @@ -197,11 +198,14 @@ class ControllerV11(Controller): """ context = req.environ['nova.context'] filters = self._get_filters(req) + params = req.GET.copy() page_params = common.get_pagination_params(req) + for key, val in page_params.iteritems(): + params[key] = val images = self._image_service.detail(context, filters=filters, **page_params) - builder = self.get_builder(req).build - return dict(images=[builder(image, detail=True) for image in images]) + + return self.get_builder(req).build_list(images, detail=True, **params) def create(self, *args, **kwargs): raise webob.exc.HTTPMethodNotAllowed() @@ -253,20 +257,23 @@ class ImageXMLSerializer(wsgi.XMLDictSerializer): image_dict.get('metadata', {})) image_elem.append(meta_elem) - for link in image_dict.get('links', []): - elem = etree.SubElement(image_elem, - '{%s}link' % xmlutil.XMLNS_ATOM) + self._populate_links(image_elem, image_dict.get('links', [])) + + def _populate_links(self, parent, links): + for link in links: + elem = etree.SubElement(parent, '{%s}link' % xmlutil.XMLNS_ATOM) elem.set('rel', link['rel']) if 'type' in link: elem.set('type', link['type']) elem.set('href', link['href']) - return image_elem def index(self, images_dict): images = etree.Element('images', nsmap=self.NSMAP) for image_dict in images_dict['images']: image = etree.SubElement(images, 'image') self._populate_image(image, image_dict, False) + + self._populate_links(images, images_dict.get('images_links', [])) return self._to_xml(images) def detail(self, images_dict): diff --git a/nova/api/openstack/schemas/v1.1/images_index.rng b/nova/api/openstack/schemas/v1.1/images_index.rng index 81af19cb5..3db0b2672 100644 --- a/nova/api/openstack/schemas/v1.1/images_index.rng +++ b/nova/api/openstack/schemas/v1.1/images_index.rng @@ -9,4 +9,7 @@ + + + diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index e366661c3..4e8584bad 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -59,6 +59,15 @@ class ViewBuilder(object): """Return an href string pointing to this object.""" return os.path.join(self.base_url, "images", str(image_id)) + def build_list(self, image_objs, detail=False, **kwargs): + """Return a standardized image list structure for display.""" + images = [] + for image_obj in image_objs: + image = self.build(image_obj, detail=detail) + images.append(image) + + return dict(images=images) + def build(self, image_obj, detail=False): """Return a standardized image structure for display by the API.""" self._format_dates(image_obj) @@ -135,6 +144,33 @@ class ViewBuilderV11(ViewBuilder): return os.path.join(self.base_url, self.project_id, "images", str(image_id)) + def generate_next_link(self, image_id, params): + """ Return an href string with proper limit and marker params""" + params['marker'] = image_id + return "%s?%s" % ( + os.path.join(self.base_url, self.project_id, "images"), + common.dict_to_query_str(params)) + + def build_list(self, image_objs, detail=False, **kwargs): + """Return a standardized image list structure for display.""" + limit = kwargs.get('limit', None) + images = [] + images_links = [] + + for image_obj in image_objs: + image = self.build(image_obj, detail=detail) + images.append(image) + + if (len(images) and limit) and (limit == len(images)): + next_link = self.generate_next_link(images[-1]["id"], kwargs) + images_links = [dict(rel="next", href=next_link)] + + reval = dict(images=images) + if len(images_links) > 0: + reval['images_links'] = images_links + + return reval + def build(self, image_obj, detail=False): """Return a standardized image structure for display by the API.""" image = ViewBuilder.build(self, image_obj, detail) diff --git a/nova/tests/api/openstack/test_images.py b/nova/tests/api/openstack/test_images.py index c935003e2..430215a63 100644 --- a/nova/tests/api/openstack/test_images.py +++ b/nova/tests/api/openstack/test_images.py @@ -21,12 +21,12 @@ and as a WSGI layer """ import copy +from lxml import etree import json import xml.dom.minidom as minidom - -from lxml import etree import mox import stubout +import urlparse import webob from nova import context @@ -284,10 +284,12 @@ class ImagesTest(test.TestCase): app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) response = request.get_response(app) + print response.body response_dict = json.loads(response.body) response_list = response_dict["images"] + self.assertTrue('images_links' not in response_dict) - expected = [ + expected_images = [ { "id": "123", "name": "public image", @@ -450,7 +452,118 @@ class ImagesTest(test.TestCase): }, ] - self.assertDictListMatch(response_list, expected) + self.assertDictListMatch(response_list, expected_images) + + def test_get_image_index_v1_1_with_limit(self): + request = webob.Request.blank('/v1.1/fake/images?limit=3') + app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) + response = request.get_response(app) + + print response.body + response_dict = json.loads(response.body) + response_list = response_dict["images"] + response_links = response_dict["images_links"] + alternate = "%s/fake/images/%s" + + expected_images = [ + { + "id": "123", + "name": "public image", + "links": [ + { + "rel": "self", + "href": "http://localhost/v1.1/fake/images/123", + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/images/123", + }, + { + "rel": "alternate", + "type": "application/vnd.openstack.image", + "href": alternate % (utils.generate_glance_url(), 123), + }, + ], + }, + { + "id": "124", + "name": "queued snapshot", + "links": [ + { + "rel": "self", + "href": "http://localhost/v1.1/fake/images/124", + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/images/124", + }, + { + "rel": "alternate", + "type": "application/vnd.openstack.image", + "href": alternate % (utils.generate_glance_url(), 124), + }, + ], + }, + { + "id": "125", + "name": "saving snapshot", + "links": [ + { + "rel": "self", + "href": "http://localhost/v1.1/fake/images/125", + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/images/125", + }, + { + "rel": "alternate", + "type": "application/vnd.openstack.image", + "href": alternate % (utils.generate_glance_url(), 125), + }, + ], + }, + ] + + self.assertDictListMatch(response_list, expected_images) + self.assertEqual(response_links[0]['rel'], 'next') + + href_parts = urlparse.urlparse(response_links[0]['href']) + self.assertEqual('/v1.1/fake/images', href_parts.path) + params = urlparse.parse_qs(href_parts.query) + self.assertDictMatch({'limit': ['3'], 'marker': ['125']}, params) + + def test_get_image_index_v1_1_with_limit_and_extra_params(self): + request = webob.Request.blank('/v1.1/fake/images?limit=3&extra=boo') + app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) + response = request.get_response(app) + + response_dict = json.loads(response.body) + response_links = response_dict["images_links"] + + self.assertEqual(response_links[0]['rel'], 'next') + + href_parts = urlparse.urlparse(response_links[0]['href']) + self.assertEqual('/v1.1/fake/images', href_parts.path) + params = urlparse.parse_qs(href_parts.query) + self.assertDictMatch( + {'limit': ['3'], 'marker': ['125'], 'extra': ['boo']}, + params) + + def test_get_image_index_v1_1_with_big_limit(self): + """ + Make sure we don't get images_links if limit is set + and the number of images returned is < limit + """ + request = webob.Request.blank('/v1.1/fake/images?limit=30') + app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) + response = request.get_response(app) + + response_dict = json.loads(response.body) + response_list = response_dict["images"] + + self.assertTrue('images_links' not in response_dict) + self.assertEqual(len(response_list), 8) def test_get_image_details(self): request = webob.Request.blank('/v1.0/images/detail') @@ -536,6 +649,7 @@ class ImagesTest(test.TestCase): response_list = response_dict["images"] server_href = "http://localhost/v1.1/servers/42" server_bookmark = "http://localhost/servers/42" + alternate = "%s/fake/images/%s" expected = [{ 'id': '123', @@ -558,7 +672,7 @@ class ImagesTest(test.TestCase): { "rel": "alternate", "type": "application/vnd.openstack.image", - "href": "%s/fake/images/123" % utils.generate_glance_url() + "href": alternate % (utils.generate_glance_url(), 123), }], }, { @@ -596,7 +710,7 @@ class ImagesTest(test.TestCase): { "rel": "alternate", "type": "application/vnd.openstack.image", - "href": "%s/fake/images/124" % utils.generate_glance_url() + "href": alternate % (utils.generate_glance_url(), 124), }], }, { @@ -817,6 +931,89 @@ class ImagesTest(test.TestCase): self.assertDictListMatch(expected, response_list) + def test_get_image_details_with_limit_v1_1(self): + request = webob.Request.blank('/v1.1/fake/images/detail?limit=2') + app = fakes.wsgi_app(fake_auth_context=self._get_fake_context()) + response = request.get_response(app) + + response_dict = json.loads(response.body) + response_list = response_dict["images"] + response_links = response_dict["images_links"] + server_href = "http://localhost/v1.1/servers/42" + server_bookmark = "http://localhost/servers/42" + alternate = "%s/fake/images/%s" + + expected = [{ + 'id': '123', + 'name': 'public image', + 'metadata': {'key1': 'value1'}, + 'updated': NOW_API_FORMAT, + 'created': NOW_API_FORMAT, + 'status': 'ACTIVE', + 'minDisk': 0, + 'progress': 100, + 'minRam': 0, + "links": [{ + "rel": "self", + "href": "http://localhost/v1.1/fake/images/123", + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/images/123", + }, + { + "rel": "alternate", + "type": "application/vnd.openstack.image", + "href": alternate % (utils.generate_glance_url(), 123), + }], + }, + { + 'id': '124', + 'name': 'queued snapshot', + 'metadata': { + u'instance_ref': u'http://localhost/v1.1/servers/42', + u'user_id': u'fake', + }, + 'updated': NOW_API_FORMAT, + 'created': NOW_API_FORMAT, + 'status': 'SAVING', + 'minDisk': 0, + 'progress': 0, + 'minRam': 0, + 'server': { + 'id': '42', + "links": [{ + "rel": "self", + "href": server_href, + }, + { + "rel": "bookmark", + "href": server_bookmark, + }], + }, + "links": [{ + "rel": "self", + "href": "http://localhost/v1.1/fake/images/124", + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/images/124", + }, + { + "rel": "alternate", + "type": "application/vnd.openstack.image", + "href": alternate % (utils.generate_glance_url(), 124), + }], + }] + + self.assertDictListMatch(expected, response_list) + + href_parts = urlparse.urlparse(response_links[0]['href']) + self.assertEqual('/v1.1/fake/images', href_parts.path) + params = urlparse.parse_qs(href_parts.query) + + self.assertDictMatch({'limit': ['2'], 'marker': ['124']}, params) + def test_image_filter_with_name(self): image_service = self.mox.CreateMockAnything() context = self._get_fake_context() @@ -1115,6 +1312,7 @@ class ImageXMLSerializationTest(test.TestCase): SERVER_HREF = 'http://localhost/v1.1/servers/123' SERVER_BOOKMARK = 'http://localhost/servers/123' IMAGE_HREF = 'http://localhost/v1.1/fake/images/%s' + IMAGE_NEXT = 'http://localhost/v1.1/fake/images?limit=%s&marker=%s' IMAGE_BOOKMARK = 'http://localhost/fake/images/%s' def test_xml_declaration(self): @@ -1612,6 +1810,72 @@ class ImageXMLSerializationTest(test.TestCase): for key, value in link.items(): self.assertEqual(link_nodes[i].get(key), value) + def test_index_with_links(self): + serializer = images.ImageXMLSerializer() + + fixture = { + 'images': [ + { + 'id': 1, + 'name': 'Image1', + 'links': [ + { + 'href': self.IMAGE_HREF % 1, + 'rel': 'self', + }, + { + 'href': self.IMAGE_BOOKMARK % 1, + 'rel': 'bookmark', + }, + ], + }, + { + 'id': 2, + 'name': 'Image2', + 'links': [ + { + 'href': self.IMAGE_HREF % 2, + 'rel': 'self', + }, + { + 'href': self.IMAGE_BOOKMARK % 2, + 'rel': 'bookmark', + }, + ], + }, + ], + 'images_links': [ + { + 'rel': 'next', + 'href': self.IMAGE_NEXT % (2, 2), + } + ], + } + + output = serializer.serialize(fixture, 'index') + print output + root = etree.XML(output) + xmlutil.validate_schema(root, 'images_index') + image_elems = root.findall('{0}image'.format(NS)) + self.assertEqual(len(image_elems), 2) + for i, image_elem in enumerate(image_elems): + image_dict = fixture['images'][i] + + for key in ['name', 'id']: + self.assertEqual(image_elem.get(key), str(image_dict[key])) + + link_nodes = image_elem.findall('{0}link'.format(ATOMNS)) + self.assertEqual(len(link_nodes), 2) + for i, link in enumerate(image_dict['links']): + for key, value in link.items(): + self.assertEqual(link_nodes[i].get(key), value) + + # Check images_links + images_links = root.findall('{0}link'.format(ATOMNS)) + for i, link in enumerate(fixture['images_links']): + for key, value in link.items(): + self.assertEqual(images_links[i].get(key), value) + def test_index_zero_images(self): serializer = images.ImageXMLSerializer() -- cgit