From afb851e2316903d28b19cebdc4b3a98763119f6d Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 18 Oct 2016 10:06:16 -0700 Subject: [PATCH 1/2] Moving to iterators in DNS. --- dns/google/cloud/dns/client.py | 42 +++++++++------- dns/google/cloud/dns/zone.py | 90 +++++++++++++++++++--------------- dns/unit_tests/test_client.py | 10 +++- dns/unit_tests/test_zone.py | 24 +++++++-- 4 files changed, 102 insertions(+), 64 deletions(-) diff --git a/dns/google/cloud/dns/client.py b/dns/google/cloud/dns/client.py index d645b1499423..b1355c038be2 100644 --- a/dns/google/cloud/dns/client.py +++ b/dns/google/cloud/dns/client.py @@ -18,6 +18,7 @@ from google.cloud.client import JSONClient from google.cloud.dns.connection import Connection from google.cloud.dns.zone import ManagedZone +from google.cloud.iterator import Iterator class Client(JSONClient): @@ -70,31 +71,19 @@ def list_zones(self, max_results=None, page_token=None): :param max_results: maximum number of zones to return, If not passed, defaults to a value set by the API. - :type page_token: string + :type page_token: str :param page_token: opaque marker for the next "page" of zones. If not passed, the API will return the first page of zones. - :rtype: tuple, (list, str) - :returns: list of :class:`google.cloud.dns.zone.ManagedZone`, plus a - "next page token" string: if the token is not None, - indicates that more zones can be retrieved with another - call (pass that value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of :class:`~google.cloud.dns.zone.ManagedZone` + belonging to this project. """ - params = {} - - if max_results is not None: - params['maxResults'] = max_results - - if page_token is not None: - params['pageToken'] = page_token - path = '/projects/%s/managedZones' % (self.project,) - resp = self.connection.api_request(method='GET', path=path, - query_params=params) - zones = [ManagedZone.from_api_repr(resource, self) - for resource in resp['managedZones']] - return zones, resp.get('nextPageToken') + return Iterator(client=self, path=path, items_key='managedZones', + item_to_value=_item_to_zone, page_token=page_token, + max_results=max_results) def zone(self, name, dns_name=None, description=None): """Construct a zone bound to this client. @@ -115,3 +104,18 @@ def zone(self, name, dns_name=None, description=None): """ return ManagedZone(name, dns_name, client=self, description=description) + + +def _item_to_zone(iterator, resource): + """Convert a JSON managed zone to the native object. + + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. + + :type resource: dict + :param resource: An item to be converted to a managed zone. + + :rtype: :class:`.ManagedZone` + :returns: The next managed zone in the page. + """ + return ManagedZone.from_api_repr(resource, iterator.client) diff --git a/dns/google/cloud/dns/zone.py b/dns/google/cloud/dns/zone.py index 6558a3a6a8ab..c4aa77cd93cd 100644 --- a/dns/google/cloud/dns/zone.py +++ b/dns/google/cloud/dns/zone.py @@ -13,12 +13,14 @@ # limitations under the License. """Define API ManagedZones.""" + import six from google.cloud._helpers import _rfc3339_to_datetime from google.cloud.exceptions import NotFound from google.cloud.dns.changes import Changes from google.cloud.dns.resource_record_set import ResourceRecordSet +from google.cloud.iterator import Iterator class ManagedZone(object): @@ -330,29 +332,19 @@ def list_resource_record_sets(self, max_results=None, page_token=None, :param client: the client to use. If not passed, falls back to the ``client`` stored on the current zone. - :rtype: tuple, (list, str) - :returns: list of - :class:`~.resource_record_set.ResourceRecordSet`, - plus a "next page token" string: if the token is not None, - indicates that more zones can be retrieved with another - call (pass that value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of :class:`~.resource_record_set.ResourceRecordSet` + belonging to this zone. """ - params = {} - - if max_results is not None: - params['maxResults'] = max_results - - if page_token is not None: - params['pageToken'] = page_token - + client = self._require_client(client) path = '/projects/%s/managedZones/%s/rrsets' % ( self.project, self.name) - client = self._require_client(client) - conn = client.connection - resp = conn.api_request(method='GET', path=path, query_params=params) - zones = [ResourceRecordSet.from_api_repr(resource, self) - for resource in resp['rrsets']] - return zones, resp.get('nextPageToken') + iterator = Iterator( + client=client, path=path, items_key='rrsets', + item_to_value=_item_to_resource_record_set, + page_token=page_token, max_results=max_results) + iterator.zone = self + return iterator def list_changes(self, max_results=None, page_token=None, client=None): """List change sets for this zone. @@ -373,26 +365,46 @@ def list_changes(self, max_results=None, page_token=None, client=None): :param client: the client to use. If not passed, falls back to the ``client`` stored on the current zone. - :rtype: tuple, (list, str) - :returns: list of - :class:`~.resource_record_set.ResourceRecordSet`, - plus a "next page token" string: if the token is not None, - indicates that more zones can be retrieved with another - call (pass that value as ``page_token``). + :rtype: :class:`~google.cloud.iterator.Iterator` + :returns: Iterator of :class:`~.changes.Changes` + belonging to this zone. """ - params = {} + client = self._require_client(client) + path = '/projects/%s/managedZones/%s/changes' % ( + self.project, self.name) + iterator = Iterator( + client=client, path=path, items_key='changes', + item_to_value=_item_to_changes, + page_token=page_token, max_results=max_results) + iterator.zone = self + return iterator - if max_results is not None: - params['maxResults'] = max_results - if page_token is not None: - params['pageToken'] = page_token +def _item_to_resource_record_set(iterator, resource): + """Convert a JSON resource record set value to the native object. - path = '/projects/%s/managedZones/%s/changes' % ( - self.project, self.name) - client = self._require_client(client) - conn = client.connection - resp = conn.api_request(method='GET', path=path, query_params=params) - zones = [Changes.from_api_repr(resource, self) - for resource in resp['changes']] - return zones, resp.get('nextPageToken') + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. + + :type resource: dict + :param resource: An item to be converted to a resource record set. + + :rtype: :class:`~.resource_record_set.ResourceRecordSet` + :returns: The next resource record set in the page. + """ + return ResourceRecordSet.from_api_repr(resource, iterator.zone) + + +def _item_to_changes(iterator, resource): + """Convert a JSON "changes" value to the native object. + + :type iterator: :class:`~google.cloud.iterator.Iterator` + :param iterator: The iterator that has retrieved the item. + + :type resource: dict + :param resource: An item to be converted to a "changes". + + :rtype: :class:`.Changes` + :returns: The next "changes" in the page. + """ + return Changes.from_api_repr(resource, iterator.zone) diff --git a/dns/unit_tests/test_client.py b/dns/unit_tests/test_client.py index 6c8dcf0ebd0f..e49f43084d33 100644 --- a/dns/unit_tests/test_client.py +++ b/dns/unit_tests/test_client.py @@ -132,7 +132,10 @@ def test_list_zones_defaults(self): client = self._makeOne(self.PROJECT, creds) conn = client.connection = _Connection(DATA) - zones, token = client.list_zones() + iterator = client.list_zones() + iterator.update_page() + zones = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(zones), len(DATA['managedZones'])) for found, expected in zip(zones, DATA['managedZones']): @@ -173,7 +176,10 @@ def test_list_zones_explicit(self): client = self._makeOne(self.PROJECT, creds) conn = client.connection = _Connection(DATA) - zones, token = client.list_zones(max_results=3, page_token=TOKEN) + iterator = client.list_zones(max_results=3, page_token=TOKEN) + iterator.update_page() + zones = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(zones), len(DATA['managedZones'])) for found, expected in zip(zones, DATA['managedZones']): diff --git a/dns/unit_tests/test_zone.py b/dns/unit_tests/test_zone.py index dae6453d7c0b..b65751f23394 100644 --- a/dns/unit_tests/test_zone.py +++ b/dns/unit_tests/test_zone.py @@ -440,7 +440,11 @@ def test_list_resource_record_sets_defaults(self): client = _Client(project=self.PROJECT, connection=conn) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client) - rrsets, token = zone.list_resource_record_sets() + iterator = zone.list_resource_record_sets() + self.assertIs(zone, iterator.zone) + iterator.update_page() + rrsets = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(rrsets), len(DATA['rrsets'])) for found, expected in zip(rrsets, DATA['rrsets']): @@ -489,8 +493,12 @@ def test_list_resource_record_sets_explicit(self): client2 = _Client(project=self.PROJECT, connection=conn2) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client1) - rrsets, token = zone.list_resource_record_sets( + iterator = zone.list_resource_record_sets( max_results=3, page_token=TOKEN, client=client2) + self.assertIs(zone, iterator.zone) + iterator.update_page() + rrsets = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(rrsets), len(DATA['rrsets'])) for found, expected in zip(rrsets, DATA['rrsets']): @@ -551,7 +559,11 @@ def test_list_changes_defaults(self): client = _Client(project=self.PROJECT, connection=conn) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client) - changes, token = zone.list_changes() + iterator = zone.list_changes() + self.assertIs(zone, iterator.zone) + iterator.update_page() + changes = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(changes), len(DATA['changes'])) for found, expected in zip(changes, DATA['changes']): @@ -628,8 +640,12 @@ def test_list_changes_explicit(self): client2 = _Client(project=self.PROJECT, connection=conn2) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client1) - changes, token = zone.list_changes( + iterator = zone.list_changes( max_results=3, page_token=TOKEN, client=client2) + self.assertIs(zone, iterator.zone) + iterator.update_page() + changes = list(iterator.page) + token = iterator.next_page_token self.assertEqual(len(changes), len(DATA['changes'])) for found, expected in zip(changes, DATA['changes']): From ea9c6536efa9bc3f0af96f70a0f13765b3a43f9c Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 18 Oct 2016 14:00:14 -0700 Subject: [PATCH 2/2] Consolidating DNS zone unit tests (to fix lint). --- dns/unit_tests/test_zone.py | 125 +++++++++++++++--------------------- 1 file changed, 53 insertions(+), 72 deletions(-) diff --git a/dns/unit_tests/test_zone.py b/dns/unit_tests/test_zone.py index b65751f23394..3616cc438e8a 100644 --- a/dns/unit_tests/test_zone.py +++ b/dns/unit_tests/test_zone.py @@ -517,45 +517,53 @@ def test_list_resource_record_sets_explicit(self): self.assertEqual(req['query_params'], {'maxResults': 3, 'pageToken': TOKEN}) - def test_list_changes_defaults(self): + def _get_changes(self, token, changes_name): from google.cloud._helpers import _datetime_to_rfc3339 - from google.cloud.dns.changes import Changes - from google.cloud.dns.resource_record_set import ResourceRecordSet - self._setUpConstants() - PATH = 'projects/%s/managedZones/%s/changes' % ( - self.PROJECT, self.ZONE_NAME) - TOKEN = 'TOKEN' - NAME_1 = 'www.example.com' - TYPE_1 = 'A' - TTL_1 = '86400' - RRDATAS_1 = ['123.45.67.89'] - NAME_2 = 'alias.example.com' - TYPE_2 = 'CNAME' - TTL_2 = '3600' - RRDATAS_2 = ['www.example.com'] - CHANGES_NAME = 'changeset_id' - DATA = { - 'nextPageToken': TOKEN, + + name_1 = 'www.example.com' + type_1 = 'A' + ttl_1 = '86400' + rrdatas_1 = ['123.45.67.89'] + name_2 = 'alias.example.com' + type_2 = 'CNAME' + ttl_2 = '3600' + rrdatas_2 = ['www.example.com'] + result = { 'changes': [{ 'kind': 'dns#change', - 'id': CHANGES_NAME, + 'id': changes_name, 'status': 'pending', 'startTime': _datetime_to_rfc3339(self.WHEN), 'additions': [ {'kind': 'dns#resourceRecordSet', - 'name': NAME_1, - 'type': TYPE_1, - 'ttl': TTL_1, - 'rrdatas': RRDATAS_1}], + 'name': name_1, + 'type': type_1, + 'ttl': ttl_1, + 'rrdatas': rrdatas_1}], 'deletions': [ {'kind': 'dns#change', - 'name': NAME_2, - 'type': TYPE_2, - 'ttl': TTL_2, - 'rrdatas': RRDATAS_2}], + 'name': name_2, + 'type': type_2, + 'ttl': ttl_2, + 'rrdatas': rrdatas_2}], }] } - conn = _Connection(DATA) + if token is not None: + result['nextPageToken'] = token + return result + + def test_list_changes_defaults(self): + from google.cloud.dns.changes import Changes + from google.cloud.dns.resource_record_set import ResourceRecordSet + + self._setUpConstants() + path = 'projects/%s/managedZones/%s/changes' % ( + self.PROJECT, self.ZONE_NAME) + token = 'TOKEN' + changes_name = 'changeset_id' + data = self._get_changes(token, changes_name) + + conn = _Connection(data) client = _Client(project=self.PROJECT, connection=conn) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client) @@ -565,10 +573,10 @@ def test_list_changes_defaults(self): changes = list(iterator.page) token = iterator.next_page_token - self.assertEqual(len(changes), len(DATA['changes'])) - for found, expected in zip(changes, DATA['changes']): + self.assertEqual(len(changes), len(data['changes'])) + for found, expected in zip(changes, data['changes']): self.assertIsInstance(found, Changes) - self.assertEqual(found.name, CHANGES_NAME) + self.assertEqual(found.name, changes_name) self.assertEqual(found.status, 'pending') self.assertEqual(found.started, self.WHEN) @@ -590,67 +598,40 @@ def test_list_changes_defaults(self): self.assertEqual(found_rr.ttl, int(expected_rr['ttl'])) self.assertEqual(found_rr.rrdatas, expected_rr['rrdatas']) - self.assertEqual(token, TOKEN) + self.assertEqual(token, token) self.assertEqual(len(conn._requested), 1) req = conn._requested[0] self.assertEqual(req['method'], 'GET') - self.assertEqual(req['path'], '/%s' % PATH) + self.assertEqual(req['path'], '/%s' % (path,)) def test_list_changes_explicit(self): - from google.cloud._helpers import _datetime_to_rfc3339 from google.cloud.dns.changes import Changes from google.cloud.dns.resource_record_set import ResourceRecordSet + self._setUpConstants() - PATH = 'projects/%s/managedZones/%s/changes' % ( + path = 'projects/%s/managedZones/%s/changes' % ( self.PROJECT, self.ZONE_NAME) - TOKEN = 'TOKEN' - NAME_1 = 'www.example.com' - TYPE_1 = 'A' - TTL_1 = '86400' - RRDATAS_1 = ['123.45.67.89'] - NAME_2 = 'alias.example.com' - TYPE_2 = 'CNAME' - TTL_2 = '3600' - RRDATAS_2 = ['www.example.com'] - CHANGES_NAME = 'changeset_id' - DATA = { - 'changes': [{ - 'kind': 'dns#change', - 'id': CHANGES_NAME, - 'status': 'pending', - 'startTime': _datetime_to_rfc3339(self.WHEN), - 'additions': [ - {'kind': 'dns#resourceRecordSet', - 'name': NAME_1, - 'type': TYPE_1, - 'ttl': TTL_1, - 'rrdatas': RRDATAS_1}], - 'deletions': [ - {'kind': 'dns#change', - 'name': NAME_2, - 'type': TYPE_2, - 'ttl': TTL_2, - 'rrdatas': RRDATAS_2}], - }] - } + changes_name = 'changeset_id' + data = self._get_changes(None, changes_name) conn1 = _Connection() client1 = _Client(project=self.PROJECT, connection=conn1) - conn2 = _Connection(DATA) + conn2 = _Connection(data) client2 = _Client(project=self.PROJECT, connection=conn2) zone = self._makeOne(self.ZONE_NAME, self.DNS_NAME, client1) + page_token = 'TOKEN' iterator = zone.list_changes( - max_results=3, page_token=TOKEN, client=client2) + max_results=3, page_token=page_token, client=client2) self.assertIs(zone, iterator.zone) iterator.update_page() changes = list(iterator.page) token = iterator.next_page_token - self.assertEqual(len(changes), len(DATA['changes'])) - for found, expected in zip(changes, DATA['changes']): + self.assertEqual(len(changes), len(data['changes'])) + for found, expected in zip(changes, data['changes']): self.assertIsInstance(found, Changes) - self.assertEqual(found.name, CHANGES_NAME) + self.assertEqual(found.name, changes_name) self.assertEqual(found.status, 'pending') self.assertEqual(found.started, self.WHEN) @@ -678,9 +659,9 @@ def test_list_changes_explicit(self): self.assertEqual(len(conn2._requested), 1) req = conn2._requested[0] self.assertEqual(req['method'], 'GET') - self.assertEqual(req['path'], '/%s' % PATH) + self.assertEqual(req['path'], '/%s' % (path,)) self.assertEqual(req['query_params'], - {'maxResults': 3, 'pageToken': TOKEN}) + {'maxResults': 3, 'pageToken': page_token}) class _Client(object):