From f9e9103eeafe36ee62de4928e996d11aabe00880 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 7 Mar 2016 13:43:51 -0500 Subject: [PATCH 1/4] Add 'Topic.list_subscriptions'. Make it handle the fact that the API returns only subscription paths, not resources. Drop the 'topic_name' argument to 'Client.list_subscriptions'. Fixes #1485. --- docs/pubsub-usage.rst | 4 +- gcloud/pubsub/_helpers.py | 31 +++++++++ gcloud/pubsub/client.py | 13 +--- gcloud/pubsub/test__helpers.py | 32 ++++++++++ gcloud/pubsub/test_client.py | 41 ------------ gcloud/pubsub/test_topic.py | 111 ++++++++++++++++++++++++++++++++- gcloud/pubsub/topic.py | 46 ++++++++++++++ system_tests/pubsub.py | 8 +-- 8 files changed, 225 insertions(+), 61 deletions(-) diff --git a/docs/pubsub-usage.rst b/docs/pubsub-usage.rst index a18b981f416d..99d01d2cde50 100644 --- a/docs/pubsub-usage.rst +++ b/docs/pubsub-usage.rst @@ -186,8 +186,8 @@ List subscriptions for a topic: >>> from gcloud import pubsub >>> client = pubsub.Client() - >>> subscriptions, next_page_token = client.list_subscriptions( - ... topic_name='topic_name') # API request + >>> topic = client.topic('topic_name') + >>> subscriptions, next_page_token = topic.list_subscriptions() # API request >>> [subscription.name for subscription in subscriptions] ['subscription_name'] diff --git a/gcloud/pubsub/_helpers.py b/gcloud/pubsub/_helpers.py index dad877c0f91b..3d70a6e8a45d 100644 --- a/gcloud/pubsub/_helpers.py +++ b/gcloud/pubsub/_helpers.py @@ -43,3 +43,34 @@ def topic_name_from_path(path, project): 'project from resource.') return path_parts[3] + + +def subscription_name_from_path(path, project): + """Validate a subscription URI path and get the subscription name. + + :type path: string + :param path: URI path for a subscription API request. + + :type project: string + :param project: The project associated with the request. It is + included for validation purposes. + + :rtype: string + :returns: subscription name parsed from ``path``. + :raises: :class:`ValueError` if the ``path`` is ill-formed or if + the project from the ``path`` does not agree with the + ``project`` passed in. + """ + # PATH = 'projects/%s/subscriptions/%s' % (PROJECT, TOPIC_NAME) + path_parts = path.split('/') + if (len(path_parts) != 4 or path_parts[0] != 'projects' or + path_parts[2] != 'subscriptions'): + raise ValueError( + 'Expected path to be of the form ' + 'projects/{project}/subscriptions/{subscription_name}') + if (len(path_parts) != 4 or path_parts[0] != 'projects' or + path_parts[2] != 'subscriptions' or path_parts[1] != project): + raise ValueError('Project from client should agree with ' + 'project from resource.') + + return path_parts[3] diff --git a/gcloud/pubsub/client.py b/gcloud/pubsub/client.py index 3e1374927a70..ecafae99af7d 100644 --- a/gcloud/pubsub/client.py +++ b/gcloud/pubsub/client.py @@ -80,8 +80,7 @@ def list_topics(self, page_size=None, page_token=None): for resource in resp.get('topics', ())] return topics, resp.get('nextPageToken') - def list_subscriptions(self, page_size=None, page_token=None, - topic_name=None): + def list_subscriptions(self, page_size=None, page_token=None): """List subscriptions for the project associated with this client. See: @@ -99,10 +98,6 @@ def list_subscriptions(self, page_size=None, page_token=None, passed, the API will return the first page of topics. - :type topic_name: string - :param topic_name: limit results to subscriptions bound to the given - topic. - :rtype: tuple, (list, str) :returns: list of :class:`gcloud.pubsub.subscription.Subscription`, plus a "next page token" string: if not None, indicates that @@ -117,11 +112,7 @@ def list_subscriptions(self, page_size=None, page_token=None, if page_token is not None: params['pageToken'] = page_token - if topic_name is None: - path = '/projects/%s/subscriptions' % (self.project,) - else: - path = '/projects/%s/topics/%s/subscriptions' % (self.project, - topic_name) + path = '/projects/%s/subscriptions' % (self.project,) resp = self.connection.api_request(method='GET', path=path, query_params=params) diff --git a/gcloud/pubsub/test__helpers.py b/gcloud/pubsub/test__helpers.py index 514883a922d8..65bb3c6f3359 100644 --- a/gcloud/pubsub/test__helpers.py +++ b/gcloud/pubsub/test__helpers.py @@ -45,3 +45,35 @@ def test_valid_data(self): PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME) topic_name = self._callFUT(PATH, PROJECT) self.assertEqual(topic_name, TOPIC_NAME) + + +class Test_subscription_name_from_path(unittest2.TestCase): + + def _callFUT(self, path, project): + from gcloud.pubsub._helpers import subscription_name_from_path + return subscription_name_from_path(path, project) + + def test_invalid_path_length(self): + PATH = 'projects/foo' + PROJECT = None + self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) + + def test_invalid_path_format(self): + TOPIC_NAME = 'TOPIC_NAME' + PROJECT = 'PROJECT' + PATH = 'foo/%s/bar/%s' % (PROJECT, TOPIC_NAME) + self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) + + def test_invalid_project(self): + TOPIC_NAME = 'TOPIC_NAME' + PROJECT1 = 'PROJECT1' + PROJECT2 = 'PROJECT2' + PATH = 'projects/%s/subscriptions/%s' % (PROJECT1, TOPIC_NAME) + self.assertRaises(ValueError, self._callFUT, PATH, PROJECT2) + + def test_valid_data(self): + TOPIC_NAME = 'TOPIC_NAME' + PROJECT = 'PROJECT' + PATH = 'projects/%s/subscriptions/%s' % (PROJECT, TOPIC_NAME) + subscription_name = self._callFUT(PATH, PROJECT) + self.assertEqual(subscription_name, TOPIC_NAME) diff --git a/gcloud/pubsub/test_client.py b/gcloud/pubsub/test_client.py index 32b4674a7172..54d54cc72162 100644 --- a/gcloud/pubsub/test_client.py +++ b/gcloud/pubsub/test_client.py @@ -196,47 +196,6 @@ def test_list_subscriptions_w_missing_key(self): self.assertEqual(req['path'], '/projects/%s/subscriptions' % PROJECT) self.assertEqual(req['query_params'], {}) - def test_list_subscriptions_with_topic_name(self): - from gcloud.pubsub.subscription import Subscription - PROJECT = 'PROJECT' - CREDS = _Credentials() - - CLIENT_OBJ = self._makeOne(project=PROJECT, credentials=CREDS) - - SUB_NAME_1 = 'subscription_1' - SUB_PATH_1 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_1) - SUB_NAME_2 = 'subscription_2' - SUB_PATH_2 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_2) - TOPIC_NAME = 'topic_name' - TOPIC_PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME) - SUB_INFO = [{'name': SUB_PATH_1, 'topic': TOPIC_PATH}, - {'name': SUB_PATH_2, 'topic': TOPIC_PATH}] - TOKEN = 'TOKEN' - RETURNED = {'subscriptions': SUB_INFO, 'nextPageToken': TOKEN} - # Replace the connection on the client with one of our own. - CLIENT_OBJ.connection = _Connection(RETURNED) - - # Execute request. - subscriptions, next_page_token = CLIENT_OBJ.list_subscriptions( - topic_name=TOPIC_NAME) - # Test values are correct. - self.assertEqual(len(subscriptions), 2) - self.assertTrue(isinstance(subscriptions[0], Subscription)) - self.assertEqual(subscriptions[0].name, SUB_NAME_1) - self.assertEqual(subscriptions[0].topic.name, TOPIC_NAME) - self.assertTrue(isinstance(subscriptions[1], Subscription)) - self.assertEqual(subscriptions[1].name, SUB_NAME_2) - self.assertEqual(subscriptions[1].topic.name, TOPIC_NAME) - self.assertTrue(subscriptions[1].topic is subscriptions[0].topic) - self.assertEqual(next_page_token, TOKEN) - self.assertEqual(len(CLIENT_OBJ.connection._requested), 1) - req = CLIENT_OBJ.connection._requested[0] - self.assertEqual(req['method'], 'GET') - self.assertEqual(req['path'], - '/projects/%s/topics/%s/subscriptions' - % (PROJECT, TOPIC_NAME)) - self.assertEqual(req['query_params'], {}) - def test_topic(self): PROJECT = 'PROJECT' TOPIC_NAME = 'TOPIC_NAME' diff --git a/gcloud/pubsub/test_topic.py b/gcloud/pubsub/test_topic.py index b390104c26e2..fa0c908efacb 100644 --- a/gcloud/pubsub/test_topic.py +++ b/gcloud/pubsub/test_topic.py @@ -336,8 +336,7 @@ def test_subscription(self): TOPIC_NAME = 'topic_name' PROJECT = 'PROJECT' CLIENT = _Client(project=PROJECT) - topic = self._makeOne(TOPIC_NAME, - client=CLIENT) + topic = self._makeOne(TOPIC_NAME, client=CLIENT) SUBSCRIPTION_NAME = 'subscription_name' subscription = topic.subscription(SUBSCRIPTION_NAME) @@ -345,6 +344,114 @@ def test_subscription(self): self.assertEqual(subscription.name, SUBSCRIPTION_NAME) self.assertTrue(subscription.topic is topic) + def test_list_subscriptions_no_paging(self): + from gcloud.pubsub.subscription import Subscription + TOPIC_NAME = 'topic_name' + PROJECT = 'PROJECT' + SUB_NAME_1 = 'subscription_1' + SUB_PATH_1 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_1) + SUB_NAME_2 = 'subscription_2' + SUB_PATH_2 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_2) + TOPIC_NAME = 'topic_name' + SUBS_LIST = [SUB_PATH_1, SUB_PATH_2] + TOKEN = 'TOKEN' + RETURNED = {'subscriptions': SUBS_LIST, 'nextPageToken': TOKEN} + + conn = _Connection(RETURNED) + CLIENT = _Client(project=PROJECT, connection=conn) + topic = self._makeOne(TOPIC_NAME, client=CLIENT) + + # Execute request. + subscriptions, next_page_token = topic.list_subscriptions() + # Test values are correct. + self.assertEqual(len(subscriptions), 2) + + subscription = subscriptions[0] + self.assertTrue(isinstance(subscription, Subscription)) + self.assertEqual(subscriptions[0].name, SUB_NAME_1) + self.assertTrue(subscription.topic is topic) + + subscription = subscriptions[1] + self.assertTrue(isinstance(subscription, Subscription)) + self.assertEqual(subscriptions[1].name, SUB_NAME_2) + self.assertTrue(subscription.topic is topic) + + self.assertEqual(next_page_token, TOKEN) + self.assertEqual(len(conn._requested), 1) + req = conn._requested[0] + self.assertEqual(req['method'], 'GET') + self.assertEqual(req['path'], + '/projects/%s/topics/%s/subscriptions' + % (PROJECT, TOPIC_NAME)) + self.assertEqual(req['query_params'], {}) + + def test_list_subscriptions_with_paging(self): + from gcloud.pubsub.subscription import Subscription + TOPIC_NAME = 'topic_name' + PROJECT = 'PROJECT' + SUB_NAME_1 = 'subscription_1' + SUB_PATH_1 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_1) + SUB_NAME_2 = 'subscription_2' + SUB_PATH_2 = 'projects/%s/subscriptions/%s' % (PROJECT, SUB_NAME_2) + TOPIC_NAME = 'topic_name' + SUBS_LIST = [SUB_PATH_1, SUB_PATH_2] + PAGE_SIZE = 10 + TOKEN = 'TOKEN' + RETURNED = {'subscriptions': SUBS_LIST} + + conn = _Connection(RETURNED) + CLIENT = _Client(project=PROJECT, connection=conn) + topic = self._makeOne(TOPIC_NAME, client=CLIENT) + + # Execute request. + subscriptions, next_page_token = topic.list_subscriptions( + page_size=PAGE_SIZE, page_token=TOKEN) + # Test values are correct. + self.assertEqual(len(subscriptions), 2) + + subscription = subscriptions[0] + self.assertTrue(isinstance(subscription, Subscription)) + self.assertEqual(subscriptions[0].name, SUB_NAME_1) + self.assertTrue(subscription.topic is topic) + + subscription = subscriptions[1] + self.assertTrue(isinstance(subscription, Subscription)) + self.assertEqual(subscriptions[1].name, SUB_NAME_2) + self.assertTrue(subscription.topic is topic) + + self.assertEqual(next_page_token, None) + self.assertEqual(len(conn._requested), 1) + req = conn._requested[0] + self.assertEqual(req['method'], 'GET') + self.assertEqual(req['path'], + '/projects/%s/topics/%s/subscriptions' + % (PROJECT, TOPIC_NAME)) + self.assertEqual(req['query_params'], + {'pageSize': PAGE_SIZE, 'pageToken': TOKEN}) + + def test_list_subscriptions_missing_key(self): + TOPIC_NAME = 'topic_name' + PROJECT = 'PROJECT' + TOPIC_NAME = 'topic_name' + + conn = _Connection({}) + CLIENT = _Client(project=PROJECT, connection=conn) + topic = self._makeOne(TOPIC_NAME, client=CLIENT) + + # Execute request. + subscriptions, next_page_token = topic.list_subscriptions() + # Test values are correct. + self.assertEqual(len(subscriptions), 0) + self.assertEqual(next_page_token, None) + + self.assertEqual(len(conn._requested), 1) + req = conn._requested[0] + self.assertEqual(req['method'], 'GET') + self.assertEqual(req['path'], + '/projects/%s/topics/%s/subscriptions' + % (PROJECT, TOPIC_NAME)) + self.assertEqual(req['query_params'], {}) + class TestBatch(unittest2.TestCase): diff --git a/gcloud/pubsub/topic.py b/gcloud/pubsub/topic.py index c4ce30645938..b2143b86be72 100644 --- a/gcloud/pubsub/topic.py +++ b/gcloud/pubsub/topic.py @@ -19,6 +19,7 @@ from gcloud._helpers import _datetime_to_rfc3339 from gcloud._helpers import _NOW from gcloud.exceptions import NotFound +from gcloud.pubsub._helpers import subscription_name_from_path from gcloud.pubsub._helpers import topic_name_from_path from gcloud.pubsub.subscription import Subscription @@ -212,6 +213,51 @@ def delete(self, client=None): client = self._require_client(client) client.connection.api_request(method='DELETE', path=self.path) + def list_subscriptions(self, page_size=None, page_token=None, client=None): + """List subscriptions for the project associated with this client. + + See: + https://cloud.google.com/pubsub/reference/rest/v1/projects.topics.subscriptions/list + + :type page_size: int + :param page_size: maximum number of topics to return, If not passed, + defaults to a value set by the API. + + :type page_token: string + :param page_token: opaque marker for the next "page" of topics. If not + passed, the API will return the first page of + topics. + + :type client: :class:`gcloud.pubsub.client.Client` or ``NoneType`` + :param client: the client to use. If not passed, falls back to the + ``client`` stored on the current topic. + + :rtype: tuple, (list, str) + :returns: list of :class:`gcloud.pubsub.subscription.Subscription`, + plus a "next page token" string: if not None, indicates that + more topics can be retrieved with another call (pass that + value as ``page_token``). + """ + client = self._require_client(client) + params = {} + + if page_size is not None: + params['pageSize'] = page_size + + if page_token is not None: + params['pageToken'] = page_token + + path = '/projects/%s/topics/%s/subscriptions' % ( + self.project, self.name) + + resp = client.connection.api_request(method='GET', path=path, + query_params=params) + subscriptions = [] + for sub_path in resp.get('subscriptions', ()): + sub_name = subscription_name_from_path(sub_path, self.project) + subscriptions.append(Subscription(sub_name, self)) + return subscriptions, resp.get('nextPageToken') + class Batch(object): """Context manager: collect messages to publish via a single API call. diff --git a/system_tests/pubsub.py b/system_tests/pubsub.py index b4e73f161779..956a788c6d36 100644 --- a/system_tests/pubsub.py +++ b/system_tests/pubsub.py @@ -118,8 +118,7 @@ def test_list_subscriptions(self): self.assertFalse(topic.exists()) topic.create() self.to_delete.append(topic) - empty, _ = Config.CLIENT.list_subscriptions( - topic_name=DEFAULT_TOPIC_NAME) + empty, _ = topic.list_subscriptions() self.assertEqual(len(empty), 0) subscriptions_to_create = [ 'new%d' % (1000 * time.time(),), @@ -132,10 +131,9 @@ def test_list_subscriptions(self): self.to_delete.append(subscription) # Retrieve the subscriptions. - all_subscriptions, _ = Config.CLIENT.list_subscriptions() + all_subscriptions, _ = topic.list_subscriptions() created = [subscription for subscription in all_subscriptions - if subscription.name in subscriptions_to_create and - subscription.topic.name == DEFAULT_TOPIC_NAME] + if subscription.name in subscriptions_to_create] self.assertEqual(len(created), len(subscriptions_to_create)) def test_message_pull_mode_e2e(self): From 40a72be52291a3dabdaf9293e0a1ebc5683618f8 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 8 Mar 2016 17:02:19 -0500 Subject: [PATCH 2/4] Factor out guts of '{topic,subscription}_name_from_path'. Use a passed-in regex, in spite of Zawinski's law. Put the factored-out code in 'gcloud._helpers', because I already know I need it in logging. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1580#discussion_r55428044 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1580#discussion_r55428077 https://github.com/GoogleCloudPlatform/gcloud-python/pull/1580#discussion_r55428197. --- gcloud/_helpers.py | 40 +++++++++++++++++++++++++++++++++- gcloud/pubsub/_helpers.py | 37 +++++++++++-------------------- gcloud/pubsub/test__helpers.py | 40 ++-------------------------------- gcloud/test__helpers.py | 35 +++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 63 deletions(-) diff --git a/gcloud/_helpers.py b/gcloud/_helpers.py index 6d9cf168e8ec..9def3c122c2a 100644 --- a/gcloud/_helpers.py +++ b/gcloud/_helpers.py @@ -19,9 +19,10 @@ import calendar import datetime import os -from threading import local as Local +import re import socket import sys +from threading import local as Local from google.protobuf import timestamp_pb2 import six @@ -388,6 +389,43 @@ def _datetime_to_pb_timestamp(when): return timestamp_pb2.Timestamp(seconds=seconds, nanos=nanos) +def _name_from_project_path(path, project, template): + """Validate a URI path and get the leaf object's name. + + :type path: string + :param path: URI path containing the name. + + :type project: string + :param project: The project associated with the request. It is + included for validation purposes. + + :type template: string + :param template: Template regex describing the expected form of the path. + The regex must have two named groups, 'project' and + 'name'. + + :rtype: string + :returns: Name parsed from ``path``. + :raises: :class:`ValueError` if the ``path`` is ill-formed or if + the project from the ``path`` does not agree with the + ``project`` passed in. + """ + if isinstance(template, str): + template = re.compile(template) + + match = template.match(path) + + if not match: + raise ValueError('path did not match: %s' % (template.pattern)) + + found_project = match.group('project') + if found_project != project: + raise ValueError('Project from client should agree with ' + 'project from resource.') + + return match.group('name') + + try: from pytz import UTC # pylint: disable=unused-import,wrong-import-order except ImportError: diff --git a/gcloud/pubsub/_helpers.py b/gcloud/pubsub/_helpers.py index 3d70a6e8a45d..77346bd44fe2 100644 --- a/gcloud/pubsub/_helpers.py +++ b/gcloud/pubsub/_helpers.py @@ -14,6 +14,14 @@ """Helper functions for shared behavior.""" +import re + +from gcloud._helpers import _name_from_project_path + + +_TOPIC_PATH_TEMPLATE = re.compile( + r'projects/(?P\w+)/topics/(?P\w+)') + def topic_name_from_path(path, project): """Validate a topic URI path and get the topic name. @@ -31,18 +39,11 @@ def topic_name_from_path(path, project): the project from the ``path`` does not agree with the ``project`` passed in. """ - # PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME) - path_parts = path.split('/') - if (len(path_parts) != 4 or path_parts[0] != 'projects' or - path_parts[2] != 'topics'): - raise ValueError('Expected path to be of the form ' - 'projects/{project}/topics/{topic_name}') - if (len(path_parts) != 4 or path_parts[0] != 'projects' or - path_parts[2] != 'topics' or path_parts[1] != project): - raise ValueError('Project from client should agree with ' - 'project from resource.') + return _name_from_project_path(path, project, _TOPIC_PATH_TEMPLATE) + - return path_parts[3] +_SUBSCRIPTION_PATH_TEMPLATE = re.compile( + r'projects/(?P\w+)/subscriptions/(?P\w+)') def subscription_name_from_path(path, project): @@ -61,16 +62,4 @@ def subscription_name_from_path(path, project): the project from the ``path`` does not agree with the ``project`` passed in. """ - # PATH = 'projects/%s/subscriptions/%s' % (PROJECT, TOPIC_NAME) - path_parts = path.split('/') - if (len(path_parts) != 4 or path_parts[0] != 'projects' or - path_parts[2] != 'subscriptions'): - raise ValueError( - 'Expected path to be of the form ' - 'projects/{project}/subscriptions/{subscription_name}') - if (len(path_parts) != 4 or path_parts[0] != 'projects' or - path_parts[2] != 'subscriptions' or path_parts[1] != project): - raise ValueError('Project from client should agree with ' - 'project from resource.') - - return path_parts[3] + return _name_from_project_path(path, project, _SUBSCRIPTION_PATH_TEMPLATE) diff --git a/gcloud/pubsub/test__helpers.py b/gcloud/pubsub/test__helpers.py index 65bb3c6f3359..efa6bcef74e6 100644 --- a/gcloud/pubsub/test__helpers.py +++ b/gcloud/pubsub/test__helpers.py @@ -21,25 +21,7 @@ def _callFUT(self, path, project): from gcloud.pubsub._helpers import topic_name_from_path return topic_name_from_path(path, project) - def test_invalid_path_length(self): - PATH = 'projects/foo' - PROJECT = None - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) - - def test_invalid_path_format(self): - TOPIC_NAME = 'TOPIC_NAME' - PROJECT = 'PROJECT' - PATH = 'foo/%s/bar/%s' % (PROJECT, TOPIC_NAME) - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) - - def test_invalid_project(self): - TOPIC_NAME = 'TOPIC_NAME' - PROJECT1 = 'PROJECT1' - PROJECT2 = 'PROJECT2' - PATH = 'projects/%s/topics/%s' % (PROJECT1, TOPIC_NAME) - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT2) - - def test_valid_data(self): + def test_it(self): TOPIC_NAME = 'TOPIC_NAME' PROJECT = 'PROJECT' PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME) @@ -53,25 +35,7 @@ def _callFUT(self, path, project): from gcloud.pubsub._helpers import subscription_name_from_path return subscription_name_from_path(path, project) - def test_invalid_path_length(self): - PATH = 'projects/foo' - PROJECT = None - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) - - def test_invalid_path_format(self): - TOPIC_NAME = 'TOPIC_NAME' - PROJECT = 'PROJECT' - PATH = 'foo/%s/bar/%s' % (PROJECT, TOPIC_NAME) - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT) - - def test_invalid_project(self): - TOPIC_NAME = 'TOPIC_NAME' - PROJECT1 = 'PROJECT1' - PROJECT2 = 'PROJECT2' - PATH = 'projects/%s/subscriptions/%s' % (PROJECT1, TOPIC_NAME) - self.assertRaises(ValueError, self._callFUT, PATH, PROJECT2) - - def test_valid_data(self): + def test_it(self): TOPIC_NAME = 'TOPIC_NAME' PROJECT = 'PROJECT' PATH = 'projects/%s/subscriptions/%s' % (PROJECT, TOPIC_NAME) diff --git a/gcloud/test__helpers.py b/gcloud/test__helpers.py index 5b6f329d2cb8..4f2cb849c89d 100644 --- a/gcloud/test__helpers.py +++ b/gcloud/test__helpers.py @@ -526,6 +526,41 @@ def test_it(self): self.assertEqual(self._callFUT(dt_stamp), timestamp) +class Test__name_from_project_path(unittest2.TestCase): + + PROJECT = 'PROJECT' + THING_NAME = 'THING_NAME' + TEMPLATE = r'projects/(?P\w+)/things/(?P\w+)' + + def _callFUT(self, path, project, template): + from gcloud._helpers import _name_from_project_path + return _name_from_project_path(path, project, template) + + def test_w_invalid_path_length(self): + PATH = 'projects/foo' + with self.assertRaises(ValueError): + self._callFUT(PATH, None, self.TEMPLATE) + + def test_w_invalid_path_segments(self): + PATH = 'foo/%s/bar/%s' % (self.PROJECT, self.THING_NAME) + with self.assertRaises(ValueError): + self._callFUT(PATH, self.PROJECT, self.TEMPLATE) + + def test_w_mismatched_project(self): + PROJECT1 = 'PROJECT1' + PROJECT2 = 'PROJECT2' + PATH = 'projects/%s/things/%s' % (PROJECT1, self.THING_NAME) + with self.assertRaises(ValueError): + self._callFUT(PATH, PROJECT2, self.TEMPLATE) + + def test_w_valid_data_w_compiled_regex(self): + import re + template = re.compile(self.TEMPLATE) + PATH = 'projects/%s/things/%s' % (self.PROJECT, self.THING_NAME) + name = self._callFUT(PATH, self.PROJECT, template) + self.assertEqual(name, self.THING_NAME) + + class _AppIdentity(object): def __init__(self, app_id): From 30ace605340460d4c219266f491f209308b58461 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 8 Mar 2016 20:05:44 -0500 Subject: [PATCH 3/4] Make string formatting args actually a tuple. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1580#discussion_r55454851 --- gcloud/_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcloud/_helpers.py b/gcloud/_helpers.py index 9def3c122c2a..8b0d7f266c4b 100644 --- a/gcloud/_helpers.py +++ b/gcloud/_helpers.py @@ -416,7 +416,7 @@ def _name_from_project_path(path, project, template): match = template.match(path) if not match: - raise ValueError('path did not match: %s' % (template.pattern)) + raise ValueError('path did not match: %s' % (template.pattern,)) found_project = match.group('project') if found_project != project: From d846dd3afda8195859b3b46bdc6341a8f70d4195 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Tue, 8 Mar 2016 20:20:00 -0500 Subject: [PATCH 4/4] Inline path template patterns as locals. Addresses: https://github.com/GoogleCloudPlatform/gcloud-python/pull/1580#discussion_r55454910 --- gcloud/pubsub/_helpers.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/gcloud/pubsub/_helpers.py b/gcloud/pubsub/_helpers.py index 77346bd44fe2..3cb07fd452eb 100644 --- a/gcloud/pubsub/_helpers.py +++ b/gcloud/pubsub/_helpers.py @@ -14,15 +14,9 @@ """Helper functions for shared behavior.""" -import re - from gcloud._helpers import _name_from_project_path -_TOPIC_PATH_TEMPLATE = re.compile( - r'projects/(?P\w+)/topics/(?P\w+)') - - def topic_name_from_path(path, project): """Validate a topic URI path and get the topic name. @@ -39,11 +33,8 @@ def topic_name_from_path(path, project): the project from the ``path`` does not agree with the ``project`` passed in. """ - return _name_from_project_path(path, project, _TOPIC_PATH_TEMPLATE) - - -_SUBSCRIPTION_PATH_TEMPLATE = re.compile( - r'projects/(?P\w+)/subscriptions/(?P\w+)') + template = r'projects/(?P\w+)/topics/(?P\w+)' + return _name_from_project_path(path, project, template) def subscription_name_from_path(path, project): @@ -62,4 +53,5 @@ def subscription_name_from_path(path, project): the project from the ``path`` does not agree with the ``project`` passed in. """ - return _name_from_project_path(path, project, _SUBSCRIPTION_PATH_TEMPLATE) + template = r'projects/(?P\w+)/subscriptions/(?P\w+)' + return _name_from_project_path(path, project, template)