review comment changes

GoogleCloudPlatform · Jun 11, 2024 · fa11940 · fa11940
1 parent c58d61b
commit fa11940
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 46 deletions.
diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go
@@ -841,10 +841,24 @@ func (d *dirInode) DeleteChildDir(
 	}
 	childName := NewDirName(d.Name(), name)
 
-	// Delete Folder deletes backing object as well as folder (in case of Hierarchical Bucket).
-	// Unfortunately we have no way to precondition
+	// Delete the backing object. Unfortunately we have no way to precondition
 	// this on the directory being empty.
-	err = d.bucket.DeleteFolder(ctx, childName.GcsObjectName())
+	err = d.bucket.DeleteObject(
+		ctx,
+		&gcs.DeleteObjectRequest{
+			Name:       childName.GcsObjectName(),
+			Generation: 0, // Delete the latest version of object named after dir.
+		})
+
+	if err != nil {
+		err = fmt.Errorf("DeleteObject: %w", err)
+		return
+	}
+
+	if d.bucket.BucketType() == gcs.Hierarchical {
+		// Delete Folder deletes folder (in case of Hierarchical Bucket).
+		err = d.bucket.DeleteFolder(ctx, childName.GcsObjectName())
+	}
 
 	if err != nil {
 		return

diff --git a/internal/storage/bucket_handle.go b/internal/storage/bucket_handle.go
@@ -83,8 +83,8 @@ func (bh *bucketHandle) BucketType() gcs.BucketType {
 }
 
 func (bh *bucketHandle) NewReader(
-	ctx context.Context,
-	req *gcs.ReadObjectRequest) (io.ReadCloser, error) {
+		ctx context.Context,
+		req *gcs.ReadObjectRequest) (io.ReadCloser, error) {
 	// Initialising the starting offset and the length to be read by the reader.
 	start := int64(0)
 	length := int64(-1)
@@ -146,7 +146,7 @@ func (b *bucketHandle) DeleteObject(ctx context.Context, req *gcs.DeleteObjectRe
 }
 
 func (b *bucketHandle) StatObject(ctx context.Context,
-	req *gcs.StatObjectRequest) (m *gcs.MinObject, e *gcs.ExtendedObjectAttributes, err error) {
+		req *gcs.StatObjectRequest) (m *gcs.MinObject, e *gcs.ExtendedObjectAttributes, err error) {
 	var attrs *storage.ObjectAttrs
 	// Retrieving object attrs through Go Storage Client.
 	attrs, err = b.bucket.Object(req.Name).Attrs(ctx)
@@ -468,26 +468,6 @@ func (b *bucketHandle) ComposeObjects(ctx context.Context, req *gcs.ComposeObjec
 }
 
 func (b *bucketHandle) DeleteFolder(ctx context.Context, folderName string) (err error) {
-	if b.BucketType() == gcs.NonHierarchical {
-		err = b.DeleteObject(
-			ctx,
-			&gcs.DeleteObjectRequest{
-				Name:       folderName,
-				Generation: 0, // Delete the latest version of object named after dir.
-			})
-
-		if err != nil {
-			return fmt.Errorf("DeleteObject: %w", err)
-		}
-		return nil
-	}
-
-	err = b.deleteFolderForHierarchicalBucket(ctx, folderName)
-
-	return err
-}
-
-func (b *bucketHandle) deleteFolderForHierarchicalBucket(ctx context.Context, folderName string) (err error) {
 	var notfound *gcs.NotFoundError
 	var callOptions []gax.CallOption
 

diff --git a/internal/storage/bucket_handle_test.go b/internal/storage/bucket_handle_test.go
@@ -1285,23 +1285,3 @@ func (testSuite *BucketHandleTest) TestDeleteFolderWhenFolderNotExistForHierarch
 
 	assert.Equal(testSuite.T(), "DeleteFolder: mock error", err.Error())
 }
-
-func (testSuite *BucketHandleTest) TestDeleteFolderWhenObjectExitForNonHierarchicalBucket() {
-	mockClient := new(MockStorageControlClient)
-	testSuite.bucketHandle.controlClient = mockClient
-	testSuite.bucketHandle.bucketType = gcs.NonHierarchical
-
-	err := testSuite.bucketHandle.DeleteFolder(context.Background(), TestObjectName)
-
-	assert.Nil(testSuite.T(), err)
-}
-
-func (testSuite *BucketHandleTest) TestDeleteFolderWhenObjectNotExitForNonHierarchicalBucket() {
-	mockClient := new(MockStorageControlClient)
-	testSuite.bucketHandle.controlClient = mockClient
-	testSuite.bucketHandle.bucketType = gcs.NonHierarchical
-
-	err := testSuite.bucketHandle.DeleteFolder(context.Background(), missingObjectName)
-
-	assert.Equal(testSuite.T(), "DeleteObject: gcs.NotFoundError: storage: object doesn't exist", err.Error())
-}