Skip to content

Commit

Permalink
fix(storage): fix AllObjects condition in gRPC (#8184)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tritone authored Jun 29, 2023
1 parent 06553b3 commit 2b99e4f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 7 deletions.
11 changes: 7 additions & 4 deletions storage/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions storage/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 2b99e4f

Please sign in to comment.