From d9cb47c3d42e4fc8e760907a71ba9d77eb0efe63 Mon Sep 17 00:00:00 2001 From: Brett Lawson Date: Mon, 13 Jan 2020 01:15:10 -0800 Subject: [PATCH] GOCBC-727: Modified all managers to not return errors. Motivation ---------- Managers are created as pointer objects as opposed to actually performing work when they are created. They should reflect this in terms of not returning errors. Changes ------- Modified the behaviour of manager creation such that they only rely on having access to the cluster at operation execution time, as opposed to at manager creation time. Removed the error return value from the manager creation methods as well. Change-Id: I0c87c81af68a3eed0245f03b6bbcf2f2c4394cdf Reviewed-on: http://review.couchbase.org/120377 Reviewed-by: Michael Nitschinger Tested-by: Brett Lawson --- bucket.go | 26 ++++++++++++++++----- bucket_collectionsmgr_test.go | 7 ++---- cluster.go | 40 +++++++++++++++++++------------- cluster_analyticsindexes_test.go | 7 ++---- cluster_bucketmgr_test.go | 7 ++---- cluster_searchindexes_test.go | 35 ++++++++-------------------- cluster_usermgr_test.go | 26 ++++++--------------- collection_crud_test.go | 5 +--- 8 files changed, 68 insertions(+), 85 deletions(-) diff --git a/bucket.go b/bucket.go index 2184ed3b..f987c965 100644 --- a/bucket.go +++ b/bucket.go @@ -1,5 +1,9 @@ package gocb +import ( + gocbcore "github.com/couchbase/gocbcore/v8" +) + // Bucket represents a single bucket within a cluster. type Bucket struct { sb stateBlock @@ -79,25 +83,35 @@ func (b *Bucket) stateBlock() stateBlock { } // ViewIndexes returns a ViewIndexManager instance for managing views. -func (b *Bucket) ViewIndexes() (*ViewIndexManager, error) { +func (b *Bucket) ViewIndexes() *ViewIndexManager { return &ViewIndexManager{ bucket: b, tracer: b.sb.Tracer, - }, nil + } +} + +type bucketHTTPWrapper struct { + b *Bucket } -// CollectionManager provides functions for managing collections. -func (b *Bucket) CollectionManager() (*CollectionManager, error) { - provider, err := b.sb.getCachedClient().getHTTPProvider() +func (bw bucketHTTPWrapper) DoHTTPRequest(req *gocbcore.HTTPRequest) (*gocbcore.HTTPResponse, error) { + provider, err := bw.b.sb.getCachedClient().getHTTPProvider() if err != nil { return nil, err } + return provider.DoHTTPRequest(req) +} + +// Collections provides functions for managing collections. +func (b *Bucket) Collections() *CollectionManager { + provider := bucketHTTPWrapper{b} + return &CollectionManager{ httpClient: provider, bucketName: b.Name(), globalTimeout: b.sb.ManagementTimeout, defaultRetryStrategy: b.sb.RetryStrategyWrapper, tracer: b.sb.Tracer, - }, nil + } } diff --git a/bucket_collectionsmgr_test.go b/bucket_collectionsmgr_test.go index 1853f787..08138656 100644 --- a/bucket_collectionsmgr_test.go +++ b/bucket_collectionsmgr_test.go @@ -10,12 +10,9 @@ func TestCollectionManagerCrud(t *testing.T) { t.Skip("Skipping test as collections not supported") } - mgr, err := globalBucket.CollectionManager() - if err != nil { - t.Fatalf("Failed to get collections manager %v", err) - } + mgr := globalBucket.Collections() - err = mgr.CreateScope("testScope", nil) + err := mgr.CreateScope("testScope", nil) if err != nil { t.Fatalf("Failed to create scope %v", err) } diff --git a/cluster.go b/cluster.go index 82543f7f..b23f3541 100644 --- a/cluster.go +++ b/cluster.go @@ -7,6 +7,7 @@ import ( "sync/atomic" "time" + gocbcore "github.com/couchbase/gocbcore/v8" "github.com/couchbaselabs/gocbconnstr" "github.com/pkg/errors" ) @@ -478,56 +479,63 @@ func (c *Cluster) setSupportsEnhancedPreparedStatements(supports bool) { } } -// Users returns a UserManager for managing users. -func (c *Cluster) Users() (*UserManager, error) { - provider, err := c.getHTTPProvider() +type clusterHTTPWrapper struct { + c *Cluster +} + +func (cw clusterHTTPWrapper) DoHTTPRequest(req *gocbcore.HTTPRequest) (*gocbcore.HTTPResponse, error) { + provider, err := cw.c.getHTTPProvider() if err != nil { return nil, err } + return provider.DoHTTPRequest(req) +} + +// Users returns a UserManager for managing users. +func (c *Cluster) Users() *UserManager { + provider := clusterHTTPWrapper{c} + return &UserManager{ httpClient: provider, globalTimeout: c.sb.ManagementTimeout, defaultRetryStrategy: c.sb.RetryStrategyWrapper, tracer: c.sb.Tracer, - }, nil + } } // Buckets returns a BucketManager for managing buckets. -func (c *Cluster) Buckets() (*BucketManager, error) { - provider, err := c.getHTTPProvider() - if err != nil { - return nil, err - } +func (c *Cluster) Buckets() *BucketManager { + provider := clusterHTTPWrapper{c} return &BucketManager{ httpClient: provider, globalTimeout: c.sb.ManagementTimeout, defaultRetryStrategy: c.sb.RetryStrategyWrapper, tracer: c.sb.Tracer, - }, nil + } } // AnalyticsIndexes returns an AnalyticsIndexManager for managing analytics indexes. -func (c *Cluster) AnalyticsIndexes() (*AnalyticsIndexManager, error) { +func (c *Cluster) AnalyticsIndexes() *AnalyticsIndexManager { return &AnalyticsIndexManager{ cluster: c, tracer: c.sb.Tracer, - }, nil + } } // QueryIndexes returns a QueryIndexManager for managing N1QL indexes. -func (c *Cluster) QueryIndexes() (*QueryIndexManager, error) { +func (c *Cluster) QueryIndexes() *QueryIndexManager { return &QueryIndexManager{ cluster: c, tracer: c.sb.Tracer, - }, nil + } } // SearchIndexes returns a SearchIndexManager for managing Search indexes. -func (c *Cluster) SearchIndexes() (*SearchIndexManager, error) { +func (c *Cluster) SearchIndexes() *SearchIndexManager { return &SearchIndexManager{ cluster: c, tracer: c.sb.Tracer, - }, nil + } } diff --git a/cluster_analyticsindexes_test.go b/cluster_analyticsindexes_test.go index 797b3b92..88a04d06 100644 --- a/cluster_analyticsindexes_test.go +++ b/cluster_analyticsindexes_test.go @@ -10,12 +10,9 @@ func TestAnalyticsIndexesCrud(t *testing.T) { t.Skip("Skipping test, analytics indexes not supported.") } - mgr, err := globalCluster.AnalyticsIndexes() - if err != nil { - t.Fatalf("Expected AnalyticsIndexes to not error %v", err) - } + mgr := globalCluster.AnalyticsIndexes() - err = mgr.CreateDataverse("testaverse", nil) + err := mgr.CreateDataverse("testaverse", nil) if err != nil { t.Fatalf("Expected CreateDataverse to not error %v", err) } diff --git a/cluster_bucketmgr_test.go b/cluster_bucketmgr_test.go index 5fdb3c2d..789eebf8 100644 --- a/cluster_bucketmgr_test.go +++ b/cluster_bucketmgr_test.go @@ -16,12 +16,9 @@ func TestBucketMgrOps(t *testing.T) { t.Skip("Skipping test as bucket manager not supported.") } - mgr, err := globalCluster.Buckets() - if err != nil { - t.Fatalf("Failed to create bucket manager %v", err) - } + mgr := globalCluster.Buckets() - err = mgr.CreateBucket(CreateBucketSettings{ + err := mgr.CreateBucket(CreateBucketSettings{ BucketSettings: BucketSettings{ Name: "test22", RAMQuotaMB: 100, diff --git a/cluster_searchindexes_test.go b/cluster_searchindexes_test.go index 3da150a5..a5c85d18 100644 --- a/cluster_searchindexes_test.go +++ b/cluster_searchindexes_test.go @@ -10,12 +10,9 @@ func TestSearchIndexesCrud(t *testing.T) { t.Skip("Skipping test as search indexes not supported") } - mgr, err := globalCluster.SearchIndexes() - if err != nil { - t.Fatalf("Expected err to be nil but was %v", err) - } + mgr := globalCluster.SearchIndexes() - err = mgr.UpsertIndex(SearchIndex{ + err := mgr.UpsertIndex(SearchIndex{ Name: "test", Type: "fulltext-index", SourceType: "couchbase", @@ -144,12 +141,9 @@ func TestSearchIndexesUpsertIndexNoName(t *testing.T) { t.Skip("Skipping test as search indexes not supported") } - mgr, err := globalCluster.SearchIndexes() - if err != nil { - t.Fatalf("Expected err to be nil but was %v", err) - } + mgr := globalCluster.SearchIndexes() - err = mgr.UpsertIndex(SearchIndex{}, nil) + err := mgr.UpsertIndex(SearchIndex{}, nil) if err == nil { t.Fatalf("Expected UpsertIndex err to be not nil but was") } @@ -164,12 +158,9 @@ func TestSearchIndexesIngestControl(t *testing.T) { t.Skip("Skipping test as search indexes not supported") } - mgr, err := globalCluster.SearchIndexes() - if err != nil { - t.Fatalf("Expected err to be nil but was %v", err) - } + mgr := globalCluster.SearchIndexes() - err = mgr.UpsertIndex(SearchIndex{ + err := mgr.UpsertIndex(SearchIndex{ Name: "test", Type: "fulltext-index", SourceType: "couchbase", @@ -197,12 +188,9 @@ func TestSearchIndexesQueryControl(t *testing.T) { t.Skip("Skipping test as search indexes not supported") } - mgr, err := globalCluster.SearchIndexes() - if err != nil { - t.Fatalf("Expected err to be nil but was %v", err) - } + mgr := globalCluster.SearchIndexes() - err = mgr.UpsertIndex(SearchIndex{ + err := mgr.UpsertIndex(SearchIndex{ Name: "test", Type: "fulltext-index", SourceType: "couchbase", @@ -230,12 +218,9 @@ func TestSearchIndexesPartitionControl(t *testing.T) { t.Skip("Skipping test as search indexes not supported") } - mgr, err := globalCluster.SearchIndexes() - if err != nil { - t.Fatalf("Expected err to be nil but was %v", err) - } + mgr := globalCluster.SearchIndexes() - err = mgr.UpsertIndex(SearchIndex{ + err := mgr.UpsertIndex(SearchIndex{ Name: "test", Type: "fulltext-index", SourceType: "couchbase", diff --git a/cluster_usermgr_test.go b/cluster_usermgr_test.go index 6ae6248e..136e6e3f 100644 --- a/cluster_usermgr_test.go +++ b/cluster_usermgr_test.go @@ -10,12 +10,9 @@ func TestUserManagerGroupCrud(t *testing.T) { t.Skip("Skipping test as groups not supported.") } - mgr, err := globalCluster.Users() - if err != nil { - t.Fatalf("Expected Groups to not error: %v", err) - } + mgr := globalCluster.Users() - err = mgr.UpsertGroup(Group{ + err := mgr.UpsertGroup(Group{ Name: "test", Description: "this is a test", Roles: []Role{ @@ -76,12 +73,9 @@ func TestUserManagerWithGroupsCrud(t *testing.T) { t.Skip("Skipping test as groups not supported.") } - mgr, err := globalCluster.Users() - if err != nil { - t.Fatalf("Expected Users to not error: %v", err) - } + mgr := globalCluster.Users() - err = mgr.UpsertGroup(Group{ + err := mgr.UpsertGroup(Group{ Name: "test", Description: "this is a test", Roles: []Role{ @@ -206,10 +200,7 @@ func TestUserManagerCrud(t *testing.T) { t.Skip("Skipping test as rbac not supported.") } - mgr, err := globalCluster.Users() - if err != nil { - t.Fatalf("Expected Users to not error: %v", err) - } + mgr := globalCluster.Users() expectedUser := User{ Username: "barry", @@ -222,7 +213,7 @@ func TestUserManagerCrud(t *testing.T) { }, }, } - err = mgr.UpsertUser(expectedUser, nil) + err := mgr.UpsertUser(expectedUser, nil) if err != nil { t.Fatalf("Expected UpsertUser to not error: %v", err) } @@ -288,10 +279,7 @@ func TestUserManagerAvailableRoles(t *testing.T) { t.Skip("Skipping test as rbac not supported.") } - mgr, err := globalCluster.Users() - if err != nil { - t.Fatalf("Expected Users to not error: %v", err) - } + mgr := globalCluster.Users() roles, err := mgr.GetRoles(nil) if err != nil { diff --git a/collection_crud_test.go b/collection_crud_test.go index 70ff47fe..d3e881b1 100644 --- a/collection_crud_test.go +++ b/collection_crud_test.go @@ -584,10 +584,7 @@ func TestCollectionRetry(t *testing.T) { collectionName := "insertRetry" // cli := globalBucket.sb.getCachedClient() - mgr, err := globalBucket.CollectionManager() - if err != nil { - t.Fatalf("Could not get CollectionManager: %v", err) - } + mgr := globalBucket.Collections() err = mgr.CreateCollection(CollectionSpec{ScopeName: "_default", Name: collectionName}, nil) if err != nil {