Making Subscription.reload() update the topic if unset. (#3397)

Also: removing Subscription._project and Snapshot._project since
they are never used (and they shadow data that they shouldn't).

pubsub/google/cloud/pubsub/snapshot.py

@@ -35,7 +35,6 @@ def __init__(self, name, subscription=None, topic=None, client=None):
         self._subscription = subscription
         self._client = client or getattr(
             subscription, '_client', None) or topic._client
-        self._project = self._client.project
 
     @classmethod
     def from_api_repr(cls, resource, client, topics=None):

pubsub/google/cloud/pubsub/subscription.py

@@ -86,7 +86,6 @@ def __init__(self, name, topic=None, ack_deadline=None, push_endpoint=None,
         self.name = name
         self.topic = topic
         self._client = client or topic._client
-        self._project = self._client.project
         self.ack_deadline = ack_deadline
         self.push_endpoint = push_endpoint
         self.retain_acked_messages = retain_acked_messages
@@ -274,6 +273,9 @@ def reload(self, client=None):
         self.ack_deadline = data.get('ackDeadlineSeconds')
         push_config = data.get('pushConfig', {})
         self.push_endpoint = push_config.get('pushEndpoint')
+        if self.topic is None and 'topic' in data:
+            topic_name = topic_name_from_path(data['topic'], client.project)
+            self.topic = client.topic(topic_name)
 
     def delete(self, client=None):
         """API call:  delete the subscription via a DELETE request.

pubsub/tests/unit/test__gax.py

@@ -475,7 +475,7 @@ def test_list_subscriptions_no_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertIsNone(subscription.ack_deadline)
         self.assertEqual(subscription.push_endpoint, self.PUSH_ENDPOINT)
 
@@ -523,7 +523,7 @@ def test_list_subscriptions_with_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertIsNone(subscription.ack_deadline)
         self.assertEqual(subscription.push_endpoint, self.PUSH_ENDPOINT)
 
@@ -560,7 +560,7 @@ def test_subscription_create(self):
 
     def test_subscription_create_optional_params(self):
         import datetime
-        
+
         from google.cloud.proto.pubsub.v1.pubsub_pb2 import Subscription
 
         sub_pb = Subscription(name=self.SUB_PATH, topic=self.TOPIC_PATH)
@@ -1009,7 +1009,7 @@ def test_list_snapshots_no_paging(self):
         self.assertIsInstance(snapshot.topic, Topic)
         self.assertEqual(snapshot.topic.name, self.TOPIC_NAME)
         self.assertIs(snapshot._client, client)
-        self.assertEqual(snapshot._project, self.PROJECT)
+        self.assertEqual(snapshot.project, self.PROJECT)
 
     def test_list_snapshots_with_paging(self):
         from google.cloud.proto.pubsub.v1.pubsub_pb2 import (
@@ -1047,7 +1047,7 @@ def test_list_snapshots_with_paging(self):
         self.assertIsInstance(snapshot.topic, Topic)
         self.assertEqual(snapshot.topic.name, self.TOPIC_NAME)
         self.assertIs(snapshot._client, client)
-        self.assertEqual(snapshot._project, self.PROJECT)
+        self.assertEqual(snapshot.project, self.PROJECT)
 
     def test_subscription_seek_hit(self):
         gax_api = _GAXSubscriberAPI(_seek_ok=True)
@@ -1548,7 +1548,7 @@ def delete_snapshot(self, snapshot, options=None):
             raise GaxError('error')
         if not self._delete_snapshot_ok:
             raise GaxError('miss', self._make_grpc_not_found())
-        
+
     def seek(self, subscription, time=None, snapshot=None, options=None):
         from google.gax.errors import GaxError
 

pubsub/tests/unit/test__http.py

@@ -521,7 +521,7 @@ def test_list_subscriptions_no_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertIsNone(subscription.ack_deadline)
         self.assertIsNone(subscription.push_endpoint)
 
@@ -566,7 +566,7 @@ def test_list_subscriptions_with_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertIsNone(subscription.ack_deadline)
         self.assertIsNone(subscription.push_endpoint)
 
@@ -612,7 +612,7 @@ def test_subscription_create_defaults(self):
 
     def test_subscription_create_retain_messages(self):
         import datetime
-        
+
         RESOURCE = {'topic': self.TOPIC_PATH,
                     'retainAckedMessages': True,
                     'messageRetentionDuration': {
@@ -637,7 +637,7 @@ def test_subscription_create_retain_messages(self):
         path = '/%s' % (self.SUB_PATH,)
         self.assertEqual(connection._called_with['path'], path)
         self.assertEqual(connection._called_with['data'], RESOURCE)
-        
+
     def test_subscription_create_explicit(self):
         ACK_DEADLINE = 90
         PUSH_ENDPOINT = 'https://api.example.com/push'

pubsub/tests/unit/test_client.py

@@ -280,7 +280,7 @@ def test_list_subscriptions_no_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertIsNone(subscription.ack_deadline)
         self.assertIsNone(subscription.push_endpoint)
 
@@ -334,7 +334,7 @@ def test_list_subscriptions_with_paging(self):
         self.assertIsInstance(subscription.topic, Topic)
         self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
         self.assertIs(subscription._client, client)
-        self.assertEqual(subscription._project, self.PROJECT)
+        self.assertEqual(subscription.project, self.PROJECT)
         self.assertEqual(subscription.ack_deadline, ACK_DEADLINE)
         self.assertEqual(subscription.push_endpoint, PUSH_ENDPOINT)
 
@@ -408,7 +408,7 @@ def test_subscription_factory(self):
         self.assertEqual(new_subscription.name, sub_name)
         self.assertIsNone(new_subscription.topic)
         self.assertIs(new_subscription._client, client_obj)
-        self.assertEqual(new_subscription._project, project)
+        self.assertEqual(new_subscription.project, project)
         self.assertEqual(new_subscription.ack_deadline, ack_deadline)
         self.assertEqual(new_subscription.push_endpoint, push_endpoint)
         self.assertTrue(new_subscription.retain_acked_messages)

pubsub/tests/unit/test_subscription.py

@@ -259,6 +259,29 @@ def test_reload_w_bound_client(self):
         self.assertEqual(subscription.push_endpoint, self.ENDPOINT)
         self.assertEqual(api._subscription_got, self.SUB_PATH)
 
+    def test_reload_sets_topic(self):
+        from google.cloud.pubsub.topic import Topic
+
+        response = {
+            'name': self.SUB_PATH,
+            'topic': self.TOPIC_PATH,
+            'ackDeadlineSeconds': self.DEADLINE,
+            'pushConfig': {'pushEndpoint': self.ENDPOINT},
+        }
+        client = _Client(project=self.PROJECT)
+        api = client.subscriber_api = _FauxSubscribererAPI()
+        api._subscription_get_response = response
+        subscription = self._make_one(self.SUB_NAME, client=client)
+
+        self.assertIsNone(subscription.topic)
+        subscription.reload()
+
+        self.assertEqual(subscription.ack_deadline, self.DEADLINE)
+        self.assertEqual(subscription.push_endpoint, self.ENDPOINT)
+        self.assertEqual(api._subscription_got, self.SUB_PATH)
+        self.assertIsInstance(subscription.topic, Topic)
+        self.assertEqual(subscription.topic.name, self.TOPIC_NAME)
+
     def test_reload_w_alternate_client(self):
         RESPONSE = {
             'name': self.SUB_PATH,
@@ -506,7 +529,7 @@ def test_seek_snapshot_w_alternate_client(self):
 
     def test_seek_time_w_bound_client(self):
         import datetime
-        
+
         from google.cloud import _helpers
 
         time = datetime.time()