Moving backend specific behavior from Page to Iterator.

This is to lower the burden on implementers. The previous
approach (requiring a Page and Iterator subclass) ended
up causing lots of copy-pasta docstrings that were just
a distraction.

Follow up to #2531.

Author: dhermes

packages/google-cloud-core/google/cloud/iterator.py

@@ -17,22 +17,19 @@
 These iterators simplify the process of paging through API responses
 where the response is a list of results with a ``nextPageToken``.
 
-To make an iterator work, just override the ``PAGE_CLASS`` class
-attribute so that given a response (containing a page of results) can
-be parsed into an iterable page of the actual objects you want::
+To make an iterator work, you may need to override the
+``ITEMS_KEY`` class attribute so that given a response (containing a page of
+results) can be parsed into an iterable page of the actual objects you want::
 
-  class MyPage(Page):
+  class MyIterator(Iterator):
+
+      ITEMS_KEY = 'blocks'
 
       def _item_to_value(self, item):
           my_item = MyItemClass(other_arg=True)
           my_item._set_properties(item)
           return my_item
 
-
-  class MyIterator(Iterator):
-
-      PAGE_CLASS = MyPage
-
 You then can use this to get **all** the results from a resource::
 
     >>> iterator = MyIterator(...)
@@ -69,6 +66,30 @@ class MyIterator(Iterator):
     2
     >>> iterator.page.remaining
     19
+
+It's also possible to consume an entire page and handle the paging process
+manually::
+
+    >>> iterator = MyIterator(...)
+    >>> items = list(iterator.page)
+    >>> items
+    [
+        <MyItemClass at 0x7fd64a098ad0>,
+        <MyItemClass at 0x7fd64a098ed0>,
+        <MyItemClass at 0x7fd64a098e90>,
+    ]
+    >>> iterator.page.remaining
+    0
+    >>> iterator.page.num_items
+    3
+    >>> iterator.next_page_token
+    'eav1OzQB0OM8rLdGXOEsyQWSG'
+    >>> # And just do the same thing to consume the next page.
+    >>> list(iterator.page)
+    [
+        <MyItemClass at 0x7fea740abdd0>,
+        <MyItemClass at 0x7fea740abe50>,
+    ]
 """
 
 
@@ -83,16 +104,19 @@ class Page(object):
 
     :type response: dict
     :param response: The JSON API response for a page.
-    """
 
-    ITEMS_KEY = 'items'
+    :type items_key: str
+    :param items_key: The dictionary key used to retrieve items
+                      from the response.
+    """
 
-    def __init__(self, parent, response):
+    def __init__(self, parent, response, items_key):
         self._parent = parent
-        items = response.get(self.ITEMS_KEY, ())
+        items = response.get(items_key, ())
         self._num_items = len(items)
         self._remaining = self._num_items
         self._item_iter = iter(items)
+        self.response = response
 
     @property
     def num_items(self):
@@ -116,23 +140,10 @@ def __iter__(self):
         """The :class:`Page` is an iterator."""
         return self
 
-    def _item_to_value(self, item):
-        """Get the next item in the page.
-
-        This method (along with the constructor) is the workhorse
-        of this class. Subclasses will need to implement this method.
-
-        :type item: dict
-        :param item: An item to be converted to a native object.
-
-        :raises NotImplementedError: Always
-        """
-        raise NotImplementedError
-
     def next(self):
         """Get the next value in the iterator."""
         item = six.next(self._item_iter)
-        result = self._item_to_value(item)
+        result = self._parent._item_to_value(item)
         # Since we've successfully got the next value from the
         # iterator, we update the number of remaining.
         self._remaining -= 1
@@ -145,7 +156,8 @@ def next(self):
 class Iterator(object):
     """A generic class for iterating through Cloud JSON APIs list responses.
 
-    Sub-classes need to over-write ``PAGE_CLASS``.
+    Sub-classes need to over-write :attr:`ITEMS_KEY` and to define
+    :meth:`_item_to_value`.
 
     :type client: :class:`google.cloud.client.Client`
     :param client: The client, which owns a connection to make requests.
@@ -166,8 +178,9 @@ class Iterator(object):
     PAGE_TOKEN = 'pageToken'
     MAX_RESULTS = 'maxResults'
     RESERVED_PARAMS = frozenset([PAGE_TOKEN, MAX_RESULTS])
-    PAGE_CLASS = Page
     PATH = None
+    ITEMS_KEY = 'items'
+    """The dictionary key used to retrieve items from each response."""
 
     def __init__(self, client, page_token=None, max_results=None,
                  extra_params=None, path=None):
@@ -200,31 +213,46 @@ def page(self):
         :rtype: :class:`Page`
         :returns: The page of items that has been retrieved.
         """
+        self._update_page()
         return self._page
 
     def __iter__(self):
         """The :class:`Iterator` is an iterator."""
         return self
 
     def _update_page(self):
-        """Replace the current page.
+        """Update the current page if needed.
 
-        Does nothing if the current page is non-null and has items
-        remaining.
+        Subclasses will need to implement this method if they
+        use data from the ``response`` other than the items.
 
+        :rtype: bool
+        :returns: Flag indicated if the page was updated.
         :raises: :class:`~exceptions.StopIteration` if there is no next page.
         """
