addibg method in bucketType only

GoogleCloudPlatform · May 22, 2024 · 4fa8b51 · 4fa8b51
1 parent efdc715
commit 4fa8b51
Show file tree

Hide file tree

Showing 12 changed files with 40 additions and 74 deletions.
diff --git a/internal/gcsx/bucket_manager.go b/internal/gcsx/bucket_manager.go
@@ -223,7 +223,7 @@ func (bm *bucketManager) SetUpBucket(
 		b)
 
 	// Fetch bucket type from storage layout api and set bucket type.
-	b.FetchAndSetBucketType()
+	b.BucketType()
 
 	// Check whether this bucket works, giving the user a warning early if there
 	// is some problem.

diff --git a/internal/gcsx/prefix_bucket.go b/internal/gcsx/prefix_bucket.go
@@ -67,10 +67,6 @@ func (b *prefixBucket) BucketType() gcs.BucketType {
 	return b.wrapped.BucketType()
 }
 
-func (b *prefixBucket) FetchAndSetBucketType() {
-	b.wrapped.FetchAndSetBucketType()
-}
-
 func (b *prefixBucket) NewReader(
 	ctx context.Context,
 	req *gcs.ReadObjectRequest) (rc io.ReadCloser, err error) {

diff --git a/internal/monitor/bucket.go b/internal/monitor/bucket.go
@@ -110,10 +110,6 @@ type monitoringBucket struct {
 	wrapped gcs.Bucket
 }
 
-func (mb *monitoringBucket) FetchAndSetBucketType() {
-	mb.wrapped.FetchAndSetBucketType()
-}
-
 func (mb *monitoringBucket) Name() string {
 	return mb.wrapped.Name()
 }

diff --git a/internal/ratelimit/throttled_bucket.go b/internal/ratelimit/throttled_bucket.go
@@ -54,10 +54,6 @@ func (b *throttledBucket) BucketType() gcs.BucketType {
 	return b.wrapped.BucketType()
 }
 
-func (b *throttledBucket) FetchAndSetBucketType() {
-	b.wrapped.FetchAndSetBucketType()
-}
-
 func (b *throttledBucket) NewReader(
 	ctx context.Context,
 	req *gcs.ReadObjectRequest) (rc io.ReadCloser, err error) {

diff --git a/internal/storage/bucket_handle.go b/internal/storage/bucket_handle.go
@@ -51,6 +51,31 @@ func (bh *bucketHandle) Name() string {
 }
 
 func (bh *bucketHandle) BucketType() gcs.BucketType {
+	var nilControlClient *control.StorageControlClient = nil
+	if bh.bucketType == gcs.Nil {
+		if bh.controlClient == nilControlClient {
+			bh.bucketType = gcs.NonHierarchical
+			return bh.bucketType
+		}
+
+		storageLayout, err := bh.getStorageLayout()
+
+		// In case bucket does not exist, set type unknown instead of panic.
+		if err != nil {
+			bh.bucketType = gcs.Unknown
+			logger.Errorf("Error returned from GetStorageLayout: %v", err)
+			return bh.bucketType
+		}
+
+		hierarchicalNamespace := storageLayout.GetHierarchicalNamespace()
+		if hierarchicalNamespace != nil && hierarchicalNamespace.Enabled {
+			bh.bucketType = gcs.Hierarchical
+			return bh.bucketType
+		}
+
+		bh.bucketType = gcs.NonHierarchical
+	}
+
 	return bh.bucketType
 }
 
@@ -436,35 +461,17 @@ func (b *bucketHandle) ComposeObjects(ctx context.Context, req *gcs.ComposeObjec
 	return
 }
 
-// Fetch bucket type from GetStorageLayout and set it. e.g. Hierarchical or NonHierarchical
-func (b *bucketHandle) FetchAndSetBucketType() {
-	var nilControlClient *control.StorageControlClient = nil
-	if b.controlClient == nilControlClient {
-		b.bucketType = gcs.NonHierarchical
-		return
-	}
-
+// TODO: Consider adding this method to the bucket interface if additional
+// layout options are needed in the future.
+func (b *bucketHandle) getStorageLayout() (*controlpb.StorageLayout, error) {
 	var callOptions []gax.CallOption
 	stoargeLayout, err := b.controlClient.GetStorageLayout(context.Background(), &controlpb.GetStorageLayoutRequest{
 		Name:      "projects/_/buckets/" + b.bucketName + "/storageLayout",
 		Prefix:    "",
 		RequestId: "",
 	}, callOptions...)
 
-	// In case bucket does not exist, set type unknown instead of panic.
-	if err != nil {
-		b.bucketType = gcs.Unknown
-		logger.Errorf("Error returned from GetStorageLayout: %v", err)
-		return
-	}
-
-	hierarchicalNamespace := stoargeLayout.GetHierarchicalNamespace()
-	if hierarchicalNamespace != nil && hierarchicalNamespace.Enabled {
-		b.bucketType = gcs.Hierarchical
-		return
-	}
-
-	b.bucketType = gcs.NonHierarchical
+	return stoargeLayout, err
 }
 
 func isStorageConditionsNotEmpty(conditions storage.Conditions) bool {

diff --git a/internal/storage/bucket_handle_test.go b/internal/storage/bucket_handle_test.go
@@ -980,7 +980,7 @@ func (testSuite *BucketHandleTest) TestNameMethod() {
 func (testSuite *BucketHandleTest) TestBucketTypeMethod() {
 	bucketType := testSuite.bucketHandle.BucketType()
 
-	assert.Equal(testSuite.T(), gcs.Unknown, bucketType)
+	assert.Equal(testSuite.T(), gcs.NonHierarchical, bucketType)
 }
 
 func (testSuite *BucketHandleTest) TestIsStorageConditionsNotEmptyWithEmptyConditions() {
@@ -1199,7 +1199,7 @@ func (testSuite *BucketHandleTest) TestFetchBucketTypeForHierarchicalNameSpaceTr
 		}, nil)
 	testSuite.bucketHandle.controlClient = mockClient
 
-	testSuite.bucketHandle.FetchAndSetBucketType()
+	testSuite.bucketHandle.BucketType()
 
 	assert.Equal(testSuite.T(), gcs.Hierarchical, testSuite.bucketHandle.bucketType, "Expected Hierarchical bucket type")
 }
@@ -1212,7 +1212,7 @@ func (testSuite *BucketHandleTest) TestFetchBucketTypeForHierarchicalNameSpaceFa
 		}, nil)
 	testSuite.bucketHandle.controlClient = mockClient
 
-	testSuite.bucketHandle.FetchAndSetBucketType()
+	testSuite.bucketHandle.BucketType()
 
 	assert.Equal(testSuite.T(), gcs.NonHierarchical, testSuite.bucketHandle.bucketType, "Expected NonHierarchical bucket type")
 }
