Merge pull request googleapis#1592 from tseaver/fix-topic-subscription-path-patterns

tseaver · tseaver · commit 72617b3b0cb2 · 2016-03-09T17:45:02.000-05:00
Fix topic subscription path patterns
diff --git a/gcloud/_helpers.py b/gcloud/_helpers.py
@@ -416,12 +416,14 @@ 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 "%s" did not match expected pattern "%s"' % (
+            path, template.pattern,))
 
     found_project = match.group('project')
     if found_project != project:
-        raise ValueError('Project from client should agree with '
-                         'project from resource.')
+        raise ValueError(
+            'Project from client (%s) should agree with '
+            'project from resource(%s).' % (project, found_project))
 
     return match.group('name')
 
diff --git a/gcloud/pubsub/_helpers.py b/gcloud/pubsub/_helpers.py
@@ -14,9 +14,27 @@
 
 """Helper functions for shared behavior."""
 
+import re
+
 from gcloud._helpers import _name_from_project_path
 
 
+_TOPIC_TEMPLATE = re.compile(r"""
+    projects/            # static prefix
+    (?P<project>[^/]+)   # initial letter, wordchars + hyphen
+    /topics/             # static midfix
+    (?P<name>[^/]+)      # initial letter, wordchars + allowed punc
+""", re.VERBOSE)
+
+
+_SUBSCRIPTION_TEMPLATE = re.compile(r"""
+    projects/            # static prefix
+    (?P<project>[^/]+)   # initial letter, wordchars + hyphen
+    /subscriptions/      # static midfix
+    (?P<name>[^/]+)      # initial letter, wordchars + allowed punc
+""", re.VERBOSE)
+
+
 def topic_name_from_path(path, project):
     """Validate a topic URI path and get the topic name.
 
@@ -33,8 +51,7 @@ def topic_name_from_path(path, project):
              the project from the ``path`` does not agree with the
              ``project`` passed in.
     """
-    template = r'projects/(?P<project>\w+)/topics/(?P<name>\w+)'
-    return _name_from_project_path(path, project, template)
+    return _name_from_project_path(path, project, _TOPIC_TEMPLATE)
 
 
 def subscription_name_from_path(path, project):
@@ -53,5 +70,4 @@ def subscription_name_from_path(path, project):
              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)
+    return _name_from_project_path(path, project, _SUBSCRIPTION_TEMPLATE)
diff --git a/gcloud/pubsub/test__helpers.py b/gcloud/pubsub/test__helpers.py
@@ -21,9 +21,16 @@ def _callFUT(self, path, project):
         from gcloud.pubsub._helpers import topic_name_from_path
         return topic_name_from_path(path, project)
 
-    def test_it(self):
+    def test_w_simple_name(self):
         TOPIC_NAME = 'TOPIC_NAME'
-        PROJECT = 'PROJECT'
+        PROJECT = 'my-project-1234'
+        PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME)
+        topic_name = self._callFUT(PATH, PROJECT)
+        self.assertEqual(topic_name, TOPIC_NAME)
+
+    def test_w_name_w_all_extras(self):
+        TOPIC_NAME = 'TOPIC_NAME-part.one~part.two%part-three'
+        PROJECT = 'my-project-1234'
         PATH = 'projects/%s/topics/%s' % (PROJECT, TOPIC_NAME)
         topic_name = self._callFUT(PATH, PROJECT)
         self.assertEqual(topic_name, TOPIC_NAME)
@@ -35,9 +42,16 @@ 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/subscriptions/%s' % (PROJECT, TOPIC_NAME)
+    def test_w_simple_name(self):
+        SUBSCRIPTION_NAME = 'SUBSCRIPTION_NAME'
+        PROJECT = 'my-project-1234'
+        PATH = 'projects/%s/subscriptions/%s' % (PROJECT, SUBSCRIPTION_NAME)
         subscription_name = self._callFUT(PATH, PROJECT)
-        self.assertEqual(subscription_name, TOPIC_NAME)
+        self.assertEqual(subscription_name, SUBSCRIPTION_NAME)
+
+    def test_w_name_w_all_extras(self):
+        SUBSCRIPTION_NAME = 'SUBSCRIPTION_NAME-part.one~part.two%part-three'
+        PROJECT = 'my-project-1234'
+        PATH = 'projects/%s/subscriptions/%s' % (PROJECT, SUBSCRIPTION_NAME)
+        topic_name = self._callFUT(PATH, PROJECT)
+        self.assertEqual(topic_name, SUBSCRIPTION_NAME)
