fix(storage): allow empty soft delete on Create (#10394)

tritone · web-flow · commit d8bd2c1ffc4f · 2024-06-19T12:50:20.000Z
Currently setting an empty soft delete policy to disable the feature does not work on bucket creation (only update). This fixes the issue by forcing the library to send a zero value in this case (sending a null does not seem to work). Also adds this case to the relevant integration test. Fixes #10380
diff --git a/storage/bucket.go b/storage/bucket.go
@@ -2123,8 +2123,11 @@ func (p *SoftDeletePolicy) toRawSoftDeletePolicy() *raw.BucketSoftDeletePolicy {
 		return nil
 	}
 	// Excluding read only field EffectiveTime.
+	// ForceSendFields must be set to send a zero value for RetentionDuration and disable
+	// soft delete.
 	return &raw.BucketSoftDeletePolicy{
 		RetentionDurationSeconds: int64(p.RetentionDuration.Seconds()),
+		ForceSendFields:          []string{"RetentionDurationSeconds"},
 	}
 }
 
diff --git a/storage/bucket_test.go b/storage/bucket_test.go
@@ -172,7 +172,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
 		Logging:               &raw.BucketLogging{LogBucket: "lb", LogObjectPrefix: "p"},
 		Website:               &raw.BucketWebsite{MainPageSuffix: "mps", NotFoundPage: "404"},
 		Autoclass:             &raw.BucketAutoclass{Enabled: true, TerminalStorageClass: "NEARLINE"},
-		SoftDeletePolicy:      &raw.BucketSoftDeletePolicy{RetentionDurationSeconds: 60 * 60},
+		SoftDeletePolicy:      &raw.BucketSoftDeletePolicy{RetentionDurationSeconds: 60 * 60, ForceSendFields: []string{"RetentionDurationSeconds"}},
 		HierarchicalNamespace: &raw.BucketHierarchicalNamespace{Enabled: true},
 		Lifecycle: &raw.BucketLifecycle{
 			Rule: []*raw.BucketLifecycleRule{{
@@ -448,7 +448,7 @@ func TestBucketAttrsToUpdateToRawBucket(t *testing.T) {
 		Website:          &raw.BucketWebsite{MainPageSuffix: "mps", NotFoundPage: "404"},
 		StorageClass:     "NEARLINE",
 		Autoclass:        &raw.BucketAutoclass{Enabled: true, TerminalStorageClass: "ARCHIVE", ForceSendFields: []string{"Enabled"}},
-		SoftDeletePolicy: &raw.BucketSoftDeletePolicy{RetentionDurationSeconds: 3600},
+		SoftDeletePolicy: &raw.BucketSoftDeletePolicy{RetentionDurationSeconds: 3600, ForceSendFields: []string{"RetentionDurationSeconds"}},
 		ForceSendFields:  []string{"DefaultEventBasedHold", "Lifecycle", "Autoclass"},
 	}
 	if msg := testutil.Diff(got, want); msg != "" {
diff --git a/storage/integration_test.go b/storage/integration_test.go
@@ -4377,7 +4377,23 @@ func TestIntegration_SoftDelete(t *testing.T) {
 			t.Fatalf("effective time of soft delete policy should not be in the past, got: %v, test start: %v", got.EffectiveTime, testStart.UTC())
 		}
 
-		// Update the soft delete policy.
+		// Create a second bucket with an empty soft delete policy to verify
+		// that this leads to no retention.
+		b2 := client.Bucket(prefix + uidSpace.New())
+		if err := b2.Create(ctx, testutil.ProjID(), &BucketAttrs{SoftDeletePolicy: &SoftDeletePolicy{}}); err != nil {
+			t.Fatalf("error creating bucket with soft delete disabled: %v", err)
+		}
+		t.Cleanup(func() { h.mustDeleteBucket(b2) })
+
+		attrs2, err := b2.Attrs(ctx)
+		if err != nil {
+			t.Fatalf("Attrs(%q): %v", b2.name, err)
+		}
+		if got, expect := attrs2.SoftDeletePolicy.RetentionDuration, time.Duration(0); got != expect {
+			t.Fatalf("mismatching retention duration; got: %+v, expected: %+v", got, expect)
+		}
+
+		// Update the soft delete policy of the original bucket.
 		policy.RetentionDuration = time.Hour * 24 * 9
 
 		attrs, err = b.Update(ctx, BucketAttrsToUpdate{SoftDeletePolicy: policy})

Original file line number	Diff line number	Diff line change
`@@ -2123,8 +2123,11 @@ func (p SoftDeletePolicy) toRawSoftDeletePolicy() raw.BucketSoftDeletePolicy {`
`2123`	`2123`	`return nil`
`2124`	`2124`	`}`
`2125`	`2125`	`// Excluding read only field EffectiveTime.`
	`2126`	`+ // ForceSendFields must be set to send a zero value for RetentionDuration and disable`
	`2127`	`+ // soft delete.`
`2126`	`2128`	`return &raw.BucketSoftDeletePolicy{`
`2127`	`2129`	`RetentionDurationSeconds: int64(p.RetentionDuration.Seconds()),`
	`2130`	`+ ForceSendFields: []string{"RetentionDurationSeconds"},`
`2128`	`2131`	`}`
`2129`	`2132`	`}`
`2130`	`2133`