@@ -1225,7 +1225,7 @@ func (testSuite *BucketHandleTest) TestFetchBucketTypeWithError() {
 		Return(x, errors.New("mocked error"))
 	testSuite.bucketHandle.controlClient = mockClient
 
-	testSuite.bucketHandle.FetchAndSetBucketType()
+	testSuite.bucketHandle.BucketType()
 
 	assert.Equal(testSuite.T(), gcs.Unknown, testSuite.bucketHandle.bucketType, "Expected Unknown when there's an error")
 }
@@ -1236,7 +1236,7 @@ func (testSuite *BucketHandleTest) TestFetchBucketTypeWithHierarchicalNamespaceI
 		Return(&controlpb.StorageLayout{}, nil)
 	testSuite.bucketHandle.controlClient = mockClient
 
-	testSuite.bucketHandle.FetchAndSetBucketType()
+	testSuite.bucketHandle.BucketType()
 
 	assert.Equal(testSuite.T(), gcs.NonHierarchical, testSuite.bucketHandle.bucketType, "Expected NonHierarchical bucket type")
 }
@@ -1245,7 +1245,7 @@ func (testSuite *BucketHandleTest) TestFetchDefaultBucketTypeWithControlClientNi
 	var nilControlClient *control.StorageControlClient = nil
 	testSuite.bucketHandle.controlClient = nilControlClient
 
-	testSuite.bucketHandle.FetchAndSetBucketType()
+	testSuite.bucketHandle.BucketType()
 
 	assert.Equal(testSuite.T(), gcs.NonHierarchical, testSuite.bucketHandle.bucketType, "Expected Hierarchical bucket type")
 }
diff --git a/internal/storage/caching/fast_stat_bucket.go b/internal/storage/caching/fast_stat_bucket.go
@@ -126,10 +126,6 @@ func (b *fastStatBucket) BucketType() gcs.BucketType {
 	return b.wrapped.BucketType()
 }
 
