Removing NO_MORE_PAGES sentinel and just making None mean no more pages.

Also renaming next_page() to update_page() on Iterator and dropping
any return value from that method. Also throwing an AttributeError
if the page is unset on @Property access.

core/google/cloud/iterator.py

@@ -71,13 +71,8 @@ def _item_to_value(self, item):
 manually::
 
     >>> iterator = MyIterator(...)
-    >>> # No page of results before the iterator has started.
-    >>> iterator.page is None
-    True
-    >>>
-    >>> # Manually pull down the next page.
-    >>> iterator.next_page()  # Returns "updated" status of page
-    True
+    >>> # Manually pull down the first page.
+    >>> iterator.update_page()
     >>> items = list(iterator.page)
     >>> items
     [
@@ -92,30 +87,31 @@ def _item_to_value(self, item):
     >>> iterator.next_page_token
     'eav1OzQB0OM8rLdGXOEsyQWSG'
     >>>
-    >>> iterator.next_page()
-    True
+    >>> # Ask for the next page to be grabbed.
+    >>> iterator.update_page()
     >>> list(iterator.page)
     [
         <MyItemClass at 0x7fea740abdd0>,
         <MyItemClass at 0x7fea740abe50>,
     ]
     >>>
     >>> # When there are no more results
-    >>> iterator.next_page()
-    True
-    >>> iterator.page is google.cloud.iterator.NO_MORE_PAGES
+    >>> iterator.update_page()
+    >>> iterator.page is None
     True
 """
 
 
 import six
 
 
-NO_MORE_PAGES = object()
-"""Sentinel object indicating an iterator has no more pages."""
+_UNSET = object()
 _NO_MORE_PAGES_ERR = 'Iterator has no more pages.'
+_UNSTARTED_ERR = (
+    'Iterator has not been started. Either begin iterating, '
+    'call next(my_iter) or call my_iter.update_page().')
 _PAGE_ERR_TEMPLATE = (
-    'Tried to get next_page() while current page (%r) still has %d '
+    'Tried to update the page while current page (%r) still has %d '
     'items remaining.')
 
 
@@ -191,11 +187,13 @@ class Iterator(object):
     :type max_results: int
     :param max_results: (Optional) The maximum number of results to fetch.
 
-    :type extra_params: :class:`dict` or :data:`None`
-    :param extra_params: Extra query string parameters for the API call.
+    :type extra_params: dict
+    :param extra_params: (Optional) Extra query string parameters for the
+                         API call.
 
     :type path: str
-    :param path: The path to query for the list of items.
+    :param path: (Optional) The path to query for the list of items. Defaults
+                 to :attr:`PATH` on the current iterator class.
     """
 
     PAGE_TOKEN = 'pageToken'
@@ -217,7 +215,7 @@ def __init__(self, client, page_token=None, max_results=None,
         self.page_number = 0
         self.next_page_token = page_token
         self.num_results = 0
-        self._page = None
+        self._page = _UNSET
 
     def _verify_params(self):
         """Verifies the parameters don't use any reserved parameter.
@@ -234,57 +232,56 @@ def _verify_params(self):
     def page(self):
         """The current page of results that has been retrieved.
 
+        If there are no more results, will return :data:`None`.
+
         :rtype: :class:`Page`
         :returns: The page of items that has been retrieved.
+        :raises AttributeError: If the page has not been set.
         """
+        if self._page is _UNSET:
+            raise AttributeError(_UNSTARTED_ERR)
         return self._page
 
     def __iter__(self):
         """The :class:`Iterator` is an iterator."""
         return self
 
-    def next_page(self, require_empty=True):
+    def update_page(self, require_empty=True):
         """Move to the next page in the result set.
 
         If the current page is not empty and ``require_empty`` is :data:`True`
         then an exception will be raised. If the current page is not empty
         and ``require_empty`` is :data:`False`, then this will return
-        without updating the current page (and will return an ``updated``
-        value of :data:`False`).
+        without updating the current page.
 
         If the current page **is** empty, but there are no more results,
-        sets the current page to :attr:`NO_MORE_PAGES`.
+        sets the current page to :data:`None`.
 
-        If the current page is :attr:`NO_MORE_PAGES`, throws an exception.
+        If there are no more pages, throws an exception.
 
         :type require_empty: bool
         :param require_empty: (Optional) Flag to indicate if the current page
                               must be empty before updating.
 
-        :rtype: bool
-        :returns: Flag indicated if the page was updated.
         :raises ValueError: If ``require_empty`` is :data:`True` but the
                             current page is not empty.
-        :raises ValueError: If the current page is :attr:`NO_MORE_PAGES`.
+        :raises ValueError: If there are no more pages.
         """
-        if self._page is NO_MORE_PAGES:
+        if self._page is None:
             raise ValueError(_NO_MORE_PAGES_ERR)
 
         # NOTE: This assumes Page.remaining can never go below 0.
-        page_empty = self._page is None or self._page.remaining == 0
+        page_empty = self._page is _UNSET or self._page.remaining == 0
         if page_empty:
             if self.has_next_page():
                 response = self._get_next_page_response()
                 self._page = self._PAGE_CLASS(self, response, self.ITEMS_KEY)
             else:
-                self._page = NO_MORE_PAGES
-
-            return True
+                self._page = None
         else:
             if require_empty:
                 msg = _PAGE_ERR_TEMPLATE % (self._page, self.page.remaining)
                 raise ValueError(msg)
-            return False
 
     def _item_to_value(self, item):
         """Get the next item in the page.
@@ -300,8 +297,8 @@ def _item_to_value(self, item):
 
     def next(self):
         """Get the next item from the request."""
-        self.next_page(require_empty=False)
-        if self.page is NO_MORE_PAGES:
+        self.update_page(require_empty=False)
+        if self.page is None:
             raise StopIteration
         item = six.next(self.page)
         self.num_results += 1
@@ -359,4 +356,4 @@ def reset(self):
         self.page_number = 0
         self.next_page_token = None
         self.num_results = 0
-        self._page = None
+        self._page = _UNSET

core/unit_tests/test_iterator.py

@@ -121,44 +121,54 @@ def test_constructor_w_extra_param_collision(self):
         with self.assertRaises(ValueError):
             self._makeOne(client, path=path, extra_params=extra_params)
 
-    def test_next_page_no_more(self):
-        from google.cloud.iterator import NO_MORE_PAGES
+    def test_page_property(self):
+        iterator = self._makeOne(None)
+        page = object()
+        iterator._page = page
+        self.assertIs(iterator.page, page)
+
+    def test_page_property_unset(self):
+        from google.cloud.iterator import _UNSET
+
+        iterator = self._makeOne(None)
+        self.assertIs(iterator._page, _UNSET)
+        with self.assertRaises(AttributeError):
+            getattr(iterator, 'page')
 
+    def test_update_page_no_more(self):
         iterator = self._makeOne(None)
-        iterator._page = NO_MORE_PAGES
+        iterator._page = None
         with self.assertRaises(ValueError):
-            iterator.next_page()
+            iterator.update_page()
 
-    def test_next_page_not_empty_success(self):
+    def test_update_page_not_empty_success(self):
         from google.cloud.iterator import Page
 
         iterator = self._makeOne(None)
-        iterator._page = Page(None, {}, '')
+        page = Page(None, {}, '')
+        iterator._page = page
         iterator._page._remaining = 1
-        updated = iterator.next_page(require_empty=False)
-        self.assertFalse(updated)
+        iterator.update_page(require_empty=False)
+        self.assertIs(iterator._page, page)
 
-    def test_next_page_not_empty_fail(self):
+    def test_update_page_not_empty_fail(self):
         from google.cloud.iterator import Page
 
         iterator = self._makeOne(None)
         iterator._page = Page(None, {}, '')
         iterator._page._remaining = 1
         with self.assertRaises(ValueError):
-            iterator.next_page(require_empty=True)
-
-    def test_next_page_empty_then_no_more(self):
-        from google.cloud.iterator import NO_MORE_PAGES
+            iterator.update_page(require_empty=True)
 
+    def test_update_page_empty_then_no_more(self):
         iterator = self._makeOne(None)
         # Fake that there are no more pages.
         iterator.page_number = 1
         iterator.next_page_token = None
-        updated = iterator.next_page()
-        self.assertTrue(updated)
-        self.assertIs(iterator.page, NO_MORE_PAGES)
+        iterator.update_page()
+        self.assertIsNone(iterator.page)
 
-    def test_next_page_empty_then_another(self):
+    def test_update_page_empty_then_another(self):
         iterator = self._makeOne(None)
         # Fake the next page class.
         fake_page = object()
@@ -173,8 +183,7 @@ def dummy_page_class(*args):
 
         iterator._get_next_page_response = dummy_response
         iterator._PAGE_CLASS = dummy_page_class
-        updated = iterator.next_page()
-        self.assertTrue(updated)
+        iterator.update_page()
         self.assertIs(iterator.page, fake_page)
         self.assertEqual(page_args, [(iterator, {}, iterator.ITEMS_KEY)])
 
@@ -332,6 +341,8 @@ def test__item_to_value_virtual(self):
             iterator._item_to_value({})
 
     def test_reset(self):
+        from google.cloud.iterator import _UNSET
+
         connection = _Connection()
         client = _Client(connection)
         path = '/foo'
@@ -344,7 +355,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.assertIs(iterator._page, _UNSET)
 
 
 class _Connection(object):

resource_manager/unit_tests/test_client.py

@@ -68,7 +68,7 @@ def dummy_response():
         iterator = self._makeOne(client)
         iterator._get_next_page_response = dummy_response
 
-        iterator.next_page()
+        iterator.update_page()
         page = iterator.page
         self.assertEqual(page.num_items, 1)
         project = iterator.next()

storage/unit_tests/test_bucket.py

@@ -64,7 +64,7 @@ def dummy_response():
         iterator = self._makeOne(bucket, client=client)
         iterator._get_next_page_response = dummy_response
 
-        iterator.next_page()
+        iterator.update_page()
         page = iterator.page
         self.assertEqual(page.prefixes, ('foo',))
         self.assertEqual(page.num_items, 1)
@@ -98,7 +98,7 @@ def dummy_response():
         iterator._get_next_page_response = dummy_response
 
         # Parse first response.
-        iterator.next_page()
+        iterator.update_page()
         page1 = iterator.page
         self.assertEqual(page1.prefixes, ('foo',))
         self.assertEqual(page1.num_items, 1)
@@ -108,7 +108,7 @@ def dummy_response():
         self.assertEqual(blob.name, BLOB_NAME)
         self.assertEqual(iterator.prefixes, set(['foo']))
         # Parse second response.
-        iterator.next_page()
+        iterator.update_page()
         page2 = iterator.page
         self.assertEqual(page2.prefixes, ('bar',))
         self.assertEqual(page2.num_items, 0)

storage/unit_tests/test_client.py

@@ -412,7 +412,7 @@ def dummy_response():
         iterator = self._makeOne(client)
         iterator._get_next_page_response = dummy_response
 
-        iterator.next_page()
+        iterator.update_page()
         page = iterator.page
         self.assertEqual(page.num_items, 1)
         bucket = iterator.next()

system_tests/storage.py

@@ -257,12 +257,12 @@ def test_paginate_files(self):
         truncation_size = 1
         count = len(self.FILENAMES) - truncation_size
         iterator = self.bucket.list_blobs(max_results=count)
-        iterator.next_page()
+        iterator.update_page()
         blobs = list(iterator.page)
         self.assertEqual(len(blobs), count)
         self.assertIsNotNone(iterator.next_page_token)
 
-        iterator.next_page()
+        iterator.update_page()
         last_blobs = list(iterator.page)
         self.assertEqual(len(last_blobs), truncation_size)
 
@@ -301,7 +301,7 @@ def tearDownClass(cls):
     @RetryErrors(unittest.TestCase.failureException)
     def test_root_level_w_delimiter(self):
         iterator = self.bucket.list_blobs(delimiter='/')
-        iterator.next_page()
+        iterator.update_page()
         blobs = list(iterator.page)
         self.assertEqual([blob.name for blob in blobs], ['file01.txt'])
         self.assertIsNone(iterator.next_page_token)
@@ -310,7 +310,7 @@ def test_root_level_w_delimiter(self):
     @RetryErrors(unittest.TestCase.failureException)
     def test_first_level(self):
         iterator = self.bucket.list_blobs(delimiter='/', prefix='parent/')
-        iterator.next_page()
+        iterator.update_page()
         blobs = list(iterator.page)
         self.assertEqual([blob.name for blob in blobs], ['parent/file11.txt'])
         self.assertIsNone(iterator.next_page_token)
@@ -325,7 +325,7 @@ def test_second_level(self):
 
         iterator = self.bucket.list_blobs(delimiter='/',
                                           prefix='parent/child/')
-        iterator.next_page()
+        iterator.update_page()
         blobs = list(iterator.page)
         self.assertEqual([blob.name for blob in blobs],
                          expected_names)
@@ -341,7 +341,7 @@ def test_third_level(self):
         # Exercise a layer deeper to illustrate this.
         iterator = self.bucket.list_blobs(delimiter='/',
                                           prefix='parent/child/grand/')
-        iterator.next_page()
+        iterator.update_page()
         blobs = list(iterator.page)
         self.assertEqual([blob.name for blob in blobs],
                          ['parent/child/grand/file31.txt'])