Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(storage): ignore 0 time when converting to bucket #6807

Merged
merged 6 commits into from Oct 11, 2022
Merged

chore(storage): ignore 0 time when converting to bucket #6807

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d2639b1
chore(storage): ignore 0 time when converting to bucket
BrennaEpp Oct 4, 2022
17cdbca
remove env setting
BrennaEpp Oct 4, 2022
0024d31
Merge branch 'main' into grpc-2
BrennaEpp Oct 4, 2022
16a1e85
Merge branch 'main' into grpc-2
BrennaEpp Oct 7, 2022
a000395
use accessor method
BrennaEpp Oct 7, 2022
233d153
Merge branch 'main' into grpc-2
BrennaEpp Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion storage/bucket.go
View file
Open in desktop
Expand Up @@ -1373,7 +1373,7 @@ func toRetentionPolicy(rp *raw.BucketRetentionPolicy) (*RetentionPolicy, error)
}

func toRetentionPolicyFromProto(rp *storagepb.Bucket_RetentionPolicy) *RetentionPolicy {
if rp == nil {
if rp == nil || rp.GetEffectiveTime().AsTime().Unix() == 0 {
return nil
}
return &RetentionPolicy{
Expand Down
287 changes: 146 additions & 141 deletions storage/integration_test.go
View file
Open in desktop
Expand Up @@ -3623,180 +3623,185 @@ func TestIntegration_UpdateRetentionExpirationTime(t *testing.T) {
}

func TestIntegration_CustomTime(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}
multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, bucket string, _ string, client *Client) {
h := testHelper{t}

// Create object with CustomTime.
bkt := client.Bucket(bucketName)
obj := bkt.Object("custom-time-obj")
w := obj.NewWriter(ctx)
ct := time.Date(2020, 8, 25, 12, 12, 12, 0, time.UTC)
w.ObjectAttrs.CustomTime = ct
h.mustWrite(w, randomContents())
// Create object with CustomTime.
bkt := client.Bucket(bucket)
obj := bkt.Object("custom-time-obj")
w := obj.NewWriter(ctx)
ct := time.Date(2020, 8, 25, 12, 12, 12, 0, time.UTC)
w.ObjectAttrs.CustomTime = ct
h.mustWrite(w, randomContents())

// Validate that CustomTime has been set
checkCustomTime := func(want time.Time) error {
attrs, err := obj.Attrs(ctx)
if err != nil {
return fmt.Errorf("failed to get object attrs: %v", err)
}
if got := attrs.CustomTime; got != want {
return fmt.Errorf("CustomTime not set correctly: got %+v, want %+v", got, ct)
// Validate that CustomTime has been set
checkCustomTime := func(want time.Time) error {
attrs, err := obj.Attrs(ctx)
if err != nil {
return fmt.Errorf("failed to get object attrs: %v", err)
}
if got := attrs.CustomTime; got != want {
return fmt.Errorf("CustomTime not set correctly: got %+v, want %+v", got, ct)
}
return nil
}
return nil
}

if err := checkCustomTime(ct); err != nil {
t.Fatalf("checking CustomTime: %v", err)
}
if err := checkCustomTime(ct); err != nil {
t.Fatalf("checking CustomTime: %v", err)
}

// Update CustomTime to the future should succeed.
laterTime := ct.Add(10 * time.Hour)
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{CustomTime: laterTime}); err != nil {
t.Fatalf("updating CustomTime: %v", err)
}
// Update CustomTime to the future should succeed.
laterTime := ct.Add(10 * time.Hour)
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{CustomTime: laterTime}); err != nil {
t.Fatalf("updating CustomTime: %v", err)
}

// Update CustomTime to the past should give error.
earlierTime := ct.Add(5 * time.Hour)
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{CustomTime: earlierTime}); err == nil {
t.Fatalf("backdating CustomTime: expected error, got none")
}
// Update CustomTime to the past should give error.
earlierTime := ct.Add(5 * time.Hour)
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{CustomTime: earlierTime}); err == nil {
t.Fatalf("backdating CustomTime: expected error, got none")
}

