Skip to content

Commit

Permalink
GOCBC-727: Modified all managers to not return errors.
Browse files Browse the repository at this point in the history
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 <michael.nitschinger@couchbase.com>
Tested-by: Brett Lawson <brett19@gmail.com>
  • Loading branch information
brett19 committed Jan 19, 2020
1 parent 3b32920 commit d9cb47c
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 85 deletions.
26 changes: 20 additions & 6 deletions bucket.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
}
}
7 changes: 2 additions & 5 deletions bucket_collectionsmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
40 changes: 24 additions & 16 deletions cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"sync/atomic"
"time"

gocbcore "github.com/couchbase/gocbcore/v8"
"github.com/couchbaselabs/gocbconnstr"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -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
}
}
7 changes: 2 additions & 5 deletions cluster_analyticsindexes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
7 changes: 2 additions & 5 deletions cluster_bucketmgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 10 additions & 25 deletions cluster_searchindexes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
}
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
26 changes: 7 additions & 19 deletions cluster_usermgr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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",
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions collection_crud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d9cb47c

Please sign in to comment.