-        if self.page is not None and self.page.remaining > 0:
-            return
-        if self.has_next_page():
+        if self._page is not None and self._page.remaining > 0:
+            return False
+        elif self.has_next_page():
             response = self._get_next_page_response()
-            self._page = self.PAGE_CLASS(self, response)
+            self._page = Page(self, response, self.ITEMS_KEY)
+            return True
         else:
             raise StopIteration
 
+    def _item_to_value(self, item):
+        """Get the next item in the page.
+
+        Subclasses will need to implement this method.
+
+        :type item: dict
+        :param item: An item to be converted to a native object.
+
+        :raises NotImplementedError: Always
+        """
+        raise NotImplementedError
+
     def next(self):
         """Get the next value in the iterator."""
-        self._update_page()
         item = six.next(self.page)
         self.num_results += 1
         return item

packages/google-cloud-core/unit_tests/test_iterator.py

@@ -25,41 +25,34 @@ def _makeOne(self, *args, **kw):
         return self._getTargetClass()(*args, **kw)
 
     def test_constructor(self):
-        klass = self._getTargetClass()
         parent = object()
-        response = {klass.ITEMS_KEY: (1, 2, 3)}
-        page = self._makeOne(parent, response)
+        items_key = 'potatoes'
+        response = {items_key: (1, 2, 3)}
+        page = self._makeOne(parent, response, items_key)
         self.assertIs(page._parent, parent)
         self.assertEqual(page._num_items, 3)
         self.assertEqual(page._remaining, 3)
 
     def test_num_items_property(self):
-        page = self._makeOne(None, {})
+        page = self._makeOne(None, {}, '')
         num_items = 42
         page._num_items = num_items
         self.assertEqual(page.num_items, num_items)
 
     def test_remaining_property(self):
-        page = self._makeOne(None, {})
+        page = self._makeOne(None, {}, '')
         remaining = 1337
         page._remaining = remaining
         self.assertEqual(page.remaining, remaining)
 
     def test___iter__(self):
-        page = self._makeOne(None, {})
+        page = self._makeOne(None, {}, '')
         self.assertIs(iter(page), page)
 
-    def test__item_to_value(self):
-        page = self._makeOne(None, {})
-        with self.assertRaises(NotImplementedError):
-            page._item_to_value(None)
-
     def test_iterator_calls__item_to_value(self):
         import six
 
-        klass = self._getTargetClass()
-
-        class CountItPage(klass):
+        class Parent(object):
 
             calls = 0
             values = None
@@ -68,20 +61,22 @@ def _item_to_value(self, item):
                 self.calls += 1
                 return item
 
-        response = {klass.ITEMS_KEY: [10, 11, 12]}
-        page = CountItPage(None, response)
+        items_key = 'turkeys'
+        response = {items_key: [10, 11, 12]}
+        parent = Parent()
+        page = self._makeOne(parent, response, items_key)
         page._remaining = 100
 
-        self.assertEqual(page.calls, 0)
+        self.assertEqual(parent.calls, 0)
         self.assertEqual(page.remaining, 100)
         self.assertEqual(six.next(page), 10)
-        self.assertEqual(page.calls, 1)
+        self.assertEqual(parent.calls, 1)
         self.assertEqual(page.remaining, 99)
         self.assertEqual(six.next(page), 11)
-        self.assertEqual(page.calls, 2)
+        self.assertEqual(parent.calls, 2)
         self.assertEqual(page.remaining, 98)
         self.assertEqual(six.next(page), 12)
-        self.assertEqual(page.calls, 3)
+        self.assertEqual(parent.calls, 3)
         self.assertEqual(page.remaining, 97)
 
 
@@ -132,24 +127,24 @@ def test___iter__(self):
 
     def test_iterate(self):
         import six
-        from google.cloud.iterator import Page
 
         path = '/foo'
         key1 = 'key1'
         key2 = 'key2'
         item1, item2 = object(), object()
         ITEMS = {key1: item1, key2: item2}
 
-        class _Page(Page):
+        klass = self._getTargetClass()
+
+        class WithItemToValue(klass):
 
             def _item_to_value(self, item):
                 return ITEMS[item['name']]
 
         connection = _Connection(
             {'items': [{'name': key1}, {'name': key2}]})
         client = _Client(connection)
-        iterator = self._makeOne(client, path=path)
-        iterator.PAGE_CLASS = _Page
+        iterator = WithItemToValue(client, path=path)
         self.assertEqual(iterator.num_results, 0)
 
         val1 = six.next(iterator)
@@ -274,6 +269,11 @@ def test__get_next_page_response_new_no_token_in_response(self):
         self.assertEqual(kw['path'], path)
         self.assertEqual(kw['query_params'], {})
 
+    def test__item_to_value_virtual(self):
+        iterator = self._makeOne(None)
+        with self.assertRaises(NotImplementedError):
+            iterator._item_to_value({})
+
     def test_reset(self):
         connection = _Connection()
         client = _Client(connection)
@@ -287,7 +287,7 @@ def test_reset(self):
         self.assertEqual(iterator.page_number, 0)
         self.assertEqual(iterator.num_results, 0)
         self.assertIsNone(iterator.next_page_token)
-        self.assertIsNone(iterator.page)
+        self.assertIsNone(iterator._page)
 
 
 class _Connection(object):