Create folder fs level integration (#2238)

* create folder fs level changes * add unit tests * small fix * unit tests changes * unit tests * rebase * fix nits * local changes * rebase * rebase * rebase * rebase * small change * review comment * set recusrsive flag true * prefix bucket fix * prefix bucket fix
GoogleCloudPlatform · Jul 31, 2024 · 0440af8 · 0440af8
1 parent a6bed5a
commit 0440af8
Show file tree

Hide file tree

Showing 7 changed files with 112 additions and 13 deletions.
diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go
@@ -835,13 +835,30 @@ func (d *dirInode) CreateChildSymlink(ctx context.Context, name string, target s
 
 // LOCKS_REQUIRED(d)
 func (d *dirInode) CreateChildDir(ctx context.Context, name string) (*Core, error) {
+	// Generate the full name for the new directory.
 	fullName := NewDirName(d.Name(), name)
-	o, err := d.createNewObject(ctx, fullName, nil)
-	if err != nil {
-		return nil, err
+	var m *gcs.MinObject
+
+	// Check the bucket type.
+	if d.isBucketHierarchical() {
+		// For hierarchical buckets, create a folder.
+		f, err := d.bucket.CreateFolder(ctx, fullName.objectName)
+		if err != nil {
+			return nil, err
+		}
+		// Convert the folder to a minimal object.
+		m = f.ConvertFolderToMinObject()
+	} else {
+		// For non-hierarchical buckets, create a new object.
+		o, err := d.createNewObject(ctx, fullName, nil)
+		if err != nil {
+			return nil, err
+		}
+		// Convert the object to a minimal object.
+		m = storageutil.ConvertObjToMinObject(o)
 	}
-	m := storageutil.ConvertObjToMinObject(o)
 
+	// Insert the new directory into the type cache.
 	d.cache.Insert(d.cacheClock.Now(), name, metadata.ExplicitDirType)
 
 	return &Core{

diff --git a/internal/fs/inode/hns_dir_test.go b/internal/fs/inode/hns_dir_test.go
@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
@@ -406,3 +407,69 @@ func (t *HNSDirTest) TestDeleteChildDir_WithImplicitDirFlagFalseAndBucketTypeIsH
 	// It will ignore the error that came from deleteObject.
 	assert.Equal(t.T(), err.Error(), "DeleteFolder: mock delete folder error")
 }
+
+func (t *HNSDirTest) TestCreateChildDirWhenBucketTypeIsHNSWithFailure() {
+	const name = "folder"
+	dirName := path.Join(dirInodeName, name) + "/"
+	t.mockBucket.On("BucketType").Return(gcs.Hierarchical)
+	t.mockBucket.On("CreateFolder", t.ctx, dirName).Return(nil, fmt.Errorf("mock error"))
+
+	result, err := t.in.CreateChildDir(t.ctx, name)
+
+	t.mockBucket.AssertExpectations(t.T())
+	assert.NotNil(t.T(), err)
+	assert.Nil(t.T(), result)
+	assert.Equal(t.T(), metadata.Type(0), t.typeCache.Get(t.fixedTime.Now(), dirName))
+}
+
+func (t *HNSDirTest) TestCreateChildDirWhenBucketTypeIsHNSWithSuccess() {
+	const name = "folder"
+	dirName := path.Join(dirInodeName, name) + "/"
+	folder := gcs.Folder{Name: dirName}
+	t.mockBucket.On("BucketType").Return(gcs.Hierarchical)
+	t.mockBucket.On("CreateFolder", t.ctx, dirName).Return(&folder, nil)
+
+	result, err := t.in.CreateChildDir(t.ctx, name)
+
+	t.mockBucket.AssertExpectations(t.T())
+	assert.NoError(t.T(), err)
+	assert.NotNil(t.T(), result)
+	assert.Equal(t.T(), dirName, result.MinObject.Name)
+	assert.Equal(t.T(), dirName, result.FullName.objectName)
+	assert.Equal(t.T(), metadata.ExplicitDirType, t.typeCache.Get(t.fixedTime.Now(), name))
+}
+
+func (t *HNSDirTest) TestCreateChildDirWhenBucketTypeIsNonHNSWithFailure() {
+	const name = "folder"
+	var preCond int64
+	dirName := path.Join(dirInodeName, name) + "/"
+	createObjectReq := gcs.CreateObjectRequest{Name: dirName, Contents: strings.NewReader(""), GenerationPrecondition: &preCond}
+	t.mockBucket.On("BucketType").Return(gcs.NonHierarchical)
+	t.mockBucket.On("CreateObject", t.ctx, &createObjectReq).Return(nil, fmt.Errorf("mock error"))
+
+	result, err := t.in.CreateChildDir(t.ctx, name)
+
+	t.mockBucket.AssertExpectations(t.T())
+	assert.NotNil(t.T(), err)
+	assert.Nil(t.T(), result)
+	assert.Equal(t.T(), metadata.Type(0), t.typeCache.Get(t.fixedTime.Now(), dirName))
+}
+
+func (t *HNSDirTest) TestCreateChildDirWhenBucketTypeIsNonHNSWithSuccess() {
+	const name = "folder"
+	dirName := path.Join(dirInodeName, name) + "/"
+	var preCond int64
+	createObjectReq := gcs.CreateObjectRequest{Name: dirName, Contents: strings.NewReader(""), GenerationPrecondition: &preCond}
+	object := gcs.Object{Name: dirName}
+	t.mockBucket.On("BucketType").Return(gcs.NonHierarchical)
+	t.mockBucket.On("CreateObject", t.ctx, &createObjectReq).Return(&object, nil)
+
+	result, err := t.in.CreateChildDir(t.ctx, name)
+
+	t.mockBucket.AssertExpectations(t.T())
+	assert.NoError(t.T(), err)
+	assert.NotNil(t.T(), result)
+	assert.Equal(t.T(), dirName, result.MinObject.Name)
+	assert.Equal(t.T(), dirName, result.FullName.objectName)
+	assert.Equal(t.T(), metadata.ExplicitDirType, t.typeCache.Get(t.fixedTime.Now(), name))
+}
diff --git a/internal/gcsx/prefix_bucket.go b/internal/gcsx/prefix_bucket.go
@@ -230,9 +230,16 @@ func (b *prefixBucket) GetFolder(ctx context.Context, folderName string) (folder
 	return f, err
 }
 
-func (b *prefixBucket) CreateFolder(ctx context.Context, folderName string) (folder *gcs.Folder, err error) {
+func (b *prefixBucket) CreateFolder(ctx context.Context, folderName string) (*gcs.Folder, error) {
 	mFolderName := b.wrappedName(folderName)
-	return b.wrapped.CreateFolder(ctx, mFolderName)
+	f, err := b.wrapped.CreateFolder(ctx, mFolderName)
+
+	// Modify the returned folder.
+	if f != nil {
+		f.Name = b.localName(mFolderName)
+	}
+
+	return f, err
 }
 
 func (b *prefixBucket) RenameFolder(ctx context.Context, folderName string, destinationFolderId string) (*gcs.Folder, error) {

diff --git a/internal/gcsx/prefix_bucket_test.go b/internal/gcsx/prefix_bucket_test.go
@@ -477,8 +477,9 @@ func TestCreateFolder(t *testing.T) {
 	require.NoError(t, err)
 	ctx := context.Background()
 
-	_, err = bucket.CreateFolder(ctx, suffix)
+	f, err := bucket.CreateFolder(ctx, suffix)
 
+	assert.Equal(t, f.Name, suffix)
 	assert.NoError(t, err)
 	// Folder should get created
 	_, err = bucket.GetFolder(ctx, suffix)

diff --git a/internal/storage/bucket_handle.go b/internal/storage/bucket_handle.go
@@ -537,8 +537,9 @@ func (b *bucketHandle) GetFolder(ctx context.Context, folderName string) (*gcs.F
 
 func (b *bucketHandle) CreateFolder(ctx context.Context, folderName string) (*gcs.Folder, error) {
 	req := &controlpb.CreateFolderRequest{
-		Parent:   fmt.Sprintf(FullBucketPathHNS, b.bucketName),
-		FolderId: folderName,
+		Parent:    fmt.Sprintf(FullBucketPathHNS, b.bucketName),
+		FolderId:  folderName,
+		Recursive: true,
 	}
 
 	clientFolder, err := b.controlClient.CreateFolder(ctx, req)

diff --git a/internal/storage/bucket_handle_test.go b/internal/storage/bucket_handle_test.go
@@ -1309,7 +1309,7 @@ func (testSuite *BucketHandleTest) TestRenameFolderWithError() {
 }
 
 func (testSuite *BucketHandleTest) TestCreateFolderWithError() {
-	createFolderReq := controlpb.CreateFolderRequest{Parent: fmt.Sprintf(FullBucketPathHNS, TestBucketName), FolderId: TestFolderName}
+	createFolderReq := controlpb.CreateFolderRequest{Parent: fmt.Sprintf(FullBucketPathHNS, TestBucketName), FolderId: TestFolderName, Recursive: true}
 	testSuite.mockClient.On("CreateFolder", context.Background(), &createFolderReq, mock.Anything).Return(nil, errors.New("mock error"))
 	testSuite.bucketHandle.bucketType = gcs.Hierarchical
 
@@ -1324,7 +1324,7 @@ func (testSuite *BucketHandleTest) TestCreateFolderWithGivenName() {
 	mockFolder := controlpb.Folder{
 		Name: fmt.Sprintf(FullFolderPathHNS, TestBucketName, TestFolderName),
 	}
-	createFolderReq := controlpb.CreateFolderRequest{Parent: fmt.Sprintf(FullBucketPathHNS, TestBucketName), FolderId: TestFolderName}
+	createFolderReq := controlpb.CreateFolderRequest{Parent: fmt.Sprintf(FullBucketPathHNS, TestBucketName), FolderId: TestFolderName, Recursive: true}
 	testSuite.mockClient.On("CreateFolder", context.Background(), &createFolderReq, mock.Anything).Return(&mockFolder, nil)
 	testSuite.bucketHandle.bucketType = gcs.Hierarchical
 

diff --git a/internal/storage/testify_mock_bucket.go b/internal/storage/testify_mock_bucket.go
@@ -44,7 +44,10 @@ func (m *TestifyMockBucket) NewReader(ctx context.Context, req *gcs.ReadObjectRe
 
 func (m *TestifyMockBucket) CreateObject(ctx context.Context, req *gcs.CreateObjectRequest) (*gcs.Object, error) {
 	args := m.Called(ctx, req)
-	return args.Get(0).(*gcs.Object), args.Error(1)
+	if args.Get(1) != nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*gcs.Object), nil
 }
 
 func (m *TestifyMockBucket) CopyObject(ctx context.Context, req *gcs.CopyObjectRequest) (*gcs.Object, error) {
@@ -103,5 +106,8 @@ func (m *TestifyMockBucket) RenameFolder(ctx context.Context, folderName string,
 
 func (m *TestifyMockBucket) CreateFolder(ctx context.Context, folderName string) (*gcs.Folder, error) {
 	args := m.Called(ctx, folderName)
-	return args.Get(0).(*gcs.Folder), args.Error(1)
+	if args.Get(0) != nil {
+		return args.Get(0).(*gcs.Folder), nil
+	}
+	return nil, args.Error(1)
 }