paginate by startkey (#437)

* startkey/bookmark pagination

Author: aogier

CHANGES.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+- [IMPROVED] Updated `Result` iteration by paginating with views' `startkey` and
+  queries' `bookmark`.
 - [FIXED] Bug where document context manager performed remote save despite
   uncaught exceptions being raised inside `with` block.
 - [FIXED] Fixed parameter type of `selector` in docstring.

src/cloudant/result.py

@@ -15,6 +15,7 @@
 """
 API module for interacting with result collections.
 """
+from functools import partial
 from ._2to3 import STRTYPE
 from .error import ResultException
 from ._common_util import py_to_couch_validate, type_or_none
@@ -327,16 +328,16 @@ def _handle_result_by_key_slice(self, key_slice):
     def __iter__(self):
         """
         Provides iteration support, primarily for large data collections.
-        The iterator uses the ``skip`` and ``limit`` options to consume
-        data in chunks controlled by the ``page_size`` option.  It retrieves
-        a batch of data from the result collection and then yields each
-        element.
+        The iterator uses the ``startkey``, ``startkey_docid``, and ``limit``
+        options to consume data in chunks controlled by the ``page_size``
+        option. It retrieves a batch of data from the result collection
+        and then yields each element.
 
         See :class:`~cloudant.result.Result` for Result iteration examples.
 
         :returns: Iterable data sequence
         """
-        invalid_options = ('skip', 'limit')
+        invalid_options = ('limit', )
         if any(x in invalid_options for x in self.options):
             raise ResultException(103, invalid_options, self.options)
 
@@ -347,21 +348,60 @@ def __iter__(self):
         except ValueError:
             raise ResultException(104, self._page_size)
 
-        skip = 0
+        init_opts = {
+            'skip': self.options.pop('skip', None),
+            'startkey': self.options.pop('startkey', None)
+        }
+
+        self._call = partial(self._ref,  #pylint: disable=attribute-defined-outside-init
+                             limit=self._real_page_size,
+                             **self.options)
+
+        response = self._call(**{k: v
+                                 for k, v
+                                 in init_opts.items()
+                                 if v is not None})
+
+        return self._iterator(response)
+
+    @property
+    def _real_page_size(self):
+        '''
+        In views we paginate with N+1 items per page.
+        https://docs.couchdb.org/en/stable/ddocs/views/pagination.html#paging-alternate-method
+        '''
+        return self._page_size + 1
+
+    def _iterator(self, response):
+        '''
+        Iterate through view data.
+        '''
+
         while True:
-            response = self._ref(
-                limit=self._page_size,
-                skip=skip,
-                **self.options
-            )
             result = self._parse_data(response)
-            skip += self._page_size
             if result:
+                doc_count = len(result)
+                last = result.pop()
                 for row in result:
                     yield row
-                if len(result) < self._page_size:
+
+                # We expect doc_count = self._page_size + 1 results, if
+                # we have self._page_size or less it means we are on the
+                # last page and need to return the last result.
+                if doc_count < self._real_page_size:
+                    yield last
                     break
                 del result
+
+                # if we are in a view, keys could be duplicate so we
+                # need to start from the right docid
+                if last['id']:
+                    response = self._call(startkey=last['key'],
+                                          startkey_docid=last['id'])
+                # reduce result keys are unique by definition
+                else:
+                    response = self._call(startkey=last['key'])
+
             else:
                 break
 
@@ -510,3 +550,32 @@ def _parse_data(self, data):
         query result JSON response content
         """
         return data.get('docs', [])
+
+    @property
+    def _real_page_size(self):
+        '''
+        During queries iteration page size is user-specified
+        '''
+        return self._page_size
+
+    def _iterator(self, response):
+        '''
+        Iterate through query data.
+        '''
+
+        while True:
+            result = self._parse_data(response)
+            bookmark = response.get('bookmark')
+            if result:
+                for row in result:
+                    yield row
+
+                del result
+
+                if not bookmark:
+                    break
+
+                response = self._call(bookmark=bookmark)
+
+            else:
+                break

tests/unit/query_result_tests.py

@@ -425,24 +425,14 @@ def test_get_item_index_slice_using_stop_only_limit_skip(self):
 
     def test_iteration_with_invalid_options(self):
         """
-        Test that iteration raises an exception when "skip" and/or "limit" are
-        used as options for the result.
+        Test that iteration raises an exception when "limit" is
+        used as option for the result.
         """
-        result = self.create_result(q_parms={'skip': 10})
-        with self.assertRaises(ResultException) as cm:
-            invalid_result = [row for row in result]
-        self.assertEqual(cm.exception.status_code, 103)
-
         result = self.create_result(q_parms={'limit': 10})
         with self.assertRaises(ResultException) as cm:
             invalid_result = [row for row in result]
         self.assertEqual(cm.exception.status_code, 103)
 
-        result = self.create_result(q_parms={'limit': 10, 'skip': 10})
-        with self.assertRaises(ResultException) as cm:
-            invalid_result = [row for row in result]
-        self.assertEqual(cm.exception.status_code, 103)
-
     def test_iteration_invalid_page_size(self):
         """
         Test that iteration raises an exception when and invalid "page_size" is

tests/unit/result_tests.py

@@ -12,13 +12,15 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import mock
 """
 result module - Unit tests for Result class
 """
 import unittest
 
 from cloudant.error import ResultException
 from cloudant.result import Result, ResultByKey
+from cloudant.view import View
 from nose.plugins.attrib import attr
 from requests.exceptions import HTTPError
 
@@ -564,24 +566,14 @@ def test_get_item_key_slice_using_stop_only(self):
 
     def test_iteration_with_invalid_options(self):
         """
-        Test that iteration raises an exception when "skip" and/or "limit" are
-        used as options for the result.
+        Test that iteration raises an exception when "limit" is
+        used as option for the result.
         """
-        result = Result(self.view001, skip=10)
-        with self.assertRaises(ResultException) as cm:
-            invalid_result = [row for row in result]
-        self.assertEqual(cm.exception.status_code, 103)
-
         result = Result(self.view001, limit=10)
         with self.assertRaises(ResultException) as cm:
             invalid_result = [row for row in result]
         self.assertEqual(cm.exception.status_code, 103)
 
-        result = Result(self.view001, skip=10, limit=10)
-        with self.assertRaises(ResultException) as cm:
-            invalid_result = [row for row in result]
-        self.assertEqual(cm.exception.status_code, 103)
-
     def test_iteration_invalid_page_size(self):
         """
         Test that iteration raises an exception when and invalid "page_size" is
@@ -643,5 +635,49 @@ def test_iteration_no_data(self):
         result = Result(self.view001, startkey='ruby')
         self.assertEqual([x for x in result], [])
 
+    def test_iteration_integer_keys(self):
+        """
+        Test that iteration works as expected when keys are integer.
+        """
+        result = Result(self.view007, page_size=10)
+        self.assertEqual(len([x for x in result]), 100)
+
+    def test_iteration_pagination(self):
+        """
+        Test that iteration pagination works as expected.
+        """
+
+        class CallMock:
+            expected_calls = [
+                {'limit': 28},
+                {'limit': 28, 'startkey': 1, 'startkey_docid': 'julia027'},
+                {'limit': 28, 'startkey': 1, 'startkey_docid': 'julia054'},
+                {'limit': 28, 'startkey': 1, 'startkey_docid': 'julia081'},
+            ]
+
+            def __init__(self, outer):
+                self.outer = outer
+                self.expected_calls.reverse()
+
+            def call(self, *args, **kwargs):
+                self.outer.assertEqual(dict(kwargs),
+                                       self.expected_calls.pop(),
+                                       'pagination error')
+                return View.__call__(self.outer.view007, *args, **kwargs)
+
+        with mock.patch.object(self, 'view007',
+                               CallMock(self).call) as _:
+
+            result = Result(self.view007, page_size=27)
+
+        expected = [
+            {'id': 'julia{0:03d}'.format(i),
+             'key': 1,
+             'value': 'julia'}
+            for i in range(100)
+        ]
+        self.assertEqual([x for x in result], expected)
+
+
 if __name__ == '__main__':
     unittest.main()

tests/unit/unit_t_db_base.py

@@ -314,13 +314,18 @@ def create_views(self):
             'function (doc) {\n emit([doc.name, doc.age], 1);\n}',
             '_count'
         )
+        self.ddoc.add_view(
+            'view007',
+            'function (doc) {\n emit(1, doc.name);\n}'
+        )
         self.ddoc.save()
         self.view001 = self.ddoc.get_view('view001')
         self.view002 = self.ddoc.get_view('view002')
         self.view003 = self.ddoc.get_view('view003')
         self.view004 = self.ddoc.get_view('view004')
         self.view005 = self.ddoc.get_view('view005')
         self.view006 = self.ddoc.get_view('view006')
+        self.view007 = self.ddoc.get_view('view007')
 
     def create_search_index(self):
         """