Include next link when default limit is reached

The /volumes and /volumes/details APIs support pagination and a
"next" link should be included when more data is available. When
the default "osapi_max" limit is reached then the "next" link is
not included in the API reply. In this case, the caller cannot
determine if there are any more volumes and has no marker value
such that they can retrieve the rest of the volumes.

The fix for this is to include the "next" link when the number of
volumes being returned is the maximum limit, even if the "limit"
parameter is not supplied.

Change-Id: I2f04192e67f80232b4019194f718625dbaf78fa6
Closes-bug: 1288429

diff --git a/cinder/api/common.py b/cinder/api/common.py
index 30a32eb7df6febb434fea68a460f196514d4f94b..d38146243c7b4e0a940ae999b7ce6a2a30548f65 100644 (file)
--- a/cinder/api/common.py
+++ b/cinder/api/common.py
@@ -247,10 +247,18 @@ class ViewBuilder(object):
                             str(identifier))
 
     def _get_collection_links(self, request, items, id_key="uuid"):
-        """Retrieve 'next' link, if applicable."""
+        """Retrieve 'next' link, if applicable. This is included if:
+        1) 'limit' param is specified and equals the number of volumes.
+        2) 'limit' param is specified but it exceeds CONF.osapi_max_limit,
+        in this case the number of volumes is CONF.osapi_max_limit.
+        3) 'limit' param is NOT specified but the number of volumes is
+        CONF.osapi_max_limit.
+        """
         links = []
-        limit = int(request.params.get("limit", 0))
-        if limit and limit == len(items):
+        max_items = min(
+            int(request.params.get("limit", CONF.osapi_max_limit)),
+            CONF.osapi_max_limit)
+        if max_items == len(items):
             last_item = items[-1]
             if id_key in last_item:
                 last_item_id = last_item[id_key]

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 18551d5e1187b3f30b04e9b8077b248ae3d1e8b3..9d7333cc8ab9d9108dc15765c7b57df24a0e5ce0 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -747,6 +747,70 @@ class VolumeApiTest(test.TestCase):
                           self.controller.index,
                           req)
 
+    def test_volume_default_limit(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
+        # Number of volumes equals the max, include next link
+        def stub_volume_get_all(context, marker, limit,
+                                sort_key, sort_dir):
+            vols = [stubs.stub_volume(i)
+                    for i in xrange(CONF.osapi_max_limit)]
+            if limit == None or limit >= len(vols):
+                return vols
+            return vols[:limit]
+        self.stubs.Set(db, 'volume_get_all', stub_volume_get_all)
+        for key in ['volumes', 'volumes/detail']:
+            req = fakes.HTTPRequest.blank('/v2/%s?all_tenants=1' % key,
+                                          use_admin_context=True)
+            res_dict = self.controller.index(req)
+            self.assertEqual(len(res_dict['volumes']), CONF.osapi_max_limit)
+            volumes_links = res_dict['volumes_links']
+            self.assertEqual(volumes_links[0]['rel'], 'next')
+
+        # Number of volumes less then max, do not include
+        def stub_volume_get_all2(context, marker, limit,
+                                 sort_key, sort_dir):
+            vols = [stubs.stub_volume(i)
+                    for i in xrange(100)]
+            if limit == None or limit >= len(vols):
+                return vols
+            return vols[:limit]
+        self.stubs.Set(db, 'volume_get_all', stub_volume_get_all2)
+        for key in ['volumes', 'volumes/detail']:
+            req = fakes.HTTPRequest.blank('/v2/%s?all_tenants=1' % key,
+                                          use_admin_context=True)
+            res_dict = self.controller.index(req)
+            self.assertEqual(len(res_dict['volumes']), 100)
+            self.assertFalse('volumes_links' in res_dict)
+
+        # Number of volumes more then the max, include next link
+        def stub_volume_get_all3(context, marker, limit,
+                                 sort_key, sort_dir):
+            vols = [stubs.stub_volume(i)
+                    for i in xrange(CONF.osapi_max_limit + 100)]
+            if limit == None or limit >= len(vols):
+                return vols
+            return vols[:limit]
+        self.stubs.Set(db, 'volume_get_all', stub_volume_get_all3)
+        for key in ['volumes', 'volumes/detail']:
+            req = fakes.HTTPRequest.blank('/v2/%s?all_tenants=1' % key,
+                                          use_admin_context=True)
+            res_dict = self.controller.index(req)
+            self.assertEqual(len(res_dict['volumes']), CONF.osapi_max_limit)
+            volumes_links = res_dict['volumes_links']
+            self.assertEqual(volumes_links[0]['rel'], 'next')
+        # Pass a limit that is greater then the max and the total number of
+        # volumes, ensure only the maximum is returned and that the next
+        # link is present
+        for key in ['volumes', 'volumes/detail']:
+            req = fakes.HTTPRequest.blank('/v2/%s?all_tenants=1&limit=%d'
+                                          % (key, CONF.osapi_max_limit * 2),
+                                          use_admin_context=True)
+            res_dict = self.controller.index(req)
+            self.assertEqual(len(res_dict['volumes']), CONF.osapi_max_limit)
+            volumes_links = res_dict['volumes_links']
+            self.assertEqual(volumes_links[0]['rel'], 'next')
+
     def test_volume_list_by_name(self):
         def stub_volume_get_all_by_project(context, project_id, marker, limit,
                                            sort_key, sort_dir):