make as_list work for all queries

2 files changed, 67 insertions, 20 deletions

diff --git a/gitlab/__init__.py b/gitlab/__init__.py
index 8d522b4..5ea1301 100644
--- a/gitlab/__init__.py
+++ b/gitlab/__init__.py
@@ -506,8 +506,7 @@ class Gitlab(object):
 
         Returns:
             list: A list of the objects returned by the server. If `as_list` is
-            False and no pagination-related arguments (`page`, `per_page`,
-            `all`) are defined then a GitlabList object (generator) is returned
+            False then a GitlabList object (generator) is returned
             instead. This object will make API calls when needed to fetch the
             next items from the server.
 
@@ -517,21 +516,16 @@ class Gitlab(object):
         """
 
         # In case we want to change the default behavior at some point
-        as_list = True if as_list is None else as_list
+        as_list = as_list is None or as_list
 
         get_all = kwargs.get('all', False)
         url = self._build_url(path)
 
-        if get_all is True:
-            return list(GitlabList(self, url, query_data, **kwargs))
-
-        if 'page' in kwargs or as_list is True:
-            # pagination requested, we return a list
-            return list(GitlabList(self, url, query_data, get_next=False,
-                                   **kwargs))
-
-        # No pagination, generator requested
-        return GitlabList(self, url, query_data, **kwargs)
+        glist = GitlabList(self, url, query_data,
+                           get_next='page' not in kwargs and get_all, **kwargs)
+        if as_list:
+            glist = list(glist)
+        return glist
 
     def http_post(self, path, query_data={}, post_data={}, files=None,
                   **kwargs):
diff --git a/gitlab/tests/test_gitlab.py b/gitlab/tests/test_gitlab.py
index 5174bd2..3220988 100644
--- a/gitlab/tests/test_gitlab.py
+++ b/gitlab/tests/test_gitlab.py
@@ -26,7 +26,7 @@ except ImportError:
 
 from httmock import HTTMock  # noqa
 from httmock import response  # noqa
-from httmock import urlmatch  # noqa
+from httmock import remember_called, urlmatch  # noqa
 import requests
 
 import gitlab
@@ -57,6 +57,7 @@ class TestGitlabList(unittest.TestCase):
     def test_build_list(self):
         @urlmatch(scheme='http', netloc="localhost", path="/api/v4/tests",
                   method="get")
+        @remember_called
         def resp_1(url, request):
             headers = {'content-type': 'application/json',
                        'X-Page': 1,
@@ -72,6 +73,7 @@ class TestGitlabList(unittest.TestCase):
 
         @urlmatch(scheme='http', netloc="localhost", path="/api/v4/tests",
                   method='get', query=r'.*page=2')
+        @remember_called
         def resp_2(url, request):
             headers = {'content-type': 'application/json',
                        'X-Page': 2,
@@ -82,7 +84,7 @@ class TestGitlabList(unittest.TestCase):
             content = '[{"c": "d"}]'
             return response(200, content, headers, None, 5, request)
 
-        with HTTMock(resp_1):
+        with HTTMock(resp_2, resp_1):
             obj = self.gl.http_list('/tests', as_list=False)
             self.assertEqual(len(obj), 2)
             self.assertEqual(obj._next_url,
@@ -94,11 +96,62 @@ class TestGitlabList(unittest.TestCase):
             self.assertEqual(obj.total_pages, 2)
             self.assertEqual(obj.total, 2)
 
-            with HTTMock(resp_2):
-                l = list(obj)
-                self.assertEqual(len(l), 2)
-                self.assertEqual(l[0]['a'], 'b')
-                self.assertEqual(l[1]['c'], 'd')
+            l = list(obj)
+            self.assertListEqual(l, [{"a": "b"}])
+            self.assertEqual(resp_1.call['count'], 1)
+            self.assertFalse(resp_2.call['called'])
+
+    def test_build_list_all(self):
+        @urlmatch(scheme='http', netloc="localhost", path="/api/v4/tests",
+                  method="get")
+        @remember_called
+        def resp_1(url, request):
+            headers = {'content-type': 'application/json',
+                       'X-Page': 1,
+                       'X-Next-Page': 2,
+                       'X-Per-Page': 1,
+                       'X-Total-Pages': 2,
+                       'X-Total': 2,
+                       'Link': (
+                           '<http://localhost/api/v4/tests?per_page=1&page=2>;'
+                           ' rel="next"')}
+            content = '[{"a": "b"}]'
+            return response(200, content, headers, None, 5, request)
+
+        @urlmatch(scheme='http', netloc="localhost", path="/api/v4/tests",
+                  method='get', query=r'.*page=2')
+        @remember_called
+        def resp_2(url, request):
+            headers = {'content-type': 'application/json',
+                       'X-Page': 2,
+                       'X-Next-Page': 2,
+                       'X-Per-Page': 1,
+                       'X-Total-Pages': 2,
+                       'X-Total': 2}
+            content = '[{"c": "d"}]'
+            return response(200, content, headers, None, 5, request)
+
+        with HTTMock(resp_2, resp_1):
+            obj = self.gl.http_list('/tests', as_list=False, all=True)
+            self.assertEqual(len(obj), 2)
+            self.assertEqual(obj._next_url,
+                             'http://localhost/api/v4/tests?per_page=1&page=2')
+            self.assertEqual(obj.current_page, 1)
+            self.assertEqual(obj.prev_page, None)
+            self.assertEqual(obj.next_page, 2)
+            self.assertEqual(obj.per_page, 1)
+            self.assertEqual(obj.total_pages, 2)
+            self.assertEqual(obj.total, 2)
+            self.assertEqual(resp_1.call['count'], 1)
+            self.assertFalse(resp_2.call['called'])
+            self.assertDictEqual(next(obj), {"a": "b"})
+            self.assertEqual(resp_1.call['count'], 1)
+            self.assertFalse(resp_2.call['called'])
+            self.assertDictEqual(next(obj), {"c": "d"})
+            self.assertEqual(resp_1.call['count'], 1)
+            self.assertEqual(resp_2.call['count'], 1)
+            with self.assertRaises(StopIteration):
+                next(obj)
 
 
 class TestGitlabHttpMethods(unittest.TestCase):