From dfeaf94099065343ececf14c12cbd9154651c00b Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Wed, 28 Jun 2023 23:18:52 +0000 Subject: [PATCH] fix(storage): fix AllObjects condition in gRPC Fixes incorrect handling of zero values for the AgeDays field of bucket lifecycle conditions. Allows re-enabling the integration test for bucket create/delete for gRPC. Fixes #6205 --- storage/bucket.go | 11 +++++++---- storage/integration_test.go | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index eb1feba22ea..4da5473092d 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -1558,7 +1558,6 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { // doc states "format: int32"), so the client types used int64, // but the proto uses int32 so we have a potentially lossy // conversion. - AgeDays: proto.Int32(int32(r.Condition.AgeInDays)), DaysSinceCustomTime: proto.Int32(int32(r.Condition.DaysSinceCustomTime)), DaysSinceNoncurrentTime: proto.Int32(int32(r.Condition.DaysSinceNoncurrentTime)), MatchesPrefix: r.Condition.MatchesPrefix, @@ -1568,7 +1567,11 @@ func toProtoLifecycle(l Lifecycle) *storagepb.Bucket_Lifecycle { }, } - // TODO(#6205): This may not be needed for gRPC + // Only set AgeDays in the proto if it is non-zero, or if the user has set + // Condition.AllObjects. + if r.Condition.AgeInDays != 0 { + rr.Condition.AgeDays = proto.Int32(int32(r.Condition.AgeInDays)) + } if r.Condition.AllObjects { rr.Condition.AgeDays = proto.Int32(0) } @@ -1667,8 +1670,8 @@ func toLifecycleFromProto(rl *storagepb.Bucket_Lifecycle) Lifecycle { }, } - // TODO(#6205): This may not be needed for gRPC - if rr.GetCondition().GetAgeDays() == 0 { + // Only set Condition.AllObjects if AgeDays is zero, not if it is nil. + if rr.GetCondition().AgeDays != nil && rr.GetCondition().GetAgeDays() == 0 { r.Condition.AllObjects = true } diff --git a/storage/integration_test.go b/storage/integration_test.go index 528f3d91d23..11138394cdd 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -291,7 +291,7 @@ func multiTransportTest(ctx context.Context, t *testing.T, } func TestIntegration_BucketCreateDelete(t *testing.T) { - ctx := skipJSONReads(skipGRPC("with attrs: https://github.com/googleapis/google-cloud-go/issues/6205"), "no reads in test") + ctx := skipJSONReads(context.Background(), "no reads in test") multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) { projectID := testutil.ProjID() @@ -445,8 +445,8 @@ func TestIntegration_BucketCreateDelete(t *testing.T) { if got, want := gotAttrs.Labels, test.wantAttrs.Labels; !testutil.Equal(got, want) { t.Errorf("labels: got %v, want %v", got, want) } - if got, want := gotAttrs.Lifecycle, test.wantAttrs.Lifecycle; !testutil.Equal(got, want) { - t.Errorf("lifecycle: \ngot\t%v\nwant\t%v", got, want) + if diff := cmp.Diff(gotAttrs.Lifecycle, test.wantAttrs.Lifecycle); diff != "" { + t.Errorf("lifecycle: diff got vs. want: %v", diff) } if gotAttrs.LocationType != test.wantAttrs.LocationType { t.Errorf("location type: got %s, want %s", gotAttrs.LocationType, test.wantAttrs.LocationType)