Merge pull request #1580 from tseaver/1485-pubsub-fix_list_topic_subs…

…criptions

Add 'Topic.list_subscriptions'.

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']
 

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:

gcloud/pubsub/_helpers.py

@@ -14,6 +14,8 @@
 
 """Helper functions for shared behavior."""
 
+from gcloud._helpers import _name_from_project_path
+
 
 def topic_name_from_path(path, project):
     """Validate a topic URI path and get the topic name.
@@ -31,15 +33,25 @@ 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 path_parts[3]
+    template = r'projects/(?P<project>\w+)/topics/(?P<name>\w+)'
+    return _name_from_project_path(path, project, template)
+
+
+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.
+    """
+    template = r'projects/(?P<project>\w+)/subscriptions/(?P<name>\w+)'
+    return _name_from_project_path(path, project, template)

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)

gcloud/pubsub/test__helpers.py

@@ -21,27 +21,23 @@ 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):
+    def test_it(self):
         TOPIC_NAME = 'TOPIC_NAME'
         PROJECT = 'PROJECT'
-        PATH = 'foo/%s/bar/%s' % (PROJECT, TOPIC_NAME)
-        self.assertRaises(ValueError, self._callFUT, PATH, PROJECT)
+        PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME)
+        topic_name = self._callFUT(PATH, PROJECT)
+        self.assertEqual(topic_name, TOPIC_NAME)
 
-    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):
+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_it(self):
         TOPIC_NAME = 'TOPIC_NAME'
         PROJECT = 'PROJECT'
-        PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME)
-        topic_name = self._callFUT(PATH, PROJECT)
-        self.assertEqual(topic_name, TOPIC_NAME)
+        PATH = 'projects/%s/subscriptions/%s' % (PROJECT, TOPIC_NAME)
+        subscription_name = self._callFUT(PATH, PROJECT)
+        self.assertEqual(subscription_name, TOPIC_NAME)

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'

gcloud/pubsub/test_topic.py

@@ -336,15 +336,122 @@ 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)
         self.assertTrue(isinstance(subscription, Subscription))
         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):