fix(grpc): more problems with UpdateObject()/UpdateBucket() (#271)

I thought my previous tests had reproduced and fixed the problem, but
evidently this is not the case.  It all seems to be working correctly
now, tested using the C++ client library.  I also filed a bug against
the protobuf project, and another bug to cleanup this code when the
protobuf bug is fixed.

testbench/grpc_server.py

@@ -17,6 +17,7 @@
 import datetime
 import json
 import re
+from warnings import catch_warnings
 
 import crc32c
 from google.iam.v1 import iam_policy_pb2
@@ -171,9 +172,15 @@ def UpdateBucket(self, request, context):
             context,
             preconditions=testbench.common.make_grpc_bucket_preconditions(request),
         )
-        request.update_mask.MergeMessage(
-            request.bucket, bucket.metadata, replace_repeated_field=False
-        )
+        # TODO(#270) - cleanup the manual steps
+        paths = set(request.update_mask.paths)
+        safe_paths = paths.copy()
+        safe_paths.discard("acl")
+        safe_paths.discard("default_object_acl")
+        safe_paths.discard("labels")
+        mask = field_mask_pb2.FieldMask()
+        mask.paths[:] = list(safe_paths)
+        mask.MergeMessage(request.bucket, bucket.metadata)
         # Manually replace the repeated fields.
         if "acl" in request.update_mask.paths:
             del bucket.metadata.acl[:]
@@ -403,9 +410,14 @@ def UpdateObject(self, request, context):
             generation=request.object.generation,
             preconditions=testbench.common.make_grpc_preconditions(request),
         )
-        request.update_mask.MergeMessage(
-            request.object, blob.metadata, replace_repeated_field=False
-        )
+        # TODO(#270) - cleanup the manual steps
+        paths = set(request.update_mask.paths)
+        safe_paths = paths.copy()
+        safe_paths.discard("acl")
+        safe_paths.discard("metadata")
+        mask = field_mask_pb2.FieldMask()
+        mask.paths[:] = list(safe_paths)
+        mask.MergeMessage(request.object, blob.metadata)
         # Manually replace the repeated fields.
         if "acl" in request.update_mask.paths:
             del blob.metadata.acl[:]

tests/test_grpc_server.py

@@ -393,22 +393,20 @@ def test_update_bucket(self):
         # Then change some properties, note that we set some attributes but not the
         # corresponding field mask, those should not change.
         context = unittest.mock.Mock()
-        response = self.grpc.UpdateBucket(
-            storage_pb2.UpdateBucketRequest(
-                bucket=storage_pb2.Bucket(
-                    name="projects/_/buckets/bucket-name",
-                    labels={"key": "value"},
-                    storage_class="NEARLINE",
-                    rpo="ASYNC_TURBO",
-                    acl=get.acl,
-                    default_object_acl=get.default_object_acl,
-                ),
-                update_mask=field_mask_pb2.FieldMask(
-                    paths=["labels", "rpo", "acl", "default_object_acl"]
-                ),
+        request = storage_pb2.UpdateBucketRequest(
+            bucket=storage_pb2.Bucket(
+                name="projects/_/buckets/bucket-name",
+                labels={"key": "value"},
+                storage_class="NEARLINE",
+                rpo="ASYNC_TURBO",
+                acl=get.acl,
+                default_object_acl=get.default_object_acl,
+            ),
+            update_mask=field_mask_pb2.FieldMask(
+                paths=["labels", "rpo", "acl", "default_object_acl"]
             ),
-            context,
         )
+        response = self.grpc.UpdateBucket(request, context)
         self.assertEqual("projects/_/buckets/bucket-name", response.name)
         self.assertEqual({"key": "value"}, response.labels)
         self.assertEqual("STANDARD", response.storage_class)
@@ -823,30 +821,31 @@ def test_read_object(self):
     def test_update_object(self):
         media = b"How vexingly quick daft zebras jump!"
         request = testbench.common.FakeRequest(
-            args={"name": "object-name"}, data=media, headers={}, environ={}
+            args={"name": "object-name", "metadata": {"key: value"}},
+            data=media,
+            headers={},
+            environ={},
         )
         blob, _ = gcs.object.Object.init_media(request, self.bucket.metadata)
         metageneration = blob.metadata.metageneration
         self.db.insert_object("bucket-name", blob, None)
         context = unittest.mock.Mock()
-        response = self.grpc.UpdateObject(
-            storage_pb2.UpdateObjectRequest(
-                object=storage_pb2.Object(
-                    bucket="projects/_/buckets/bucket-name",
-                    name="object-name",
-                    metadata={"key": "value"},
-                    content_type="text/plain",
-                    cache_control="fancy cache",
-                    acl=blob.metadata.acl,
-                ),
-                update_mask=field_mask_pb2.FieldMask(
-                    paths=["content_type", "metadata", "acl"]
-                ),
+        request = storage_pb2.UpdateObjectRequest(
+            object=storage_pb2.Object(
+                bucket="projects/_/buckets/bucket-name",
+                name="object-name",
+                metadata={"key": "value", "new-key": "new-value"},
+                content_type="text/plain",
+                cache_control="fancy cache",
+                acl=blob.metadata.acl,
+            ),
+            update_mask=field_mask_pb2.FieldMask(
+                paths=["content_type", "metadata", "acl"]
             ),
-            context,
         )
+        response = self.grpc.UpdateObject(request, context)
         self.assertEqual("text/plain", response.content_type)
-        self.assertEqual({"key": "value"}, response.metadata)
+        self.assertEqual({"key": "value", "new-key": "new-value"}, response.metadata)
         self.assertEqual("", response.cache_control)
         self.assertLess(metageneration, response.metageneration)
 
@@ -859,7 +858,7 @@ def test_update_object(self):
             context,
         )
         self.assertEqual("text/plain", get.content_type)
-        self.assertEqual({"key": "value"}, get.metadata)
+        self.assertEqual({"key": "value", "new-key": "new-value"}, get.metadata)
         self.assertEqual("", get.cache_control)
 
     def test_update_object_invalid(self):