Capture server-generated 'writerIdentity' during 'LogSink.create' (#4707)

tseaver · web-flow · commit ccbd95c2d849 · 2018-01-05T16:37:52.000-05:00
diff --git a/google/cloud/logging/_gax.py b/google/cloud/logging/_gax.py
@@ -217,13 +217,17 @@ def sink_create(self, project, sink_name, filter_, destination,
         :param unique_writer_identity: (Optional) determines the kind of
                                        IAM identity returned as
                                        writer_identity in the new sink.
+
+        :rtype: dict
+        :returns: The sink resource returned from the API (converted from a
+                  protobuf to a dictionary).
         """
         options = None
         parent = 'projects/%s' % (project,)
         sink_pb = LogSink(name=sink_name, filter=filter_,
                           destination=destination)
         try:
-            self._gax_api.create_sink(
+            created_pb = self._gax_api.create_sink(
                 parent,
                 sink_pb,
                 unique_writer_identity=unique_writer_identity,
@@ -234,6 +238,7 @@ def sink_create(self, project, sink_name, filter_, destination,
                 path = 'projects/%s/sinks/%s' % (project, sink_name)
                 raise Conflict(path)
             raise
+        return MessageToDict(created_pb)
 
     def sink_get(self, project, sink_name):
         """API call:  retrieve a sink resource.
diff --git a/google/cloud/logging/_http.py b/google/cloud/logging/_http.py
@@ -254,6 +254,9 @@ def sink_create(self, project, sink_name, filter_, destination,
         :param unique_writer_identity: (Optional) determines the kind of
                                        IAM identity returned as
                                        writer_identity in the new sink.
+
+        :rtype: dict
+        :returns: The returned (created) resource.
         """
         target = '/projects/%s/sinks' % (project,)
         data = {
@@ -262,7 +265,7 @@ def sink_create(self, project, sink_name, filter_, destination,
             'destination': destination,
         }
         query_params = {'uniqueWriterIdentity': unique_writer_identity}
-        self.api_request(
+        return self.api_request(
             method='POST',
             path=target,
             data=data,
diff --git a/google/cloud/logging/sink.py b/google/cloud/logging/sink.py
@@ -71,6 +71,12 @@ def writer_identity(self):
         """Identity used for exports via the sink"""
         return self._writer_identity
 
+    def _update_from_api_repr(self, resource):
+        """Helper for API methods returning sink resources."""
+        self.destination = resource['destination']
+        self.filter_ = resource.get('filter')
+        self._writer_identity = resource.get('writerIdentity')
+
     @classmethod
     def from_api_repr(cls, resource, client):
         """Factory:  construct a sink given its API representation
@@ -89,10 +95,8 @@ def from_api_repr(cls, resource, client):
                  from the client.
         """
         sink_name = resource['name']
-        destination = resource['destination']
-        filter_ = resource.get('filter')
-        instance = cls(sink_name, filter_, destination, client=client)
-        instance._writer_identity = resource.get('writerIdentity')
+        instance = cls(sink_name, client=client)
+        instance._update_from_api_repr(resource)
         return instance
 
     def _require_client(self, client):
@@ -127,10 +131,11 @@ def create(self, client=None, unique_writer_identity=False):
                                     writer_identity in the new sink.
         """
         client = self._require_client(client)
-        client.sinks_api.sink_create(
+        resource = client.sinks_api.sink_create(
             self.project, self.name, self.filter_, self.destination,
             unique_writer_identity=unique_writer_identity,
         )
+        self._update_from_api_repr(resource)
 
     def exists(self, client=None):
         """API call:  test for the existence of the sink via a GET request
@@ -168,9 +173,7 @@ def reload(self, client=None):
         """
         client = self._require_client(client)
         resource = client.sinks_api.sink_get(self.project, self.name)
-        self.destination = resource['destination']
-        self.filter_ = resource.get('filter')
-        self._writer_identity = resource.get('writerIdentity')
+        self._update_from_api_repr(resource)
 
     def update(self, client=None):
         """API call:  update sink configuration via a PUT request
diff --git a/tests/unit/test__gax.py b/tests/unit/test__gax.py
@@ -619,6 +619,7 @@ class Test_SinksAPI(_Base, unittest.TestCase):
     SINK_NAME = 'sink_name'
     SINK_PATH = 'projects/%s/sinks/%s' % (_Base.PROJECT, SINK_NAME)
     DESTINATION_URI = 'faux.googleapis.com/destination'
+    SINK_WRITER_IDENTITY = 'serviceAccount:project-123@example.com'
 
     @staticmethod
     def _get_target_class():
@@ -719,6 +720,7 @@ def test_sink_create_error(self):
 
     def test_sink_create_conflict(self):
         from google.cloud.exceptions import Conflict
+        from google.cloud.proto.logging.v2.logging_config_pb2 import LogSink
 
         gax_api = _GAXSinksAPI(_create_sink_conflict=True)
         api = self._make_one(gax_api, None)
@@ -728,16 +730,7 @@ def test_sink_create_conflict(self):
                 self.PROJECT, self.SINK_NAME, self.FILTER,
                 self.DESTINATION_URI)
 
-    def test_sink_create_ok(self):
-        from google.cloud.proto.logging.v2.logging_config_pb2 import LogSink
-
-        gax_api = _GAXSinksAPI()
-        api = self._make_one(gax_api, None)
-
-        api.sink_create(
-            self.PROJECT, self.SINK_NAME, self.FILTER, self.DESTINATION_URI)
-
-        parent, sink, options, unique_writer_identity = (
+        parent, sink, unique_writer_identity, options = (
             gax_api._create_sink_called_with)
         self.assertEqual(parent, self.PROJECT_PATH)
         self.assertIsInstance(sink, LogSink)
@@ -747,24 +740,36 @@ def test_sink_create_ok(self):
         self.assertIsNone(options)
         self.assertFalse(unique_writer_identity)
 
-    def test_sink_create_with_unique_writer_identity(self):
+    def test_sink_create_ok(self):
         from google.cloud.proto.logging.v2.logging_config_pb2 import LogSink
 
         gax_api = _GAXSinksAPI()
         api = self._make_one(gax_api, None)
-        api.sink_create(
-            self.PROJECT, self.SINK_NAME, self.FILTER, self.DESTINATION_URI,
+
+        returned = api.sink_create(
+            self.PROJECT,
+            self.SINK_NAME,
+            self.FILTER,
+            self.DESTINATION_URI,
             unique_writer_identity=True,
         )
-        parent, sink, options, unique_writer_identity = (
+
+        self.assertEqual(returned, {
+            'name': self.SINK_NAME,
+            'filter': self.FILTER,
+            'destination': self.DESTINATION_URI,
+            'writerIdentity': self.SINK_WRITER_IDENTITY,
+        })
+
+        parent, sink, unique_writer_identity, options = (
             gax_api._create_sink_called_with)
         self.assertEqual(parent, self.PROJECT_PATH)
         self.assertIsInstance(sink, LogSink)
         self.assertEqual(sink.name, self.SINK_NAME)
         self.assertEqual(sink.filter, self.FILTER)
         self.assertEqual(sink.destination, self.DESTINATION_URI)
-        self.assertIsNone(options)
         self.assertTrue(unique_writer_identity)
+        self.assertIsNone(options)
 
     def test_sink_get_error(self):
         from google.cloud.exceptions import NotFound
@@ -1482,14 +1487,22 @@ def list_sinks(self, parent, page_size, options):
         self._list_sinks_called_with = parent, page_size, options
         return self._list_sinks_response
 
-    def create_sink(self, parent, sink, options, unique_writer_identity=False):
+    def create_sink(self, parent, sink, unique_writer_identity, options):
         from google.gax.errors import GaxError
+        from google.cloud.proto.logging.v2.logging_config_pb2 import LogSink
 
-        self._create_sink_called_with = parent, sink, options, unique_writer_identity
+        self._create_sink_called_with = (
+            parent, sink, unique_writer_identity, options)
         if self._random_gax_error:
             raise GaxError('error')
         if self._create_sink_conflict:
             raise GaxError('conflict', self._make_grpc_failed_precondition())
+        return LogSink(
+            name=sink.name,
+            destination=sink.destination,
+            filter=sink.filter,
+            writer_identity=Test_SinksAPI.SINK_WRITER_IDENTITY,
+        )
 
     def get_sink(self, sink_name, options):
         from google.gax.errors import GaxError
diff --git a/tests/unit/test__http.py b/tests/unit/test__http.py
@@ -334,6 +334,7 @@ class Test_SinksAPI(unittest.TestCase):
     SINK_NAME = 'sink_name'
     SINK_PATH = 'projects/%s/sinks/%s' % (PROJECT, SINK_NAME)
     DESTINATION_URI = 'faux.googleapis.com/destination'
+    WRITER_IDENTITY = 'serviceAccount:project-123@example.com'
 
     @staticmethod
     def _get_target_class():
@@ -438,7 +439,7 @@ def test_list_sinks_w_paging(self):
     def test_sink_create_conflict(self):
         from google.cloud.exceptions import Conflict
 
-        SENT = {
+        sent = {
             'name': self.SINK_NAME,
             'filter': self.FILTER,
             'destination': self.DESTINATION_URI,
@@ -453,47 +454,36 @@ def test_sink_create_conflict(self):
                 self.PROJECT, self.SINK_NAME, self.FILTER,
                 self.DESTINATION_URI)
 
-        self.assertEqual(conn._called_with['method'], 'POST')
         path = '/projects/%s/sinks' % (self.PROJECT,)
-        self.assertEqual(conn._called_with['path'], path)
-        self.assertEqual(conn._called_with['data'], SENT)
-
-    def test_sink_create_ok(self):
-        SENT = {
-            'name': self.SINK_NAME,
-            'filter': self.FILTER,
-            'destination': self.DESTINATION_URI,
+        expected = {
+            'method': 'POST',
+            'path': path,
+            'data': sent,
+            'query_params': {'uniqueWriterIdentity': False},
         }
-        conn = _Connection({})
-        client = _Client(conn)
-        api = self._make_one(client)
-
-        api.sink_create(
-            self.PROJECT, self.SINK_NAME, self.FILTER, self.DESTINATION_URI)
-
-        self.assertEqual(conn._called_with['method'], 'POST')
-        path = '/projects/%s/sinks' % (self.PROJECT,)
-        self.assertEqual(conn._called_with['path'], path)
-        self.assertEqual(conn._called_with['data'], SENT)
+        self.assertEqual(conn._called_with, expected)
 
-    def test_sink_create_unique_writer_identity(self):
+    def test_sink_create_ok(self):
         sent = {
             'name': self.SINK_NAME,
             'filter': self.FILTER,
             'destination': self.DESTINATION_URI,
         }
-
-        conn = _Connection({})
+        after_create = sent.copy()
+        after_create['writerIdentity'] = self.WRITER_IDENTITY
+        conn = _Connection(after_create)
         client = _Client(conn)
         api = self._make_one(client)
 
-        api.sink_create(
+        returned = api.sink_create(
             self.PROJECT,
             self.SINK_NAME,
             self.FILTER,
             self.DESTINATION_URI,
             unique_writer_identity=True,
         )
+
+        self.assertEqual(returned, after_create)
         path = '/projects/%s/sinks' % (self.PROJECT,)
         expected = {
             'method': 'POST',
diff --git a/tests/unit/test_sink.py b/tests/unit/test_sink.py
@@ -97,12 +97,22 @@ def test_from_api_repr_full(self):
     def test_create_w_bound_client(self):
         client = _Client(project=self.PROJECT)
         api = client.sinks_api = _DummySinksAPI()
+        api._sink_create_response = {
+            'name': self.SINK_NAME,
+            'filter': self.FILTER,
+            'destination': self.DESTINATION_URI,
+            'writerIdentity': self.WRITER_IDENTITY,
+        }
         sink = self._make_one(self.SINK_NAME, self.FILTER,
                               self.DESTINATION_URI,
                               client=client)
 
         sink.create()
 
+        self.assertEqual(sink.name, self.SINK_NAME)
+        self.assertEqual(sink.filter_, self.FILTER)
+        self.assertEqual(sink.destination, self.DESTINATION_URI)
+        self.assertEqual(sink.writer_identity, self.WRITER_IDENTITY)
         self.assertEqual(
             api._sink_create_called_with,
             (
@@ -121,9 +131,19 @@ def test_create_w_alternate_client(self):
                               self.DESTINATION_URI,
                               client=client1)
         api = client2.sinks_api = _DummySinksAPI()
+        api._sink_create_response = {
+            'name': self.SINK_NAME,
+            'filter': self.FILTER,
+            'destination': self.DESTINATION_URI,
+            'writerIdentity': self.WRITER_IDENTITY,
+        }
 
         sink.create(client=client2, unique_writer_identity=True)
 
+        self.assertEqual(sink.name, self.SINK_NAME)
+        self.assertEqual(sink.filter_, self.FILTER)
+        self.assertEqual(sink.destination, self.DESTINATION_URI)
+        self.assertEqual(sink.writer_identity, self.WRITER_IDENTITY)
         self.assertEqual(
             api._sink_create_called_with,
             (
@@ -273,6 +293,7 @@ def sink_create(self, project, sink_name, filter_, destination,
                     unique_writer_identity=False):
         self._sink_create_called_with = (
             project, sink_name, filter_, destination, unique_writer_identity)
+        return self._sink_create_response
 
     def sink_get(self, project, sink_name):
         from google.cloud.exceptions import NotFound
