From b63c28b7276d4c320fde8f481f61611d28278900 Mon Sep 17 00:00:00 2001
From: Steven Kaufer <kaufer@us.ibm.com>
Date: Thu, 6 Mar 2014 17:20:50 +0000
Subject: [PATCH] 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
---
 cinder/api/common.py                | 14 +++++--
 cinder/tests/api/v2/test_volumes.py | 64 +++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/cinder/api/common.py b/cinder/api/common.py
index 30a32eb7d..d38146243 100644
--- 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 18551d5e1..9d7333cc8 100644
--- 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):
-- 
2.45.2