Restore the prvious listing behavior

Return lists by default : this makes the explicit use of pagination work
again.

Use generators only when `as_list` is explicitly set to `False`.

5 files changed, 56 insertions, 48 deletions

diff --git a/docs/switching-to-v4.rst b/docs/switching-to-v4.rst
index fb2b978..84181ff 100644
--- a/docs/switching-to-v4.rst
+++ b/docs/switching-to-v4.rst
@@ -63,15 +63,15 @@ following important changes in the python API:
      gl = gitlab.Gitlab(...)
      p = gl.projects.get(project_id)
 
-* Listing methods (``manager.list()`` for instance) now return generators
+* Listing methods (``manager.list()`` for instance) can now return generators
   (:class:`~gitlab.base.RESTObjectList`). They handle the calls to the API when
-  needed.
+  needed to fetch new items.
 
-  If you need to get all the items at once, use the ``all=True`` parameter:
+  By default you will still get lists. To get generators use ``as_list=False``:
 
   .. code-block:: python
 
-     all_projects = gl.projects.list(all=True)
+     all_projects_g = gl.projects.list(as_list=False)
 
 * The "nested" managers (for instance ``gl.project_issues`` or
   ``gl.group_members``) are not available anymore. Their goal was to provide a
@@ -114,18 +114,3 @@ following important changes in the python API:
   + :attr:`~gitlab.Gitlab.http_post`
   + :attr:`~gitlab.Gitlab.http_put`
   + :attr:`~gitlab.Gitlab.http_delete`
-
-* The users ``get_by_username`` method has been removed. It doesn't exist in
-  the GitLab API. You can use the ``username`` filter attribute when listing to
-  get a similar behavior:
-
-  .. code-block:: python
-
-     user = list(gl.users.list(username='jdoe'))[0]
-
-
-Undergoing work
-===============
-
-* The ``page`` and ``per_page`` arguments for listing don't behave as they used
-  to. Their behavior will be restored.
diff --git a/gitlab/__init__.py b/gitlab/__init__.py
index 617f50c..bdeb5c4 100644
--- a/gitlab/__init__.py
+++ b/gitlab/__init__.py
@@ -712,7 +712,7 @@ class Gitlab(object):
         else:
             return result
 
-    def http_list(self, path, query_data={}, **kwargs):
+    def http_list(self, path, query_data={}, as_list=None, **kwargs):
         """Make a GET request to the Gitlab server for list-oriented queries.
 
         Args:
@@ -723,19 +723,33 @@ class Gitlab(object):
                       all)
 
         Returns:
-            GitlabList: A generator giving access to the objects. If an ``all``
-            kwarg is defined and True, returns a list of all the objects (will
-            possibly make numerous calls to the Gtilab server and eat a lot of
-            memory)
+            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
+            instead. This object will make API calls when needed to fetch the
+            next items from the server.
 
         Raises:
             GitlabHttpError: When the return code is not 2xx
             GitlabParsingError: If the json data could not be parsed
         """
+
+        # In case we want to change the default behavior at some point
+        as_list = True if as_list is None else as_list
+
+        get_all = kwargs.get('all', False)
         url = self._build_url(path)
-        get_all = kwargs.pop('all', False)
-        obj_gen = GitlabList(self, url, query_data, **kwargs)
-        return list(obj_gen) if get_all else obj_gen
+
+        if get_all is True:
+            return list(GitlabList(self, url, query_data, **kwargs))
+
+        if 'page' in kwargs or 'per_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)
 
     def http_post(self, path, query_data={}, post_data={}, **kwargs):
         """Make a POST request to the Gitlab server.
@@ -816,9 +830,10 @@ class GitlabList(object):
     the API again when needed.
     """
 
-    def __init__(self, gl, url, query_data, **kwargs):
+    def __init__(self, gl, url, query_data, get_next=True, **kwargs):
         self._gl = gl
         self._query(url, query_data, **kwargs)
