Do not paginate stack list unless page_size is set

Pagination on the server has not yet been implemented, however
the client assumes it is, causing infinite recursion when paginate
is called.

With this change, no pagination is attempted unless page_size is
specified. However the 'limit' parameter is still honored, so it
is still possible to cap the total number of results returned.

heatclient users should not set page_size until pagination has been
implemented on the server. The stack list in Horizon currently
does not attempt to paginate, so is not affected.

Fixes: bug #1207839
Change-Id: I4087d3a8af48206d6ebe3edc441469464e4a401a

4 files changed, 234 insertions, 12 deletions

diff --git a/heatclient/tests/fakes.py b/heatclient/tests/fakes.py
index 732bd1f..199dad9 100644
--- a/heatclient/tests/fakes.py
+++ b/heatclient/tests/fakes.py
@@ -45,8 +45,7 @@ def script_heat_list():
                             {'content-type': 'application/json'},
                             json.dumps(resp_dict))
     v1client.Client.json_request('GET',
-                                 '/stacks?limit=20').AndReturn((resp,
-                                                                resp_dict))
+                                 '/stacks?').AndReturn((resp, resp_dict))
 
 
 def script_heat_normal_error():
diff --git a/heatclient/tests/test_shell.py b/heatclient/tests/test_shell.py
index 9a07e0c..e28e841 100644
--- a/heatclient/tests/test_shell.py
+++ b/heatclient/tests/test_shell.py
@@ -166,7 +166,7 @@ class ShellValidationTest(TestCase):
         self.m.StubOutWithMock(v1client.Client, 'json_request')
         fakes.script_keystone_client()
         v1client.Client.json_request(
-            'GET', '/stacks?limit=20').AndRaise(exc.Unauthorized)
+            'GET', '/stacks?').AndRaise(exc.Unauthorized)
 
         self.m.ReplayAll()
         fake_env = {
diff --git a/heatclient/tests/test_stacks.py b/heatclient/tests/test_stacks.py
index f2a4b96..fe0f60b 100644
--- a/heatclient/tests/test_stacks.py
+++ b/heatclient/tests/test_stacks.py
@@ -14,16 +14,32 @@
 #    under the License.
 
 from heatclient.v1.stacks import Stack
+from heatclient.v1.stacks import StackManager
 
 from mock import MagicMock
 import testscenarios
 from testscenarios.scenarios import multiply_scenarios
 import testtools
 
-
 load_tests = testscenarios.load_tests_apply_scenarios
 
 
+def mock_stack(manager, stack_name, stack_id):
+    return Stack(manager, {
+        "id": stack_id,
+        "stack_name": stack_name,
+        "links": [{
+            "href": "http://192.0.2.1:8004/v1/1234/stacks/%s/%s" % (
+                stack_name, stack_id),
+            "rel": "self"}],
+        "description": "No description",
+        "stack_status_reason": "Stack create completed successfully",
+        "creation_time": "2013-08-04T20:57:55Z",
+        "updated_time": "2013-08-04T20:57:55Z",
+        "stack_status": "CREATE_COMPLETE"
+    })
+
+
 class StackStatusActionTest(testtools.TestCase):
 
     scenarios = multiply_scenarios([
@@ -41,7 +57,8 @@ class StackStatusActionTest(testtools.TestCase):
 
     def test_status_action(self):
         stack_status = '%s_%s' % (self.action, self.status)
-        stack = Stack(None, {'stack_status': stack_status})
+        stack = mock_stack(None, 'stack_1', 'abcd1234')
+        stack.stack_status = stack_status
         self.assertEqual(self.action, stack.action)
         self.assertEqual(self.status, stack.status)
 
@@ -50,12 +67,218 @@ class StackOperationsTest(testtools.TestCase):
 
     def test_delete_stack(self):
         manager = MagicMock()
-        stack = Stack(manager, {'id': 'abcd1234'})
+        stack = mock_stack(manager, None, 'abcd1234')
         stack.delete()
         manager.delete.assert_called_once_with('abcd1234')
 
     def test_get_stack(self):
         manager = MagicMock()
-        stack = Stack(manager, {'id': 'abcd1234', 'stack_name': 'the_stack'})
+        stack = mock_stack(manager, 'the_stack', 'abcd1234')
         stack.get()
         manager.get.assert_called_once_with('the_stack/abcd1234')
+
+
+class StackManagerNoPaginationTest(testtools.TestCase):
+
+    scenarios = [
+        ('total_0', dict(total=0)),
+        ('total_1', dict(total=1)),
+        ('total_9', dict(total=9)),
+        ('total_1', dict(total=10)),
+        ('total_11', dict(total=11)),
+        ('total_19', dict(total=19)),
+        ('total_20', dict(total=20)),
+        ('total_21', dict(total=21)),
+        ('total_49', dict(total=49)),
+        ('total_50', dict(total=50)),
+        ('total_51', dict(total=51)),
+        ('total_95', dict(total=95)),
+    ]
+
+    # absolute limit for results returned
+    limit = 50
+
+    def mock_manager(self):
+        manager = StackManager(None)
+        manager._list = MagicMock()
+
+        def mock_list(*args, **kwargs):
+            def results():
+                for i in range(0, self.total):
+                    stack_name = 'stack_%s' % (i + 1)
+                    stack_id = 'abcd1234-%s' % (i + 1)
+                    yield mock_stack(manager, stack_name, stack_id)
+
+            return list(results())
+
+        manager._list.side_effect = mock_list
+        return manager
+
+    def test_stack_list_no_pagination(self):
+        manager = self.mock_manager()
+        results = list(manager.list(limit=self.limit))
+        manager._list.assert_called_once_with(
+            '/stacks?', 'stacks')
+
+        last_result = min(self.limit, self.total)
+        # paginate is not specified, so the total
+        # results (up to the limit) is always returned
+        self.assertEqual(last_result, len(results))
+
+        if last_result > 0:
+            self.assertEqual('stack_1', results[0].stack_name)
+            self.assertEqual('stack_%s' % last_result, results[-1].stack_name)
+
+    def test_stack_list_no_pagination_no_limit(self):
+        manager = self.mock_manager()
+
+        results = list(manager.list())
+        manager._list.assert_called_once_with(
+            '/stacks?', 'stacks')
+
+        # paginate is not specified, so the total
+        # results is always returned
+        self.assertEqual(self.total, len(results))
+
+        if self.total > 0:
+            self.assertEqual('stack_1', results[0].stack_name)
+            self.assertEqual('stack_%s' % self.total, results[-1].stack_name)
+
+
+class StackManagerPaginationTest(testtools.TestCase):
+
+    scenarios = [
+        ('0_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=0,
+            results=((0, 0),)
+        )),
+        ('1_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=1,
+            results=((0, 1),)
+        )),
+        ('9_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=9,
+            results=((0, 9),)
+        )),
+        ('10_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=10,
+            results=((0, 10), (10, 10))
+        )),
+        ('11_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=11,
+            results=((0, 10), (10, 11))
+        )),
+        ('11_offset_10', dict(
+            page_size=10,
+            offset=10,
+            total=11,
+            results=((10, 11),)
+        )),
+        ('19_offset_10', dict(
+            page_size=10,
+            offset=10,
+            total=19,
+            results=((10, 19),)
+        )),
+        ('20_offset_10', dict(
+            page_size=10,
+            offset=10,
+            total=20,
+            results=((10, 20), (20, 20))
+        )),
+        ('21_offset_10', dict(
+            page_size=10,
+            offset=10,
+            total=21,
+            results=((10, 20), (20, 21))
+        )),
+        ('21_offset_0', dict(
+            page_size=10,
+            offset=0,
+            total=21,
+            results=((0, 10), (10, 20), (20, 21))
+        )),
+        ('21_offset_20', dict(
+            page_size=10,
+            offset=20,
+            total=21,
+            results=((20, 21),)
+        )),
+        ('95_offset_90', dict(
+            page_size=10,
+            offset=90,
+            total=95,
+            results=((90, 95),)
+        )),
+    ]
+
+    # absolute limit for results returned
+    limit = 50
+
+    def mock_manager(self):
+        manager = StackManager(None)
+        manager._list = MagicMock()
+
+        def mock_list(arg_url, arg_response_key):
+            try:
+                result = self.results[self.result_index]
+            except IndexError:
+                return
+            self.result_index = self.result_index + 1
+
+            offset = result[0]
+            url = '/stacks?'
+            if offset > 0:
+                url += 'marker=abcd1234-%s&' % offset
+            url += 'limit=%s' % self.page_size
+            self.assertEqual(url, arg_url)
+
+            def results():
+
+                for i in range(*result):
+                    stack_name = 'stack_%s' % (i + 1)
+                    stack_id = 'abcd1234-%s' % (i + 1)
+                    yield mock_stack(manager, stack_name, stack_id)
+
+            return list(results())
+
+        manager._list.side_effect = mock_list
+        return manager
+
+    def test_stack_list_pagination(self):
+        manager = self.mock_manager()
+
+        list_params = {
+            'page_size': self.page_size,
+            'limit': self.limit
+        }
+
+        if self.offset > 0:
+            marker = 'abcd1234-%s' % self.offset
+            list_params['marker'] = marker
+
+        self.result_index = 0
+        results = list(manager.list(**list_params))
+
+        # assert that the list method has been called enough times
+        self.assertEqual(len(self.results), self.result_index)
+
+        last_result = min(self.limit, self.total - self.offset)
+        # one or more list calls have been recomposed into a single list
+        self.assertEqual(last_result, len(results))
+
+        if last_result > 0:
+            self.assertEqual('stack_%s' % (self.offset + 1),
+                             results[0].stack_name)
+            self.assertEqual('stack_%s' % (self.offset + last_result),
+                             results[-1].stack_name)
diff --git a/heatclient/v1/stacks.py b/heatclient/v1/stacks.py
index b9e76fc..906e57e 100644
--- a/heatclient/v1/stacks.py
+++ b/heatclient/v1/stacks.py
@@ -17,8 +17,6 @@ import urllib
 
 from heatclient.common import base
 
-DEFAULT_PAGE_SIZE = 20
-
 
 class Stack(base.Resource):
     def __repr__(self):
@@ -86,10 +84,12 @@ class StackManager(base.Manager):
             if (page_size and len(stacks) == page_size and
                     (absolute_limit is None or 0 < seen < absolute_limit)):
                 qp['marker'] = stack.id
-                for image in paginate(qp, seen):
-                    yield image
+                for stack in paginate(qp, seen):
+                    yield stack
 
-        params = {'limit': kwargs.get('page_size', DEFAULT_PAGE_SIZE)}
+        params = {}
+        if 'page_size' in kwargs:
+            params['limit'] = kwargs['page_size']
 
         if 'marker' in kwargs:
             params['marker'] = kwargs['marker']