// Zero value for CustomTime should be ignored.
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{}); err != nil {
t.Fatalf("empty update: %v", err)
}
if err := checkCustomTime(laterTime); err != nil {
t.Fatalf("after sending zero value: %v", err)
}
// Zero value for CustomTime should be ignored. Set TemporaryHold so that
// we don't send an empty update request, which is invalid for gRPC.
if _, err := obj.Update(ctx, ObjectAttrsToUpdate{TemporaryHold: false}); err != nil {
t.Fatalf("empty update: %v", err)
}
if err := checkCustomTime(laterTime); err != nil {
t.Fatalf("after sending zero value: %v", err)
}
})
}

func TestIntegration_UpdateRetentionPolicy(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}
multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) {
initial := &RetentionPolicy{RetentionPeriod: time.Minute}

initial := &RetentionPolicy{RetentionPeriod: time.Minute}
for _, test := range []struct {
desc string
input *RetentionPolicy
want *RetentionPolicy
}{
{
desc: "update",
input: &RetentionPolicy{RetentionPeriod: time.Hour},
want: &RetentionPolicy{RetentionPeriod: time.Hour},
},
{
desc: "update even with timestamp (EffectiveTime should be ignored)",
input: &RetentionPolicy{RetentionPeriod: time.Hour, EffectiveTime: time.Now()},
want: &RetentionPolicy{RetentionPeriod: time.Hour},
},
{
desc: "remove",
input: &RetentionPolicy{},
want: nil,
},
{
desc: "remove even with timestamp (EffectiveTime should be ignored)",
input: &RetentionPolicy{EffectiveTime: time.Now().Add(time.Hour)},
want: nil,
},
{
desc: "ignore",
input: nil,
want: initial,
},
} {
t.Run(test.desc, func(t *testing.T) {
h := testHelper{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)

for _, test := range []struct {
input *RetentionPolicy
want *RetentionPolicy
}{
{ // Update
input: &RetentionPolicy{RetentionPeriod: time.Hour},
want: &RetentionPolicy{RetentionPeriod: time.Hour},
},
{ // Update even with timestamp (EffectiveTime should be ignored)
input: &RetentionPolicy{RetentionPeriod: time.Hour, EffectiveTime: time.Now()},
want: &RetentionPolicy{RetentionPeriod: time.Hour},
},
{ // Remove
input: &RetentionPolicy{},
want: nil,
},
{ // Remove even with timestamp (EffectiveTime should be ignored)
input: &RetentionPolicy{EffectiveTime: time.Now()},
want: nil,
},
{ // Ignore
input: nil,
want: initial,
},
} {
bkt := client.Bucket(uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: initial})
defer h.mustDeleteBucket(bkt)
h.mustUpdateBucket(bkt, BucketAttrsToUpdate{RetentionPolicy: test.input}, h.mustBucketAttrs(bkt).MetaGeneration)
attrs := h.mustBucketAttrs(bkt)
if attrs.RetentionPolicy != nil && attrs.RetentionPolicy.EffectiveTime.Unix() == 0 {
// Should be set by the server and parsed by the client
t.Fatal("EffectiveTime should be set, but it was not")
}
if diff := testutil.Diff(attrs.RetentionPolicy, test.want, cmpopts.IgnoreTypes(time.Time{})); diff != "" {
t.Errorf("input: %v\ngot=-, want=+:\n%s", test.input, diff)
attrs := h.mustBucketAttrs(bkt)
if attrs.RetentionPolicy != nil && attrs.RetentionPolicy.EffectiveTime.Unix() == 0 {
// Should be set by the server and parsed by the client
t.Fatal("EffectiveTime should be set, but it was not")
}
if diff := testutil.Diff(attrs.RetentionPolicy, test.want, cmpopts.IgnoreTypes(time.Time{})); diff != "" {
t.Errorf("input: %v\ngot=-, want=+:\n%s", test.input, diff)
}
})
}
}
})
}

func TestIntegration_DeleteObjectInBucketWithRetentionPolicy(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}
multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) {
h := testHelper{t}

bkt := client.Bucket(uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: &RetentionPolicy{RetentionPeriod: 25 * time.Hour}})
bkt := client.Bucket(prefix + uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: &RetentionPolicy{RetentionPeriod: 25 * time.Hour}})
defer h.mustDeleteBucket(bkt)

o := bkt.Object("some-object")
if err := writeObject(ctx, o, "text/plain", []byte("hello world")); err != nil {
t.Fatal(err)
}
o := bkt.Object("some-object")
if err := writeObject(ctx, o, "text/plain", []byte("hello world")); err != nil {
t.Fatal(err)
}