+        self._get_next = get_next
 
     def _query(self, url, query_data={}, **kwargs):
         result = self._gl.http_request('get', url, query_data=query_data,
@@ -856,7 +871,7 @@ class GitlabList(object):
             self._current += 1
             return item
         except IndexError:
-            if self._next_url:
+            if self._next_url and self._get_next is True:
                 self._query(self._next_url)
                 return self.next()
 
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
index 5876d58..4fc21fb 100644
--- a/gitlab/mixins.py
+++ b/gitlab/mixins.py
@@ -71,15 +71,15 @@ class ListMixin(object):
         """Retrieve a list of objects.
 
         Args:
-            **kwargs: Extra options to send to the Gitlab server (e.g. sudo).
-                      If ``all`` is passed and set to True, the entire list of
-                      objects will be returned.
+            all (bool): If True, return all the items, without pagination
+            per_page (int): Number of items to retrieve per request
+            page (int): ID of the page to return (starts with page 1)
+            as_list (bool): If set to False and no pagination option is
+                defined, return a generator instead of a list
+            **kwargs: Extra options to send to the Gitlab server (e.g. sudo)
 
         Returns:
-            RESTObjectList: Generator going through the list of objects, making
-                            queries to the server when required.
-                            If ``all=True`` is passed as argument, returns
-                            list(RESTObjectList).
+            list: The list of objects, or a generator if `as_list` is False
 
         Raises:
             GitlabAuthenticationError: If authentication is not correct
diff --git a/gitlab/v4/objects.py b/gitlab/v4/objects.py
index b94d84a..0a60924 100644
--- a/gitlab/v4/objects.py
+++ b/gitlab/v4/objects.py
@@ -1087,7 +1087,8 @@ class ProjectMergeRequest(SubscribableMixin, TodoMixin, TimeTrackingMixin,
             RESTObjectList: List of issues
         """
         path = '%s/%s/closes_issues' % (self.manager.path, self.get_id())
-        data_list = self.manager.gitlab.http_list(path, **kwargs)
+        data_list = self.manager.gitlab.http_list(path, as_list=False,
+                                                  **kwargs)
         manager = ProjectIssueManager(self.manager.gitlab,
                                       parent=self.manager._parent)
         return RESTObjectList(manager, ProjectIssue, data_list)
@@ -1108,7 +1109,8 @@ class ProjectMergeRequest(SubscribableMixin, TodoMixin, TimeTrackingMixin,
         """
 
         path = '%s/%s/commits' % (self.manager.path, self.get_id())
-        data_list = self.manager.gitlab.http_list(path, **kwargs)
+        data_list = self.manager.gitlab.http_list(path, as_list=False,
+                                                  **kwargs)
         manager = ProjectCommitManager(self.manager.gitlab,
                                        parent=self.manager._parent)
         return RESTObjectList(manager, ProjectCommit, data_list)
@@ -1197,7 +1199,8 @@ class ProjectMilestone(SaveMixin, ObjectDeleteMixin, RESTObject):
         """
 
         path = '%s/%s/issues' % (self.manager.path, self.get_id())
-        data_list = self.manager.gitlab.http_list(path, **kwargs)
+        data_list = self.manager.gitlab.http_list(path, as_list=False,
+                                                  **kwargs)
         manager = ProjectCommitManager(self.manager.gitlab,
                                        parent=self.manager._parent)
         # FIXME(gpocentek): the computed manager path is not correct
@@ -1218,7 +1221,8 @@ class ProjectMilestone(SaveMixin, ObjectDeleteMixin, RESTObject):
             RESTObjectList: The list of merge requests
         """
         path = '%s/%s/merge_requests' % (self.manager.path, self.get_id())
-        data_list = self.manager.gitlab.http_list(path, **kwargs)
+        data_list = self.manager.gitlab.http_list(path, as_list=False,
+                                                  **kwargs)
         manager = ProjectCommitManager(self.manager.gitlab,
                                        parent=self.manager._parent)
         # FIXME(gpocentek): the computed manager path is not correct
@@ -2009,6 +2013,11 @@ class RunnerManager(RetrieveMixin, UpdateMixin, DeleteMixin, RESTManager):
         Args:
             scope (str): The scope of runners to show, one of: specific,
                 shared, active, paused, online
+            all (bool): If True, return all the items, without pagination
+            per_page (int): Number of items to retrieve per request
+            page (int): ID of the page to return (starts with page 1)
+            as_list (bool): If set to False and no pagination option is
+                defined, return a generator instead of a list
             **kwargs: Extra options to send to the server (e.g. sudo)
 
         Raises:
diff --git a/tools/python_test_v4.py b/tools/python_test_v4.py
index ec3f0d3..08ee0aa 100644
--- a/tools/python_test_v4.py
+++ b/tools/python_test_v4.py
@@ -55,7 +55,7 @@ foobar_user = gl.users.create(
     {'email': 'foobar@example.com', 'username': 'foobar',
      'name': 'Foo Bar', 'password': 'foobar_password'})
 
-assert gl.users.list(search='foobar').next().id == foobar_user.id
+assert gl.users.list(search='foobar')[0].id == foobar_user.id
 usercmp = lambda x,y: cmp(x.id, y.id)
 expected = sorted([new_user, foobar_user], cmp=usercmp)
 actual = sorted(list(gl.users.list(search='foo')), cmp=usercmp)
@@ -92,7 +92,7 @@ user2 = gl.users.create({'email': 'user2@test.com', 'username': 'user2',
 group1 = gl.groups.create({'name': 'group1', 'path': 'group1'})
 group2 = gl.groups.create({'name': 'group2', 'path': 'group2'})
 
-p_id = gl.groups.list(search='group2').next().id
+p_id = gl.groups.list(search='group2')[0].id
 group3 = gl.groups.create({'name': 'group3', 'path': 'group3', 'parent_id': p_id})
 
 assert(len(gl.groups.list()) == 3)
@@ -139,12 +139,11 @@ assert(len(gl.projects.list(owned=True)) == 2)
 assert(len(gl.projects.list(search="admin")) == 1)
 
 # test pagination
-# FIXME => we should return lists, not RESTObjectList
-#l1 = gl.projects.list(per_page=1, page=1)
-#l2 = gl.projects.list(per_page=1, page=2)
-#assert(len(l1) == 1)
-#assert(len(l2) == 1)
-#assert(l1[0].id != l2[0].id)
+l1 = gl.projects.list(per_page=1, page=1)
+l2 = gl.projects.list(per_page=1, page=2)
+assert(len(l1) == 1)
+assert(len(l2) == 1)
+assert(l1[0].id != l2[0].id)
 
 # project content (files)
 admin_project.files.create({'file_path': 'README',