Skip to content

Commit

Permalink
fix(storage): fix AllObjects condition in gRPC
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 googleapis#6205
  • Loading branch information
tritone committed Jun 28, 2023
1 parent 5cdf4e2 commit dfeaf94
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 dfeaf94

Please sign in to comment.