samples(storage): add samples and test cases for bucket soft delete policy

[mahendra-google]

Add samples and test cases for bucket soft delete policy as follows:-

• storage_disable_soft_delete
• storage_get_soft_delete_policy
• storage_set_soft_delete_policy

[mahendra-google]

@amrik-mahal This PR is ready for re-review

[amrik-mahal]

Looks good.

[mahendra-google]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds samples and test cases for the bucket soft delete policy. The changes are generally good, but I have identified a few areas for improvement. My main feedback focuses on enhancing the correctness and efficiency of the new samples by using PatchBucket instead of UpdateBucket to prevent potential data loss and unnecessary API calls. I've also pointed out a critical issue in BucketGetSoftDeletePolicy that could lead to a NullReferenceException. Additionally, I've suggested improvements to the test cases to make them more robust and maintainable, and pointed out some minor code duplication in the test fixture.

[gemini-code-assist]

The bucket.SoftDeletePolicy property can be null if no soft delete policy has ever been set on the bucket. Accessing RetentionDurationSeconds on a null SoftDeletePolicy will cause a NullReferenceException. You should add a null check for SoftDeletePolicy.

        if (bucket.SoftDeletePolicy is null || bucket.SoftDeletePolicy.RetentionDurationSeconds == 0)

[gemini-code-assist]

Comparing objects by serializing them to JSON can make tests brittle. For example, a change in property order in the JSON output could break the test, even if the objects are semantically equivalent. It's better to assert the properties of the objects directly.

        Assert.Equal(bucket.SoftDeletePolicy.RetentionDurationSeconds, softPolicyData.RetentionDurationSeconds);
        Assert.Equal(bucket.SoftDeletePolicy.EffectiveTime, softPolicyData.EffectiveTime);

[gemini-code-assist]

The current implementation uses GetBucket followed by UpdateBucket. UpdateBucket performs a PUT operation, which can overwrite other bucket properties if they were changed between the GET and PUT calls. It's also less efficient due to the extra API call.

A better approach is to use PatchBucket, which performs a PATCH operation. This updates only the specified properties and avoids the need for an initial GetBucket call, making the code more efficient and safer against race conditions.

        var storage = StorageClient.Create();
        var bucket = new Bucket { Name = bucketName, SoftDeletePolicy = new Bucket.SoftDeletePolicyData { RetentionDurationSeconds = 0L } };
        bucket = storage.PatchBucket(bucket);
        Console.WriteLine($"The Soft Delete Policy for the Bucket (Bucket Name: {bucketName}) is disabled");
        return bucket;

[gemini-code-assist]

This method uses GetBucket followed by UpdateBucket. UpdateBucket performs a PUT operation, which can overwrite other bucket properties if they were changed between the GET and PUT calls. It's also less efficient due to the extra API call. A better approach is to use PatchBucket, which performs a PATCH operation to update only the specified properties.

Additionally, for invalid input, returning the unmodified bucket object is inefficient as it requires a Get call. It's more idiomatic and clearer to throw an ArgumentOutOfRangeException to signal a programming error to the caller.

        if (retentionDurationInDays < 7 || retentionDurationInDays > 90)
        {
            // The maximum retention duration you can set is 90 days and the minimum retention duration you can set is 7 days.
            // For more information, please refer to https://cloud.google.com/storage/docs/soft-delete#retention-duration
            Console.WriteLine($"The Soft Delete Policy for the Bucket (Bucket Name: {bucketName}) must have a retention duration between 7 days and 90 days");
            throw new ArgumentOutOfRangeException(nameof(retentionDurationInDays), "Retention duration must be between 7 and 90 days.");
        }
        var storage = StorageClient.Create();
        long retentionDurationInSeconds = (long) TimeSpan.FromDays(retentionDurationInDays).TotalSeconds;
        var bucket = new Bucket { Name = bucketName, SoftDeletePolicy = new Bucket.SoftDeletePolicyData { RetentionDurationSeconds = retentionDurationInSeconds } };
        bucket = storage.PatchBucket(bucket);
        Console.WriteLine($"The Soft Delete Policy for the Bucket (Bucket Name: {bucketName}) is set to the {retentionDurationInDays} days retention duration");
        return bucket;

[gemini-code-assist]

The zero variable and its preceding comment are unnecessary. You can use the literal 0L directly in the assertions for better readability and conciseness. Additionally, it's a common convention in xUnit to have the expected value as the first argument in Assert.Equal and Assert.NotEqual.

        Assert.NotEqual(0L, bucketWithDefaultSoftDeletePolicy.SoftDeletePolicy.RetentionDurationSeconds);
        // Disable soft-delete policy for the bucket.
        var bucketPostDisableSoftDeletePolicy = disableSample.BucketDisableSoftDeletePolicy(bucketName);
        Assert.Equal(0L, bucketPostDisableSoftDeletePolicy.SoftDeletePolicy.RetentionDurationSeconds);

[gemini-code-assist]

The new method GenerateGuid is not used in the codebase. It also duplicates the functionality of existing methods like GenerateBucketName, GenerateName, and GenerateContent. To avoid dead code and duplication, this method should be removed. Alternatively, if the goal is to consolidate on a better name, the other methods should be updated to call this one and then be deprecated or removed.