added get_pagination_params function in common with tests, allow fake and local image services to accept filters, markers, and limits (but ignore them for now)

8 files changed, 113 insertions, 14 deletions

diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py
index 32cd689ca..69877cbce 100644
--- a/nova/api/openstack/common.py
+++ b/nova/api/openstack/common.py
@@ -36,6 +36,37 @@ XML_NS_V10 = 'http://docs.rackspacecloud.com/servers/api/v1.0'
 XML_NS_V11 = 'http://docs.openstack.org/compute/api/v1.1'
 
 
+def get_pagination_params(request):
+    """
+    Return marker, limit tuple from request
+    
+    @param request: `wsgi.Request` possibly containing 'marker' and 'limit'
+                    GET variables. 'marker' is the id of the last element
+                    the client has seen, and 'limit' is the maximum number
+                    of items to return. If 'limit' is not specified, 0, or
+                    > max_limit, we default to max_limit. Negative values
+                    for either marker or limit will cause 
+                    exc.HTTPBadRequest() exceptions to be raised.
+    """
+    try:
+        marker = int(request.GET.get('marker', 0))
+    except ValueError:
+        raise webob.exc.HTTPBadRequest(_('offset param must be an integer'))
+
+    try:
+        limit = int(request.GET.get('limit', 0))
+    except ValueError:
+        raise webob.exc.HTTPBadRequest(_('limit param must be an integer'))
+
+    if limit < 0:
+        raise webob.exc.HTTPBadRequest(_('limit param must be positive'))
+
+    if marker < 0:
+        raise webob.exc.HTTPBadRequest(_('marker param must be positive'))
+
+    return(marker, limit)
+
+
 def limited(items, request, max_limit=FLAGS.osapi_max_limit):
     """
     Return a slice of items according to requested offset and limit.
diff --git a/nova/api/openstack/images.py b/nova/api/openstack/images.py
index c96b1c3e3..afe0f79de 100644
--- a/nova/api/openstack/images.py
+++ b/nova/api/openstack/images.py
@@ -74,7 +74,7 @@ class Controller(common.OpenstackController):
         """
         context = req.environ['nova.context']
         images = self._image_service.detail(context)
-        images = self._limited_items(images, req)
+        images = self._limit_items(images, req)
         builder = self.get_builder(req).build
         return dict(images=[builder(image, detail=True) for image in images])
 
@@ -157,5 +157,24 @@ class ControllerV11(Controller):
     def get_default_xmlns(self, req):
         return common.XML_NS_V11
 
-    def _limit_items(self, items, req):
-        return common.limited_by_marker(items, req)
+    def index(self, req):
+        """Return an index listing of images available to the request.
+
+        :param req: `wsgi.Request` object
+        """
+        context = req.environ['nova.context']
+        (marker, limit) = common.get_pagination_params(req)
+        images = self._image_service.index(context, marker, limit)
+        builder = self.get_builder(req).build
+        return dict(images=[builder(image, detail=False) for image in images])
+
+    def detail(self, req):
+        """Return a detailed index listing of images available to the request.
+
+        :param req: `wsgi.Request` object.
+        """
+        context = req.environ['nova.context']
+        (marker, limit) = common.get_pagination_params(req)
+        images = self._image_service.detail(context, marker, limit)
+        builder = self.get_builder(req).build
+        return dict(images=[builder(image, detail=True) for image in images])
diff --git a/nova/image/fake.py b/nova/image/fake.py
index b400b2adb..4aa4219fe 100644
--- a/nova/image/fake.py
+++ b/nova/image/fake.py
@@ -52,11 +52,11 @@ class FakeImageService(service.BaseImageService):
         self.create(None, image)
         super(FakeImageService, self).__init__()
 
-    def index(self, context):
+    def index(self, context, filters=None, marker=None, limit=None):
         """Returns list of images."""
         return copy.deepcopy(self.images.values())
 
-    def detail(self, context):
+    def detail(self, context, filters=None, marker=None, limit=None):
         """Return list of detailed image information."""
         return copy.deepcopy(self.images.values())
 
diff --git a/nova/image/glance.py b/nova/image/glance.py
index 193e37273..e084ed8ae 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -58,23 +58,25 @@ class GlanceImageService(service.BaseImageService):
         else:
             self.client = client
 
-    def index(self, context):
+    def index(self, context, marker=None, limit=None):
         """Calls out to Glance for a list of images available."""
         # NOTE(sirp): We need to use `get_images_detailed` and not
         # `get_images` here because we need `is_public` and `properties`
         # included so we can filter by user
         filtered = []
