samples(storage): add samples and test cases for soft delete objects #13
Conversation
mahendra-google
changed the title
|
/Gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds new samples and corresponding tests for Google Cloud Storage soft-delete features: listing soft-deleted objects, listing soft-deleted versions of an object, and restoring a soft-deleted object. The new samples and helper methods in StorageFixture are well-implemented. However, I've found critical issues in the test setup for all three new test classes. The tests create buckets without enabling the soft-delete feature (and in one case, without versioning), which is necessary for the features under test to work as expected. I've provided suggestions to correct the bucket creation in each test file.
| ListSoftDeletedObjectsSample listSoftDeletedObjects = new ListSoftDeletedObjectsSample(); | ||
| UploadObjectFromMemorySample uploadObjectFromMemory = new UploadObjectFromMemorySample(); | ||
| var bucketName = _fixture.GenerateBucketName(); | ||
| _fixture.CreateBucket(bucketName, multiVersion: false, softDelete: false, registerForDeletion: true); |
There was a problem hiding this comment.
The test is for listing soft-deleted objects, but the bucket is created with soft-delete disabled (softDelete: false). To ensure the test correctly validates the soft-delete functionality, you should enable it when creating the bucket.
_fixture.CreateBucket(bucketName, multiVersion: false, softDelete: true, registerForDeletion: true);| RestoreSoftDeletedObjectSample restoreSoftDeletedObjectSample = new RestoreSoftDeletedObjectSample(); | ||
| GetMetadataSample getMetadataSample = new GetMetadataSample(); | ||
| var bucketName = _fixture.GenerateBucketName(); | ||
| _fixture.CreateBucket(bucketName, multiVersion: false, softDelete: false, registerForDeletion: true); |
There was a problem hiding this comment.
This test is for listing soft-deleted versions of an object. For this to work correctly, both versioning and soft-delete must be enabled on the bucket. Currently, both are disabled in the bucket creation step (multiVersion: false, softDelete: false).
_fixture.CreateBucket(bucketName, multiVersion: true, softDelete: true, registerForDeletion: true);| UploadObjectFromMemorySample uploadObjectFromMemory = new UploadObjectFromMemorySample(); | ||
| GetMetadataSample getMetadataSample = new GetMetadataSample(); | ||
| var bucketName = _fixture.GenerateBucketName(); | ||
| _fixture.CreateBucket(bucketName, multiVersion: false, softDelete: false, registerForDeletion: true); |
There was a problem hiding this comment.
To test restoring a soft-deleted object, the bucket must have soft-delete enabled. The bucket is currently created with softDelete: false. Please enable it to ensure the test is valid.
_fixture.CreateBucket(bucketName, multiVersion: false, softDelete: true, registerForDeletion: true);There was a problem hiding this comment.
@gemini-code-assist I think soft delete object and bucket are a different
There was a problem hiding this comment.
You're right that soft-delete applies to objects, but the ability to soft-delete objects is a bucket-level configuration. If a bucket does not have soft-delete enabled, then deleting an object within that bucket will result in a permanent deletion, not a soft-delete.
For the RestoreSoftDeletedObjectTest to effectively test the restoration of a soft-deleted object, the bucket used in the test must first be configured to support soft-deletion. This ensures that when an object is 'deleted' in the test, it is indeed soft-deleted and thus available for restoration. The current softDelete: false setting would lead to permanent deletion, making the restoration step impossible to test correctly.
Therefore, the suggestion to set softDelete: true is crucial for the test's validity.
| _fixture.CreateBucket(bucketName, multiVersion: false, softDelete: false, registerForDeletion: true); | |
| _fixture.CreateBucket(bucketName, multiVersion: false, softDelete: true, registerForDeletion: true); |
Add samples and test cases for soft delete objects as follows:-