-func (b *fastStatBucket) FetchAndSetBucketType() {
-	b.wrapped.FetchAndSetBucketType()
-}
-
 func (b *fastStatBucket) NewReader(
 	ctx context.Context,
 	req *gcs.ReadObjectRequest) (rc io.ReadCloser, err error) {

diff --git a/internal/storage/debug_bucket.go b/internal/storage/debug_bucket.go
@@ -129,10 +129,6 @@ func (b *debugBucket) BucketType() gcs.BucketType {
 	return b.wrapped.BucketType()
 }
 
-func (b *debugBucket) FetchAndSetBucketType() {
-	b.wrapped.FetchAndSetBucketType()
-}
-
 func (b *debugBucket) NewReader(
 	ctx context.Context,
 	req *gcs.ReadObjectRequest) (rc io.ReadCloser, err error) {

diff --git a/internal/storage/gcs/bucket.go b/internal/storage/gcs/bucket.go
@@ -26,7 +26,8 @@ type BucketType int
 
 // BucketType enum values.
 const (
-	NonHierarchical BucketType = iota
+	Nil BucketType = iota
+	NonHierarchical
 	Hierarchical
 	Unknown
 )
@@ -45,9 +46,6 @@ type Bucket interface {
 	// Return Type of bucket e.g. Hierarchical or NonHierarchical
 	BucketType() BucketType
 
-	// Fetch bucket type from GetStorageLayout and set it. e.g. Hierarchical or NonHierarchical
-	FetchAndSetBucketType()
-
 	// Create a reader for the contents of a particular generation of an object.
 	// On a nil error, the caller must arrange for the reader to be closed when
 	// it is no longer needed.

diff --git a/internal/storage/mock_bucket.go b/internal/storage/mock_bucket.go
@@ -232,23 +232,6 @@ func (m *mockBucket) BucketType() (o0 gcs.BucketType) {
 	return
 }
 
-func (m *mockBucket) FetchAndSetBucketType() {
-	// Get a file name and line number for the caller.
-	_, file, line, _ := runtime.Caller(1)
-
-	// Hand the call off to the controller, which does most of the work.
-	retVals := m.controller.HandleMethodCall(
-		m,
-		"FetchAndSetBucketType",
-		file,
-		line,
-		[]interface{}{})
-
-	if len(retVals) != 1 {
-		panic(fmt.Sprintf("mockBucket.FetchAndSetBucketType: invalid return values: %v", retVals))
-	}
-}
-
 func (m *mockBucket) NewReader(p0 context.Context, p1 *gcs.ReadObjectRequest) (o0 io.ReadCloser, o1 error) {
 	// Get a file name and line number for the caller.
 	_, file, line, _ := runtime.Caller(1)

diff --git a/internal/storage/storage_handle.go b/internal/storage/storage_handle.go
@@ -24,7 +24,6 @@ import (
 	"github.com/googleapis/gax-go/v2"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/logger"
 	mountpkg "github.com/googlecloudplatform/gcsfuse/v2/internal/mount"
-	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/storageutil"
 	"golang.org/x/net/context"
 	option "google.golang.org/api/option"
@@ -208,7 +207,6 @@ func (sh *storageClient) BucketHandle(bucketName string, billingProject string)
 
 	bh = &bucketHandle{
 		bucket:        storageBucketHandle,
-		bucketType:    gcs.Unknown,
 		bucketName:    bucketName,
 		controlClient: sh.storageControlClient,
 	}

diff --git a/internal/storage/storage_handle_test.go b/internal/storage/storage_handle_test.go
@@ -60,7 +60,7 @@ func (testSuite *StorageHandleTest) TestBucketHandleWhenBucketExistsWithEmptyBil
 
 	assert.NotNil(testSuite.T(), bucketHandle)
 	assert.Equal(testSuite.T(), TestBucketName, bucketHandle.bucketName)
-	assert.Equal(testSuite.T(), gcs.Unknown, bucketHandle.bucketType)
+	assert.Equal(testSuite.T(), gcs.Nil, bucketHandle.bucketType)
 }
 
 func (testSuite *StorageHandleTest) TestBucketHandleWhenBucketDoesNotExistWithEmptyBillingProject() {
@@ -76,7 +76,7 @@ func (testSuite *StorageHandleTest) TestBucketHandleWhenBucketExistsWithNonEmpty
 
 	assert.NotNil(testSuite.T(), bucketHandle)
 	assert.Equal(testSuite.T(), TestBucketName, bucketHandle.bucketName)
-	assert.Equal(testSuite.T(), gcs.Unknown, bucketHandle.bucketType)
+	assert.Equal(testSuite.T(), gcs.Nil, bucketHandle.bucketType)
 }
 
 func (testSuite *StorageHandleTest) TestBucketHandleWhenBucketDoesNotExistWithNonEmptyBillingProject() {