if err := o.Delete(ctx); err == nil {
t.Fatal("expected to err deleting an object in a bucket with retention period, but got nil")
}
if err := o.Delete(ctx); err == nil {
t.Fatal("expected to err deleting an object in a bucket with retention period, but got nil")
}

// Remove the retention period
h.mustUpdateBucket(bkt, BucketAttrsToUpdate{RetentionPolicy: &RetentionPolicy{}}, h.mustBucketAttrs(bkt).MetaGeneration)
// Remove the retention period
h.mustUpdateBucket(bkt, BucketAttrsToUpdate{RetentionPolicy: &RetentionPolicy{}}, h.mustBucketAttrs(bkt).MetaGeneration)

// Delete with retry, as bucket metadata changes
// can take some time to propagate.
retry := func(err error) bool { return err != nil }
ctx, cancel := context.WithTimeout(ctx, time.Second*10)
defer cancel()
// Delete with retry, as bucket metadata changes
// can take some time to propagate.
retry := func(err error) bool { return err != nil }
ctx, cancel := context.WithTimeout(ctx, time.Second*10)
defer cancel()

o = o.Retryer(WithErrorFunc(retry), WithPolicy(RetryAlways))
if err := o.Delete(ctx); err != nil {
t.Fatalf("object delete: %v", err)
}
h.mustDeleteBucket(bkt)
o = o.Retryer(WithErrorFunc(retry), WithPolicy(RetryAlways))
if err := o.Delete(ctx); err != nil {
t.Fatalf("object delete: %v", err)
}
})
}

func TestIntegration_LockBucket(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}
multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) {
h := testHelper{t}

bkt := client.Bucket(uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour * 25}})
attrs := h.mustBucketAttrs(bkt)
if attrs.RetentionPolicy.IsLocked {
t.Fatal("Expected bucket to begin unlocked, but it was not")
}
err := bkt.If(BucketConditions{MetagenerationMatch: attrs.MetaGeneration}).LockRetentionPolicy(ctx)
if err != nil {
t.Fatal("could not lock", err)
}
bkt := client.Bucket(prefix + uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour * 25}})
attrs := h.mustBucketAttrs(bkt)
if attrs.RetentionPolicy.IsLocked {
t.Fatal("Expected bucket to begin unlocked, but it was not")
}
err := bkt.If(BucketConditions{MetagenerationMatch: attrs.MetaGeneration}).LockRetentionPolicy(ctx)
if err != nil {
t.Fatal("could not lock", err)
}

attrs = h.mustBucketAttrs(bkt)
if !attrs.RetentionPolicy.IsLocked {
t.Fatal("Expected bucket to be locked, but it was not")
}
attrs = h.mustBucketAttrs(bkt)
if !attrs.RetentionPolicy.IsLocked {
t.Fatal("Expected bucket to be locked, but it was not")
}

_, err = bkt.Update(ctx, BucketAttrsToUpdate{RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour}})
if err == nil {
t.Fatal("Expected error updating locked bucket, got nil")
}
_, err = bkt.Update(ctx, BucketAttrsToUpdate{RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour}})
if err == nil {
t.Fatal("Expected error updating locked bucket, got nil")
}
})
}

func TestIntegration_LockBucket_MetagenerationRequired(t *testing.T) {
ctx := context.Background()
client := testConfig(ctx, t)
defer client.Close()
h := testHelper{t}
multiTransportTest(context.Background(), t, func(t *testing.T, ctx context.Context, _ string, prefix string, client *Client) {
h := testHelper{t}

bkt := client.Bucket(uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{
RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour * 25},
bkt := client.Bucket(prefix + uidSpace.New())
h.mustCreate(bkt, testutil.ProjID(), &BucketAttrs{
RetentionPolicy: &RetentionPolicy{RetentionPeriod: time.Hour * 25},
})
err := bkt.LockRetentionPolicy(ctx)
if err == nil {
t.Fatal("expected error locking bucket without metageneration condition, got nil")
}
})
err := bkt.LockRetentionPolicy(ctx)
if err == nil {
t.Fatal("expected error locking bucket without metageneration condition, got nil")
}
}

func TestIntegration_KMS(t *testing.T) {
Expand Down