Merge "Add next links to images requests"

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 @@
       </zeroOrMore>
     </element>
   </zeroOrMore>
+  <zeroOrMore>
+    <externalRef href="../atom-link.rng"/>
+  </zeroOrMore>
 </element>
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()