diff --git a/storage/grpc_client.go b/storage/grpc_client.go index c5e2e3983d94..eae804c6ed2b 100644 --- a/storage/grpc_client.go +++ b/storage/grpc_client.go @@ -382,6 +382,13 @@ func (c *grpcStorageClient) UpdateBucket(ctx context.Context, bucket string, uat req.UpdateMask = fieldMask + if len(fieldMask.Paths) < 1 { + // Nothing to update. Send a get request for current attrs instead. This + // maintains consistency with JSON bucket updates. + opts = append(opts, idempotent(true)) + return c.GetBucket(ctx, bucket, conds, opts...) + } + var battrs *BucketAttrs err := run(ctx, func(ctx context.Context) error { res, err := c.raw.UpdateBucket(ctx, req, s.gax...) @@ -602,6 +609,17 @@ func (c *grpcStorageClient) UpdateObject(ctx context.Context, params *updateObje req.UpdateMask = fieldMask + if len(fieldMask.Paths) < 1 { + // Nothing to update. To maintain consistency with JSON, we must still + // update the object because metageneration and other fields are + // updated even on an empty update. + // gRPC will fail if the fieldmask is empty, so instead we add an + // output-only field to the update mask. Output-only fields are (and must + // be - see AIP 161) ignored, but allow us to send an empty update because + // any mask that is valid for read (as this one is) must be valid for write. + fieldMask.Paths = append(fieldMask.Paths, "create_time") + } + var attrs *ObjectAttrs err := run(ctx, func(ctx context.Context) error { res, err := c.raw.UpdateObject(ctx, req, s.gax...) diff --git a/storage/integration_test.go b/storage/integration_test.go index 954a4825ea1a..168c2ff8fcaa 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -623,6 +623,15 @@ func TestIntegration_BucketUpdate(t *testing.T) { if !testutil.Equal(attrs.StorageClass, wantStorageClass) { t.Fatalf("got %v, want %v", attrs.StorageClass, wantStorageClass) } + + // Empty update should succeed without changing the bucket. + gotAttrs, err := b.Update(ctx, BucketAttrsToUpdate{}) + if err != nil { + t.Fatalf("empty update: %v", err) + } + if !testutil.Equal(attrs, gotAttrs) { + t.Fatalf("empty update: got %v, want %v", gotAttrs, attrs) + } }) } @@ -1610,6 +1619,17 @@ func TestIntegration_ObjectUpdate(t *testing.T) { if !updated.Created.Before(updated.Updated) { t.Errorf("updated.Updated should be newer than update.Created") } + + // Test empty update. Most fields should be unchanged, but updating will + // increase the metageneration and update time. + wantAttrs := updated + gotAttrs, err := o.Update(ctx, ObjectAttrsToUpdate{}) + if err != nil { + t.Fatalf("empty update: %v", err) + } + if diff := testutil.Diff(gotAttrs, wantAttrs, cmpopts.IgnoreFields(ObjectAttrs{}, "Etag", "Metageneration", "Updated")); diff != "" { + t.Errorf("empty update: got=-, want=+:\n%s", diff) + } }) } @@ -3746,8 +3766,7 @@ func TestIntegration_UpdateCORS(t *testing.T) { bkt := client.Bucket(prefix + uidSpace.New()) h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{CORS: initialSettings}) defer h.mustDeleteBucket(bkt) - // Set VersioningEnabled so that we don't send an empty update/patch request, which is invalid for gRPC - h.mustUpdateBucket(bkt, BucketAttrsToUpdate{CORS: test.input, VersioningEnabled: false}, h.mustBucketAttrs(bkt).MetaGeneration) + h.mustUpdateBucket(bkt, BucketAttrsToUpdate{CORS: test.input}, h.mustBucketAttrs(bkt).MetaGeneration) attrs := h.mustBucketAttrs(bkt) if diff := testutil.Diff(attrs.CORS, test.want); diff != "" { t.Errorf("input: %v\ngot=-, want=+:\n%s", test.input, diff) @@ -3976,8 +3995,7 @@ func TestIntegration_UpdateRetentionPolicy(t *testing.T) { bkt := client.Bucket(prefix + uidSpace.New()) h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: initial}) defer h.mustDeleteBucket(bkt) - // Set VersioningEnabled so that we don't send an empty update request, which is invalid for gRPC - h.mustUpdateBucket(bkt, BucketAttrsToUpdate{RetentionPolicy: test.input, VersioningEnabled: false}, h.mustBucketAttrs(bkt).MetaGeneration) + h.mustUpdateBucket(bkt, BucketAttrsToUpdate{RetentionPolicy: test.input}, h.mustBucketAttrs(bkt).MetaGeneration) attrs := h.mustBucketAttrs(bkt) if attrs.RetentionPolicy != nil && attrs.RetentionPolicy.EffectiveTime.Unix() == 0 {