-        image_metas = self.client.get_images_detailed()
+        image_metas = self.client.get_images_detailed(
+            marker=marker, limit=limit)
         for image_meta in image_metas:
             if self._is_image_available(context, image_meta):
                 meta_subset = utils.subset_dict(image_meta, ('id', 'name'))
                 filtered.append(meta_subset)
         return filtered
 
-    def detail(self, context):
+    def detail(self, context, marker=None, limit=None):
         """Calls out to Glance for a list of detailed image information."""
         filtered = []
-        image_metas = self.client.get_images_detailed()
+        image_metas = self.client.get_images_detailed(
+            marker=marker, limit=limit)
         for image_meta in image_metas:
             if self._is_image_available(context, image_meta):
                 base_image_meta = self._translate_to_base(image_meta)
diff --git a/nova/image/local.py b/nova/image/local.py
index 918180bae..f320cc60c 100644
--- a/nova/image/local.py
+++ b/nova/image/local.py
@@ -63,7 +63,7 @@ class LocalImageService(service.BaseImageService):
                 images.append(unhexed_image_id)
         return images
 
-    def index(self, context):
+    def index(self, context, filters=None, marker=None, limit=None):
         filtered = []
         image_metas = self.detail(context)
         for image_meta in image_metas:
@@ -71,7 +71,7 @@ class LocalImageService(service.BaseImageService):
             filtered.append(meta)
         return filtered
 
-    def detail(self, context):
+    def detail(self, context, filters=None, marker=None, limit=None):
         images = []
         for image_id in self._ids():
             try:
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index bf51239e6..2e28e421c 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -166,11 +166,12 @@ def stub_out_glance(stubs, initial_fixtures=None):
         def __init__(self, initial_fixtures):
             self.fixtures = initial_fixtures or []
 
-        def fake_get_images(self):
+        def fake_get_images(self, filters=None, marker=None, limit=None):
             return [dict(id=f['id'], name=f['name'])
                     for f in self.fixtures]
 
-        def fake_get_images_detailed(self):
+        def fake_get_images_detailed(self, filters=None, 
+                                     marker=None, limit=None):
             return copy.deepcopy(self.fixtures)
 
         def fake_get_image_meta(self, image_id):
diff --git a/nova/tests/api/openstack/test_common.py b/nova/tests/api/openstack/test_common.py
index 8f57c5b67..34597c7ac 100644
--- a/nova/tests/api/openstack/test_common.py
+++ b/nova/tests/api/openstack/test_common.py
@@ -25,6 +25,7 @@ from webob import Request
 
 from nova import test
 from nova.api.openstack.common import limited
+from nova.api.openstack.common import get_pagination_params
 
 
 class LimiterTest(test.TestCase):
@@ -169,3 +170,48 @@ class LimiterTest(test.TestCase):
         """
         req = Request.blank('/?offset=-30')
         self.assertRaises(webob.exc.HTTPBadRequest, limited, self.tiny, req)
+
+
+class PaginationParamsTest(test.TestCase):
+    """
+    Unit tests for the `nova.api.openstack.common.get_pagination_params` 
+    method which takes in a request object and returns 'marker' and 'limit' 
+    GET params.
+    """
+
+    def test_no_params(self):
+        """
+        Test no params.
+        """
+        req = Request.blank('/')
+        self.assertEqual(get_pagination_params(req), (0, 0))
+        
+    def test_valid_marker(self):
+        """
+        Test valid marker param.
+        """
+        req = Request.blank('/?marker=1')
+        self.assertEqual(get_pagination_params(req), (1, 0))
+
+    def test_invalid_marker(self):
+        """
+        Test invalid marker param.
+        """
+        req = Request.blank('/?marker=-2')
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          get_pagination_params, req)
+
+    def test_valid_limit(self):
+        """
+        Test valid limit param.
+        """
+        req = Request.blank('/?limit=10')
+        self.assertEqual(get_pagination_params(req), (0, 10))
+
+    def test_invalid_limit(self):
+        """
+        Test invalid limit param.
+        """
+        req = Request.blank('/?limit=-2')
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          get_pagination_params, req)
diff --git a/nova/tests/image/test_glance.py b/nova/tests/image/test_glance.py
index 109905ded..041da1e13 100644
--- a/nova/tests/image/test_glance.py
+++ b/nova/tests/image/test_glance.py
@@ -34,7 +34,7 @@ class StubGlanceClient(object):
     def get_image_meta(self, image_id):
         return self.images[image_id]
 
-    def get_images_detailed(self):
+    def get_images_detailed(self, filters=None, marker=None, limit=None):
         return self.images.itervalues()
 
     def get_image(self, image_id):