From 8cc9341b839b591b422be1437f7c502861d08848 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 17 Oct 2016 17:50:03 -0700 Subject: [PATCH] Removing Iterator and Page subclasses. Instead require `Iterator` takes: - a well-formed path for the request - a callable to convert a JSON item to native obj. - (optional) the key in a response holding all items - (optional) a `page_start` (acts as proxy for `Page.__init__`) --- core/google/cloud/iterator.py | 168 ++++++++++----- core/unit_tests/test_iterator.py | 97 +++++---- .../google/cloud/resource_manager/client.py | 50 ++--- resource_manager/unit_tests/test_client.py | 121 +++++------ storage/google/cloud/storage/bucket.py | 150 ++++++------- storage/google/cloud/storage/client.py | 58 ++--- storage/unit_tests/test_bucket.py | 202 ++++++++---------- storage/unit_tests/test_client.py | 48 ++--- 8 files changed, 409 insertions(+), 485 deletions(-) diff --git a/core/google/cloud/iterator.py b/core/google/cloud/iterator.py index 80aebff37f37..d746bc5847d8 100644 --- a/core/google/cloud/iterator.py +++ b/core/google/cloud/iterator.py @@ -17,29 +17,27 @@ 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, you may need to override the -``ITEMS_KEY`` class attribute so that a given response (containing a page of -results) can be parsed into an iterable page of the actual objects you want:: - - 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 - -You then can use this to get **all** the results from a resource:: - - >>> iterator = MyIterator(...) +To make an iterator work, you'll need to provide a way to convert a JSON +item returned from the API into the object of your choice (via +``item_to_value``). You also may need to specify a custom ``items_key`` so +that a given response (containing a page of results) can be parsed into an +iterable page of the actual objects you want. You then can use this to get +**all** the results from a resource:: + + >>> def item_to_value(iterator, item): + ... my_item = MyItemClass(iterator.client, other_arg=True) + ... my_item._set_properties(item) + ... return my_item + ... + >>> iterator = Iterator(..., items_key='blocks', + ... item_to_value=item_to_value) >>> list(iterator) # Convert to a list (consumes all values). Or you can walk your way through items and call off the search early if you find what you're looking for (resulting in possibly fewer requests):: - >>> for my_item in MyIterator(...): + >>> for my_item in Iterator(...): ... print(my_item.name) ... if not my_item.is_valid: ... break @@ -47,7 +45,7 @@ def _item_to_value(self, item): When iterating, not every new item will send a request to the server. To monitor these requests, track the current page of the iterator:: - >>> iterator = MyIterator(...) + >>> iterator = Iterator(...) >>> iterator.page_number 0 >>> next(iterator) @@ -58,6 +56,8 @@ def _item_to_value(self, item): 1 >>> next(iterator) + >>> iterator.page_number + 1 >>> iterator.page.remaining 0 >>> next(iterator) @@ -70,7 +70,7 @@ def _item_to_value(self, item): It's also possible to consume an entire page and handle the paging process manually:: - >>> iterator = MyIterator(...) + >>> iterator = Iterator(...) >>> # Manually pull down the first page. >>> iterator.update_page() >>> items = list(iterator.page) @@ -96,6 +96,8 @@ def _item_to_value(self, item): ] >>> >>> # When there are no more results + >>> iterator.next_page_token is None + True >>> iterator.update_page() >>> iterator.page is None True @@ -113,6 +115,43 @@ def _item_to_value(self, item): _PAGE_ERR_TEMPLATE = ( 'Tried to update the page while current page (%r) still has %d ' 'items remaining.') +DEFAULT_ITEMS_KEY = 'items' +"""The dictionary key used to retrieve items from each response.""" + + +# pylint: disable=unused-argument +def _not_implemented_item_to_value(iterator, item): + """Helper to convert an item into the native object. + + This is a virtual stand-in as the default value, effectively + causing callers to pass in their own callable. + + :type iterator: :class:`Iterator` + :param iterator: An iterator that holds some request info. + + :type item: dict + :param item: A JSON object to be converted into a native object. + + :raises NotImplementedError: Always. + """ + raise NotImplementedError + + +def _do_nothing_page_start(iterator, page, response): + """Helper to provide custom behavior after a :class:`Page` is started. + + This is a do-nothing stand-in as the default value. + + :type iterator: :class:`Iterator` + :param iterator: An iterator that holds some request info. + + :type page: :class:`Page` + :param page: The page that was just created. + + :type response: dict + :param response: The JSON API response for a page. + """ +# pylint: enable=unused-argument class Page(object): @@ -127,15 +166,21 @@ class Page(object): :type items_key: str :param items_key: The dictionary key used to retrieve items from the response. + + :type item_to_value: callable + :param item_to_value: Callable to convert an item from JSON + into the native object. Assumed signature + takes an :class:`Iterator` and a dictionary + holding a single item. """ - def __init__(self, parent, response, items_key): + def __init__(self, parent, response, items_key, item_to_value): self._parent = parent items = response.get(items_key, ()) self._num_items = len(items) self._remaining = self._num_items self._item_iter = iter(items) - self.response = response + self._item_to_value = item_to_value @property def num_items(self): @@ -162,7 +207,7 @@ def __iter__(self): def next(self): """Get the next value in the page.""" item = six.next(self._item_iter) - result = self._parent._item_to_value(item) + result = self._item_to_value(self._parent, item) # Since we've successfully got the next value from the # iterator, we update the number of remaining. self._remaining -= 1 @@ -175,12 +220,23 @@ def next(self): class Iterator(object): """A generic class for iterating through Cloud JSON APIs list responses. - 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. + :type path: str + :param path: The path to query for the list of items. Defaults + to :attr:`PATH` on the current iterator class. + + :type items_key: str + :param items_key: The key used to grab retrieved items from an API + response. Defaults to :data:`DEFAULT_ITEMS_KEY`. + + :type item_to_value: callable + :param item_to_value: (Optional) Callable to convert an item from JSON + into the native object. Assumed signature + takes an :class:`Iterator` and a dictionary + holding a single item. + :type page_token: str :param page_token: (Optional) A token identifying a page in a result set. @@ -191,26 +247,32 @@ class Iterator(object): :param extra_params: (Optional) Extra query string parameters for the API call. - :type path: str - :param path: (Optional) The path to query for the list of items. Defaults - to :attr:`PATH` on the current iterator class. + :type page_start: callable + :param page_start: (Optional) Callable to provide any special behavior + after a new page has been created. Assumed signature + takes the :class:`Iterator` that started the page, + the :class:`Page` that was started and the dictionary + containing the page response. """ - PAGE_TOKEN = 'pageToken' - MAX_RESULTS = 'maxResults' - RESERVED_PARAMS = frozenset([PAGE_TOKEN, MAX_RESULTS]) - PATH = None - ITEMS_KEY = 'items' - """The dictionary key used to retrieve items from each response.""" - _PAGE_CLASS = Page - - def __init__(self, client, page_token=None, max_results=None, - extra_params=None, path=None): - self.extra_params = extra_params or {} - self._verify_params() - self.max_results = max_results + _PAGE_TOKEN = 'pageToken' + _MAX_RESULTS = 'maxResults' + _RESERVED_PARAMS = frozenset([_PAGE_TOKEN, _MAX_RESULTS]) + + def __init__(self, client, path, items_key=DEFAULT_ITEMS_KEY, + item_to_value=_not_implemented_item_to_value, + page_token=None, max_results=None, extra_params=None, + page_start=_do_nothing_page_start): self.client = client - self.path = path or self.PATH + self.path = path + self._items_key = items_key + self._item_to_value = item_to_value + self.max_results = max_results + self.extra_params = extra_params + self._page_start = page_start + if self.extra_params is None: + self.extra_params = {} + self._verify_params() # The attributes below will change over the life of the iterator. self.page_number = 0 self.next_page_token = page_token @@ -222,7 +284,7 @@ def _verify_params(self): :raises ValueError: If a reserved parameter is used. """ - reserved_in_use = self.RESERVED_PARAMS.intersection( + reserved_in_use = self._RESERVED_PARAMS.intersection( self.extra_params) if reserved_in_use: raise ValueError('Using a reserved parameter', @@ -275,7 +337,9 @@ def update_page(self, require_empty=True): if page_empty: if self._has_next_page(): response = self._get_next_page_response() - self._page = self._PAGE_CLASS(self, response, self.ITEMS_KEY) + self._page = Page(self, response, self._items_key, + self._item_to_value) + self._page_start(self, self._page, response) else: self._page = None else: @@ -283,18 +347,6 @@ def update_page(self, require_empty=True): msg = _PAGE_ERR_TEMPLATE % (self._page, self.page.remaining) raise ValueError(msg) - 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 item from the request.""" self.update_page(require_empty=False) @@ -330,9 +382,9 @@ def _get_query_params(self): """ result = {} if self.next_page_token is not None: - result[self.PAGE_TOKEN] = self.next_page_token + result[self._PAGE_TOKEN] = self.next_page_token if self.max_results is not None: - result[self.MAX_RESULTS] = self.max_results - self.num_results + result[self._MAX_RESULTS] = self.max_results - self.num_results result.update(self.extra_params) return result diff --git a/core/unit_tests/test_iterator.py b/core/unit_tests/test_iterator.py index b5c224fcec7a..08fb609822cb 100644 --- a/core/unit_tests/test_iterator.py +++ b/core/unit_tests/test_iterator.py @@ -15,6 +15,28 @@ import unittest +class Test__not_implemented_item_to_value(unittest.TestCase): + + def _callFUT(self, iterator, item): + from google.cloud.iterator import _not_implemented_item_to_value + return _not_implemented_item_to_value(iterator, item) + + def test_virtual(self): + with self.assertRaises(NotImplementedError): + self._callFUT(None, None) + + +class Test__do_nothing_page_start(unittest.TestCase): + + def _callFUT(self, iterator, page, response): + from google.cloud.iterator import _do_nothing_page_start + return _do_nothing_page_start(iterator, page, response) + + def test_do_nothing(self): + result = self._callFUT(None, None, None) + self.assertIsNone(result) + + class TestPage(unittest.TestCase): def _getTargetClass(self): @@ -28,25 +50,25 @@ def test_constructor(self): parent = object() items_key = 'potatoes' response = {items_key: (1, 2, 3)} - page = self._makeOne(parent, response, items_key) + page = self._makeOne(parent, response, items_key, None) 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, {}, '', 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, {}, '', None) remaining = 1337 page._remaining = remaining self.assertEqual(page.remaining, remaining) def test___iter__(self): - page = self._makeOne(None, {}, '') + page = self._makeOne(None, {}, '', None) self.assertIs(iter(page), page) def test_iterator_calls__item_to_value(self): @@ -55,16 +77,16 @@ def test_iterator_calls__item_to_value(self): class Parent(object): calls = 0 - values = None - def _item_to_value(self, item): + def item_to_value(self, item): self.calls += 1 return item items_key = 'turkeys' response = {items_key: [10, 11, 12]} parent = Parent() - page = self._makeOne(parent, response, items_key) + page = self._makeOne(parent, response, items_key, + Parent.item_to_value) page._remaining = 100 self.assertEqual(parent.calls, 0) @@ -93,26 +115,12 @@ def test_constructor(self): connection = _Connection() client = _Client(connection) path = '/foo' - iterator = self._makeOne(client, path=path) + iterator = self._makeOne(client, path) self.assertIs(iterator.client, client) self.assertEqual(iterator.path, path) self.assertEqual(iterator.page_number, 0) self.assertIsNone(iterator.next_page_token) - def test_constructor_default_path(self): - klass = self._getTargetClass() - - class WithPath(klass): - PATH = '/path' - - connection = _Connection() - client = _Client(connection) - iterator = WithPath(client) - self.assertIs(iterator.client, client) - self.assertEqual(iterator.path, WithPath.PATH) - self.assertEqual(iterator.page_number, 0) - self.assertIsNone(iterator.next_page_token) - def test_constructor_w_extra_param_collision(self): connection = _Connection() client = _Client(connection) @@ -122,7 +130,7 @@ def test_constructor_w_extra_param_collision(self): self._makeOne(client, path=path, extra_params=extra_params) def test_page_property(self): - iterator = self._makeOne(None) + iterator = self._makeOne(None, None) page = object() iterator._page = page self.assertIs(iterator.page, page) @@ -130,13 +138,13 @@ def test_page_property(self): def test_page_property_unset(self): from google.cloud.iterator import _UNSET - iterator = self._makeOne(None) + iterator = self._makeOne(None, 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 = self._makeOne(None, None) iterator._page = None with self.assertRaises(ValueError): iterator.update_page() @@ -144,8 +152,8 @@ def test_update_page_no_more(self): def test_update_page_not_empty_success(self): from google.cloud.iterator import Page - iterator = self._makeOne(None) - page = Page(None, {}, '') + iterator = self._makeOne(None, None) + page = Page(None, {}, '', None) iterator._page = page iterator._page._remaining = 1 iterator.update_page(require_empty=False) @@ -154,14 +162,14 @@ def test_update_page_not_empty_success(self): def test_update_page_not_empty_fail(self): from google.cloud.iterator import Page - iterator = self._makeOne(None) - iterator._page = Page(None, {}, '') + iterator = self._makeOne(None, None) + iterator._page = Page(None, {}, '', None) iterator._page._remaining = 1 with self.assertRaises(ValueError): iterator.update_page(require_empty=True) def test_update_page_empty_then_no_more(self): - iterator = self._makeOne(None) + iterator = self._makeOne(None, None) # Fake that there are no more pages. iterator.page_number = 1 iterator.next_page_token = None @@ -169,7 +177,11 @@ def test_update_page_empty_then_no_more(self): self.assertIsNone(iterator.page) def test_update_page_empty_then_another(self): - iterator = self._makeOne(None) + from google.cloud._testing import _Monkey + from google.cloud import iterator as MUT + + items_key = 'its-key' + iterator = self._makeOne(None, None, items_key=items_key) # Fake the next page class. fake_page = object() page_args = [] @@ -182,10 +194,11 @@ def dummy_page_class(*args): return fake_page iterator._get_next_page_response = dummy_response - iterator._PAGE_CLASS = dummy_page_class - iterator.update_page() + with _Monkey(MUT, Page=dummy_page_class): + iterator.update_page() self.assertIs(iterator.page, fake_page) - self.assertEqual(page_args, [(iterator, {}, iterator.ITEMS_KEY)]) + self.assertEqual( + page_args, [(iterator, {}, items_key, iterator._item_to_value)]) def test___iter__(self): iterator = self._makeOne(None, None) @@ -200,17 +213,14 @@ def test_iterate(self): item1, item2 = object(), object() ITEMS = {key1: item1, key2: item2} - klass = self._getTargetClass() - - class WithItemToValue(klass): - - def _item_to_value(self, item): - return ITEMS[item['name']] + def item_to_value(iterator, item): # pylint: disable=unused-argument + return ITEMS[item['name']] connection = _Connection( {'items': [{'name': key1}, {'name': key2}]}) client = _Client(connection) - iterator = WithItemToValue(client, path=path) + iterator = self._makeOne(client, path=path, + item_to_value=item_to_value) self.assertEqual(iterator.num_results, 0) val1 = six.next(iterator) @@ -335,11 +345,6 @@ 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): from google.cloud.iterator import _UNSET diff --git a/resource_manager/google/cloud/resource_manager/client.py b/resource_manager/google/cloud/resource_manager/client.py index 5d9e50c07efe..ce4723b83413 100644 --- a/resource_manager/google/cloud/resource_manager/client.py +++ b/resource_manager/google/cloud/resource_manager/client.py @@ -141,11 +141,10 @@ def list_projects(self, filter_params=None, page_size=None): single page. If not passed, defaults to a value set by the API. - :rtype: :class:`_ProjectIterator` - :returns: A project iterator. The iterator will make multiple API - requests if you continue iterating and there are more - pages of results. Each item returned will be a. + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of all :class:`~google.cloud.resource_manager.project.Project`. + that the current user has access to. """ extra_params = {} @@ -155,40 +154,21 @@ def list_projects(self, filter_params=None, page_size=None): if filter_params is not None: extra_params['filter'] = filter_params - return _ProjectIterator(self, extra_params=extra_params) + return Iterator( + client=self, items_key='projects', path='/projects', + item_to_value=_item_to_project, extra_params=extra_params) -class _ProjectIterator(Iterator): - """An iterator over a list of Project resources. +def _item_to_project(iterator, resource): + """Convert a JSON project to the native object. - You shouldn't have to use this directly, but instead should use the - helper methods on :class:`~google.cloud.resource_manager.client.Client` - objects. + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. - :type client: :class:`~google.cloud.resource_manager.client.Client` - :param client: The client to use for making connections. + :type resource: dict + :param resource: A resource to be converted to a project. - :type page_token: str - :param page_token: (Optional) A token identifying a page in a result set. - - :type max_results: int - :param max_results: (Optional) The maximum number of results to fetch. - - :type extra_params: dict - :param extra_params: (Optional) Extra query string parameters for - the API call. + :rtype: :class:`.Project` + :returns: The next project in the page. """ - - PATH = '/projects' - ITEMS_KEY = 'projects' - - def _item_to_value(self, resource): - """Convert a JSON project to the native object. - - :type resource: dict - :param resource: An resource to be converted to a project. - - :rtype: :class:`.Project` - :returns: The next project in the page. - """ - return Project.from_api_repr(resource, client=self.client) + return Project.from_api_repr(resource, client=iterator.client) diff --git a/resource_manager/unit_tests/test_client.py b/resource_manager/unit_tests/test_client.py index a8c50b815e8d..724b48e26e44 100644 --- a/resource_manager/unit_tests/test_client.py +++ b/resource_manager/unit_tests/test_client.py @@ -15,73 +15,6 @@ import unittest -class Test__ProjectIterator(unittest.TestCase): - - def _getTargetClass(self): - from google.cloud.resource_manager.client import _ProjectIterator - return _ProjectIterator - - def _makeOne(self, *args, **kw): - return self._getTargetClass()(*args, **kw) - - def test_constructor(self): - client = object() - iterator = self._makeOne(client) - self.assertEqual(iterator.path, '/projects') - self.assertEqual(iterator.page_number, 0) - self.assertIsNone(iterator.next_page_token) - self.assertIs(iterator.client, client) - self.assertEqual(iterator.extra_params, {}) - - def test_page_empty_response(self): - from google.cloud.iterator import Page - - client = object() - iterator = self._makeOne(client) - page = Page(iterator, {}, iterator.ITEMS_KEY) - iterator._page = page - self.assertEqual(page.num_items, 0) - self.assertEqual(page.remaining, 0) - self.assertEqual(list(page), []) - - def test_page_non_empty_response(self): - from google.cloud.resource_manager.project import Project - - project_id = 'project-id' - project_name = 'My Project Name' - project_number = 12345678 - project_labels = {'env': 'prod'} - project_lifecycle_state = 'ACTIVE' - api_resource = { - 'projectId': project_id, - 'name': project_name, - 'projectNumber': project_number, - 'labels': project_labels, - 'lifecycleState': project_lifecycle_state, - } - response = {'projects': [api_resource]} - client = object() - - def dummy_response(): - return response - - iterator = self._makeOne(client) - iterator._get_next_page_response = dummy_response - - iterator.update_page() - page = iterator.page - self.assertEqual(page.num_items, 1) - project = iterator.next() - self.assertEqual(page.remaining, 0) - self.assertIsInstance(project, Project) - self.assertEqual(project.project_id, project_id) - self.assertEqual(project._client, client) - self.assertEqual(project.name, project_name) - self.assertEqual(project.number, project_number) - self.assertEqual(project.labels, project_labels) - self.assertEqual(project.status, project_lifecycle_state) - - class TestClient(unittest.TestCase): def _getTargetClass(self): @@ -145,7 +78,7 @@ def test_fetch_project(self): self.assertEqual(project.labels, labels) def test_list_projects_return_type(self): - from google.cloud.resource_manager.client import _ProjectIterator + from google.cloud.iterator import Iterator credentials = _Credentials() client = self._makeOne(credentials=credentials) @@ -153,7 +86,7 @@ def test_list_projects_return_type(self): client.connection = _Connection({}) results = client.list_projects() - self.assertIsInstance(results, _ProjectIterator) + self.assertIsInstance(results, Iterator) def test_list_projects_no_paging(self): credentials = _Credentials() @@ -283,6 +216,56 @@ def test_list_projects_with_filter(self): }, }) + def test_page_empty_response(self): + from google.cloud.iterator import Page + + credentials = _Credentials() + client = self._makeOne(credentials=credentials) + iterator = client.list_projects() + page = Page(iterator, {}, iterator._items_key, None) + iterator._page = page + self.assertEqual(page.num_items, 0) + self.assertEqual(page.remaining, 0) + self.assertEqual(list(page), []) + + def test_page_non_empty_response(self): + from google.cloud.resource_manager.project import Project + + project_id = 'project-id' + project_name = 'My Project Name' + project_number = 12345678 + project_labels = {'env': 'prod'} + project_lifecycle_state = 'ACTIVE' + api_resource = { + 'projectId': project_id, + 'name': project_name, + 'projectNumber': project_number, + 'labels': project_labels, + 'lifecycleState': project_lifecycle_state, + } + response = {'projects': [api_resource]} + credentials = _Credentials() + client = self._makeOne(credentials=credentials) + + def dummy_response(): + return response + + iterator = client.list_projects() + iterator._get_next_page_response = dummy_response + + iterator.update_page() + page = iterator.page + self.assertEqual(page.num_items, 1) + project = iterator.next() + self.assertEqual(page.remaining, 0) + self.assertIsInstance(project, Project) + self.assertEqual(project.project_id, project_id) + self.assertEqual(project._client, client) + self.assertEqual(project.name, project_name) + self.assertEqual(project.number, project_number) + self.assertEqual(project.labels, project_labels) + self.assertEqual(project.status, project_lifecycle_state) + class _Credentials(object): diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index bf06faa540f0..2c4aee98cccc 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -20,7 +20,6 @@ from google.cloud._helpers import _rfc3339_to_datetime from google.cloud.exceptions import NotFound -from google.cloud.iterator import Page from google.cloud.iterator import Iterator from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property @@ -29,76 +28,43 @@ from google.cloud.storage.blob import Blob -class _BlobPage(Page): - """Iterator for a single page of results. +def _blobs_page_start(iterator, page, response): + """Grab prefixes after a :class:`~google.cloud.iterator.Page` started. - :type parent: :class:`_BlobIterator` - :param parent: The iterator that owns the current page. + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that is currently in use. + + :type page: :class:`~google.cloud.iterator.Page` + :param page: The page that was just created. :type response: dict :param response: The JSON API response for a page of blobs. - - :type items_key: str - :param items_key: The dictionary key used to retrieve items - from the response. """ - - def __init__(self, parent, response, items_key): - super(_BlobPage, self).__init__(parent, response, items_key) - # Grab the prefixes from the response. - self.prefixes = tuple(response.get('prefixes', ())) - parent.prefixes.update(self.prefixes) + page.prefixes = tuple(response.get('prefixes', ())) + iterator.prefixes.update(page.prefixes) -class _BlobIterator(Iterator): - """An iterator listing blobs in a bucket +def _item_to_blob(iterator, item): + """Convert a JSON blob to the native object. - You shouldn't have to use this directly, but instead should use the - :class:`google.cloud.storage.blob.Bucket.list_blobs` method. + .. note:: - :type bucket: :class:`google.cloud.storage.bucket.Bucket` - :param bucket: The bucket from which to list blobs. + This assumes that the ``bucket`` attribute has been + added to the iterator after being created. - :type page_token: str - :param page_token: (Optional) A token identifying a page in a result set. + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. - :type max_results: int - :param max_results: (Optional) The maximum number of results to fetch. + :type item: dict + :param item: An item to be converted to a blob. - :type extra_params: dict or None - :param extra_params: Extra query string parameters for the API call. - - :type client: :class:`google.cloud.storage.client.Client` - :param client: Optional. The client to use for making connections. - Defaults to the bucket's client. + :rtype: :class:`.Blob` + :returns: The next blob in the page. """ - - _PAGE_CLASS = _BlobPage - - def __init__(self, bucket, page_token=None, max_results=None, - extra_params=None, client=None): - if client is None: - client = bucket.client - self.bucket = bucket - self.prefixes = set() - super(_BlobIterator, self).__init__( - client=client, path=bucket.path + '/o', - page_token=page_token, max_results=max_results, - extra_params=extra_params) - - def _item_to_value(self, item): - """Convert a JSON blob to the native object. - - :type item: dict - :param item: An item to be converted to a blob. - - :rtype: :class:`.Blob` - :returns: The next blob in the page. - """ - name = item.get('name') - blob = Blob(name, bucket=self.bucket) - blob._set_properties(item) - return blob + name = item.get('name') + blob = Blob(name, bucket=iterator.bucket) + blob._set_properties(item) + return blob class Bucket(_PropertyMixin): @@ -111,7 +77,6 @@ class Bucket(_PropertyMixin): :type name: string :param name: The name of the bucket. """ - _iterator_class = _BlobIterator _MAX_OBJECTS_FOR_ITERATION = 256 """Maximum number of existing objects allowed in iteration. @@ -283,42 +248,44 @@ def list_blobs(self, max_results=None, page_token=None, prefix=None, projection='noAcl', fields=None, client=None): """Return an iterator used to find blobs in the bucket. - :type max_results: integer or ``NoneType`` - :param max_results: maximum number of blobs to return. + :type max_results: int + :param max_results: (Optional) Maximum number of blobs to return. - :type page_token: string - :param page_token: opaque marker for the next "page" of blobs. If not - passed, will return the first page of blobs. + :type page_token: str + :param page_token: (Optional) Opaque marker for the next "page" of + blobs. If not passed, will return the first page + of blobs. - :type prefix: string or ``NoneType`` - :param prefix: optional prefix used to filter blobs. + :type prefix: str + :param prefix: (Optional) prefix used to filter blobs. - :type delimiter: string or ``NoneType`` - :param delimiter: optional delimter, used with ``prefix`` to + :type delimiter: str + :param delimiter: (Optional) Delimiter, used with ``prefix`` to emulate hierarchy. - :type versions: boolean or ``NoneType`` - :param versions: whether object versions should be returned as - separate blobs. + :type versions: bool + :param versions: (Optional) Whether object versions should be returned + as separate blobs. - :type projection: string or ``NoneType`` - :param projection: If used, must be 'full' or 'noAcl'. Defaults to - 'noAcl'. Specifies the set of properties to return. + :type projection: str + :param projection: (Optional) If used, must be 'full' or 'noAcl'. + Defaults to ``'noAcl'``. Specifies the set of + properties to return. - :type fields: string or ``NoneType`` - :param fields: Selector specifying which fields to include in a - partial response. Must be a list of fields. For example - to get a partial response with just the next page token - and the language of each blob returned: - 'items/contentLanguage,nextPageToken' + :type fields: str + :param fields: (Optional) Selector specifying which fields to include + in a partial response. Must be a list of fields. For + example to get a partial response with just the next + page token and the language of each blob returned: + ``'items/contentLanguage,nextPageToken'``. - :type client: :class:`~google.cloud.storage.client.Client` or - ``NoneType`` - :param client: Optional. The client to use. If not passed, falls back + :type client: :class:`~google.cloud.storage.client.Client` + :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. - :rtype: :class:`_BlobIterator`. - :returns: An iterator of blobs. + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of all :class:`~google.cloud.storage.blob.Blob` + in this bucket matching the arguments. """ extra_params = {} @@ -336,10 +303,15 @@ def list_blobs(self, max_results=None, page_token=None, prefix=None, if fields is not None: extra_params['fields'] = fields - result = self._iterator_class( - self, page_token=page_token, max_results=max_results, - extra_params=extra_params, client=client) - return result + client = self._require_client(client) + path = self.path + '/o' + iterator = Iterator( + client=client, path=path, item_to_value=_item_to_blob, + page_token=page_token, max_results=max_results, + extra_params=extra_params, page_start=_blobs_page_start) + iterator.bucket = self + iterator.prefixes = set() + return iterator def delete(self, force=False, client=None): """Delete this bucket. diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 96c7b88a660d..a65770054495 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -250,9 +250,9 @@ def list_buckets(self, max_results=None, page_token=None, prefix=None, and the language of each bucket returned: 'items/id,nextPageToken' - :rtype: iterable of :class:`google.cloud.storage.bucket.Bucket` - objects. - :returns: All buckets belonging to this project. + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of all :class:`~google.cloud.storage.bucket.Bucket` + belonging to this project. """ extra_params = {'project': self.project} @@ -264,45 +264,25 @@ def list_buckets(self, max_results=None, page_token=None, prefix=None, if fields is not None: extra_params['fields'] = fields - result = _BucketIterator( - client=self, page_token=page_token, - max_results=max_results, extra_params=extra_params) + return Iterator( + client=self, path='/b', item_to_value=_item_to_bucket, + page_token=page_token, max_results=max_results, + extra_params=extra_params) - return result +def _item_to_bucket(iterator, item): + """Convert a JSON bucket to the native object. -class _BucketIterator(Iterator): - """An iterator listing all buckets. + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. - You shouldn't have to use this directly, but instead should use the - helper methods on :class:`~google.cloud.storage.connection.Connection` - objects. + :type item: dict + :param item: An item to be converted to a bucket. - :type client: :class:`~google.cloud.storage.client.Client` - :param client: The client to use for making connections. - - :type page_token: str - :param page_token: (Optional) A token identifying a page in a result set. - - :type max_results: int - :param max_results: (Optional) The maximum number of results to fetch. - - :type extra_params: dict or ``NoneType`` - :param extra_params: Extra query string parameters for the API call. + :rtype: :class:`.Bucket` + :returns: The next bucket in the page. """ - - PATH = '/b' - - def _item_to_value(self, item): - """Convert a JSON bucket to the native object. - - :type item: dict - :param item: An item to be converted to a bucket. - - :rtype: :class:`.Bucket` - :returns: The next bucket in the page. - """ - name = item.get('name') - bucket = Bucket(self.client, name) - bucket._set_properties(item) - return bucket + name = item.get('name') + bucket = Bucket(iterator.client, name) + bucket._set_properties(item) + return bucket diff --git a/storage/unit_tests/test_bucket.py b/storage/unit_tests/test_bucket.py index 2f0c160b6162..c887c2bc7929 100644 --- a/storage/unit_tests/test_bucket.py +++ b/storage/unit_tests/test_bucket.py @@ -15,106 +15,6 @@ import unittest -class Test__BlobIterator(unittest.TestCase): - - def _getTargetClass(self): - from google.cloud.storage.bucket import _BlobIterator - return _BlobIterator - - def _makeOne(self, *args, **kw): - return self._getTargetClass()(*args, **kw) - - def test_ctor(self): - connection = _Connection() - client = _Client(connection) - bucket = _Bucket() - iterator = self._makeOne(bucket, client=client) - self.assertIs(iterator.bucket, bucket) - self.assertIs(iterator.client, client) - self.assertEqual(iterator.path, '%s/o' % bucket.path) - self.assertEqual(iterator.page_number, 0) - self.assertIsNone(iterator.next_page_token) - self.assertEqual(iterator.prefixes, set()) - - def test_page_empty_response(self): - from google.cloud.iterator import Page - - connection = _Connection() - client = _Client(connection) - bucket = _Bucket() - iterator = self._makeOne(bucket, client=client) - page = Page(iterator, {}, iterator.ITEMS_KEY) - iterator._page = page - blobs = list(page) - self.assertEqual(blobs, []) - self.assertEqual(iterator.prefixes, set()) - - def test_page_non_empty_response(self): - from google.cloud.storage.blob import Blob - - blob_name = 'blob-name' - response = {'items': [{'name': blob_name}], 'prefixes': ['foo']} - connection = _Connection() - client = _Client(connection) - bucket = _Bucket() - - def dummy_response(): - return response - - iterator = self._makeOne(bucket, client=client) - iterator._get_next_page_response = dummy_response - - iterator.update_page() - page = iterator.page - self.assertEqual(page.prefixes, ('foo',)) - self.assertEqual(page.num_items, 1) - blob = iterator.next() - self.assertEqual(page.remaining, 0) - self.assertIsInstance(blob, Blob) - self.assertEqual(blob.name, blob_name) - self.assertEqual(iterator.prefixes, set(['foo'])) - - def test_cumulative_prefixes(self): - from google.cloud.storage.blob import Blob - - BLOB_NAME = 'blob-name1' - response1 = { - 'items': [{'name': BLOB_NAME}], - 'prefixes': ['foo'], - } - response2 = { - 'items': [], - 'prefixes': ['bar'], - } - connection = _Connection() - client = _Client(connection) - bucket = _Bucket() - responses = [response1, response2] - - def dummy_response(): - return responses.pop(0) - - iterator = self._makeOne(bucket, client=client) - iterator._get_next_page_response = dummy_response - - # Parse first response. - iterator.update_page() - page1 = iterator.page - self.assertEqual(page1.prefixes, ('foo',)) - self.assertEqual(page1.num_items, 1) - blob = iterator.next() - self.assertEqual(page1.remaining, 0) - self.assertIsInstance(blob, Blob) - self.assertEqual(blob.name, BLOB_NAME) - self.assertEqual(iterator.prefixes, set(['foo'])) - # Parse second response. - iterator.update_page() - page2 = iterator.page - self.assertEqual(page2.prefixes, ('bar',)) - self.assertEqual(page2.num_items, 0) - self.assertEqual(iterator.prefixes, set(['foo', 'bar'])) - - class Test_Bucket(unittest.TestCase): def _makeOne(self, client=None, name=None, properties=None): @@ -972,8 +872,9 @@ def test_make_public_w_future_reload_default(self): self._make_public_w_future_helper(default_object_acl_loaded=False) def test_make_public_recursive(self): + from google.cloud._testing import _Monkey from google.cloud.storage.acl import _ACLEntity - from google.cloud.storage.bucket import _BlobIterator + from google.cloud.storage import bucket as MUT _saved = [] @@ -999,9 +900,8 @@ def save(self, client=None): _saved.append( (self._bucket, self._name, self._granted, client)) - class _Iterator(_BlobIterator): - def _item_to_value(self, item): - return _Blob(self.bucket, item['name']) + def item_to_blob(self, item): + return _Blob(self.bucket, item['name']) NAME = 'name' BLOB_NAME = 'blob-name' @@ -1012,8 +912,9 @@ def _item_to_value(self, item): bucket = self._makeOne(client=client, name=NAME) bucket.acl.loaded = True bucket.default_object_acl.loaded = True - bucket._iterator_class = _Iterator - bucket.make_public(recursive=True) + + with _Monkey(MUT, _item_to_blob=item_to_blob): + bucket.make_public(recursive=True) self.assertEqual(list(bucket.acl), permissive) self.assertEqual(list(bucket.default_object_acl), []) self.assertEqual(_saved, [(bucket, BLOB_NAME, True, None)]) @@ -1054,6 +955,87 @@ def test_make_public_recursive_too_many(self): bucket._MAX_OBJECTS_FOR_ITERATION = 1 self.assertRaises(ValueError, bucket.make_public, recursive=True) + def test_page_empty_response(self): + from google.cloud.iterator import Page + + connection = _Connection() + client = _Client(connection) + name = 'name' + bucket = self._makeOne(client=client, name=name) + iterator = bucket.list_blobs() + page = Page(iterator, {}, iterator._items_key, None) + iterator._page = page + blobs = list(page) + self.assertEqual(blobs, []) + self.assertEqual(iterator.prefixes, set()) + + def test_page_non_empty_response(self): + from google.cloud.storage.blob import Blob + + blob_name = 'blob-name' + response = {'items': [{'name': blob_name}], 'prefixes': ['foo']} + connection = _Connection() + client = _Client(connection) + name = 'name' + bucket = self._makeOne(client=client, name=name) + + def dummy_response(): + return response + + iterator = bucket.list_blobs() + iterator._get_next_page_response = dummy_response + + iterator.update_page() + page = iterator.page + self.assertEqual(page.prefixes, ('foo',)) + self.assertEqual(page.num_items, 1) + blob = iterator.next() + self.assertEqual(page.remaining, 0) + self.assertIsInstance(blob, Blob) + self.assertEqual(blob.name, blob_name) + self.assertEqual(iterator.prefixes, set(['foo'])) + + def test_cumulative_prefixes(self): + from google.cloud.storage.blob import Blob + + BLOB_NAME = 'blob-name1' + response1 = { + 'items': [{'name': BLOB_NAME}], + 'prefixes': ['foo'], + } + response2 = { + 'items': [], + 'prefixes': ['bar'], + } + connection = _Connection() + client = _Client(connection) + name = 'name' + bucket = self._makeOne(client=client, name=name) + responses = [response1, response2] + + def dummy_response(): + return responses.pop(0) + + iterator = bucket.list_blobs() + iterator._get_next_page_response = dummy_response + + # Parse first response. + iterator.update_page() + page1 = iterator.page + self.assertEqual(page1.prefixes, ('foo',)) + self.assertEqual(page1.num_items, 1) + blob = iterator.next() + self.assertEqual(page1.remaining, 0) + self.assertIsInstance(blob, Blob) + self.assertEqual(blob.name, BLOB_NAME) + self.assertEqual(iterator.prefixes, set(['foo'])) + # Parse second response. + iterator.update_page() + page2 = iterator.page + self.assertEqual(page2.prefixes, ('bar',)) + self.assertEqual(page2.num_items, 0) + self.assertEqual(iterator.prefixes, set(['foo', 'bar'])) + class _Connection(object): _delete_bucket = False @@ -1089,14 +1071,6 @@ def api_request(self, **kw): return response -class _Bucket(object): - path = '/b/name' - name = 'name' - - def __init__(self, client=None): - self.client = client - - class _Client(object): def __init__(self, connection, project=None): diff --git a/storage/unit_tests/test_client.py b/storage/unit_tests/test_client.py index 3cc272c8cb41..5cd6e2b1694e 100644 --- a/storage/unit_tests/test_client.py +++ b/storage/unit_tests/test_client.py @@ -369,47 +369,31 @@ def test_list_buckets_all_arguments(self): uri_parts = urlparse(URI) self.assertEqual(parse_qs(uri_parts.query), EXPECTED_QUERY) - -class Test__BucketIterator(unittest.TestCase): - - def _getTargetClass(self): - from google.cloud.storage.client import _BucketIterator - return _BucketIterator - - def _makeOne(self, *args, **kw): - return self._getTargetClass()(*args, **kw) - - def test_ctor(self): - connection = object() - client = _Client(connection) - iterator = self._makeOne(client) - self.assertEqual(iterator.path, '/b') - self.assertEqual(iterator.page_number, 0) - self.assertIsNone(iterator.next_page_token) - self.assertIs(iterator.client, client) - def test_page_empty_response(self): from google.cloud.iterator import Page - connection = object() - client = _Client(connection) - iterator = self._makeOne(client) - page = Page(iterator, {}, iterator.ITEMS_KEY) + project = 'PROJECT' + credentials = _Credentials() + client = self._makeOne(project=project, credentials=credentials) + iterator = client.list_buckets() + page = Page(iterator, {}, iterator._items_key, None) iterator._page = page self.assertEqual(list(page), []) def test_page_non_empty_response(self): from google.cloud.storage.bucket import Bucket - BLOB_NAME = 'blob-name' - response = {'items': [{'name': BLOB_NAME}]} - connection = object() - client = _Client(connection) + project = 'PROJECT' + credentials = _Credentials() + client = self._makeOne(project=project, credentials=credentials) + + blob_name = 'blob-name' + response = {'items': [{'name': blob_name}]} def dummy_response(): return response - iterator = self._makeOne(client) + iterator = client.list_buckets() iterator._get_next_page_response = dummy_response iterator.update_page() @@ -418,7 +402,7 @@ def dummy_response(): bucket = iterator.next() self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) - self.assertEqual(bucket.name, BLOB_NAME) + self.assertEqual(bucket.name, blob_name) class _Credentials(object): @@ -446,9 +430,3 @@ def __init__(self, headers, content): def request(self, **kw): self._called_with = kw return self._response, self._content - - -class _Client(object): - - def __init__(self, connection): - self.connection = connection