diff --git a/authorization/error.go b/authorization/error.go index 8247169e03b..2aec2ac32ee 100644 --- a/authorization/error.go +++ b/authorization/error.go @@ -1,6 +1,7 @@ package authorization import ( + errors2 "errors" "fmt" "github.com/influxdata/influxdb/v2/kit/platform/errors" @@ -49,18 +50,18 @@ func ErrInvalidAuthIDError(err error) *errors.Error { } } -// ErrInternalServiceError is used when the error comes from an internal system. -func ErrInternalServiceError(err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Err: err, - } -} - // UnexpectedAuthIndexError is used when the error comes from an internal system. func UnexpectedAuthIndexError(err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Msg: fmt.Sprintf("unexpected error retrieving auth index; Err: %v", err), + var e *errors.Error + if !errors2.As(err, &e) { + e = &errors.Error{ + Msg: fmt.Sprintf("unexpected error retrieving auth index; Err: %v", err), + Code: errors.EInternal, + Err: err, + } + } + if e.Code == "" { + e.Code = errors.EInternal } + return e } diff --git a/authorization/storage_authorization.go b/authorization/storage_authorization.go index 3a25146655d..59637d10370 100644 --- a/authorization/storage_authorization.go +++ b/authorization/storage_authorization.go @@ -52,7 +52,10 @@ func decodeAuthorization(b []byte, a *influxdb.Authorization) error { // CreateAuthorization takes an Authorization object and saves it in storage using its token // using its token property as an index -func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.Authorization) error { +func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.Authorization) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpCreateAuthorization)) + }() // if the provided ID is invalid, or already maps to an existing Auth, then generate a new one if !a.ID.Valid() { id, err := s.generateSafeID(ctx, tx, authBucket) @@ -91,10 +94,7 @@ func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.A } if err := idx.Put(authIndexKey(a.Token), encodedID); err != nil { - return &errors.Error{ - Code: errors.EInternal, - Err: err, - } + return err } b, err := tx.Bucket(authBucket) @@ -103,16 +103,17 @@ func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.A } if err := b.Put(encodedID, v); err != nil { - return &errors.Error{ - Err: err, - } + return err } return nil } // GetAuthorization gets an authorization by its ID from the auth bucket in kv -func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform.ID) (*influxdb.Authorization, error) { +func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform.ID) (auth *influxdb.Authorization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindAuthorizationByID)) + }() encodedID, err := id.Encode() if err != nil { return nil, ErrInvalidAuthID @@ -120,7 +121,7 @@ func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform. b, err := tx.Bucket(authBucket) if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } v, err := b.Get(encodedID) @@ -129,21 +130,21 @@ func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform. } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } a := &influxdb.Authorization{} if err := decodeAuthorization(v, a); err != nil { - return nil, &errors.Error{ - Code: errors.EInvalid, - Err: err, - } + return nil, err } return a, nil } -func (s *Store) GetAuthorizationByToken(ctx context.Context, tx kv.Tx, token string) (*influxdb.Authorization, error) { +func (s *Store) GetAuthorizationByToken(ctx context.Context, tx kv.Tx, token string) (auth *influxdb.Authorization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindAuthorizationByToken)) + }() idx, err := authIndexBucket(tx) if err != nil { return nil, err @@ -171,7 +172,10 @@ func (s *Store) GetAuthorizationByToken(ctx context.Context, tx kv.Tx, token str // ListAuthorizations returns all the authorizations matching a set of FindOptions. This function is used for // FindAuthorizationByID, FindAuthorizationByToken, and FindAuthorizations in the AuthorizationService implementation -func (s *Store) ListAuthorizations(ctx context.Context, tx kv.Tx, f influxdb.AuthorizationFilter) ([]*influxdb.Authorization, error) { +func (s *Store) ListAuthorizations(ctx context.Context, tx kv.Tx, f influxdb.AuthorizationFilter) (auths []*influxdb.Authorization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindAuthorizations)) + }() var as []*influxdb.Authorization pred := authorizationsPredicateFn(f) filterFn := filterAuthorizationsFn(f) @@ -223,21 +227,18 @@ func (s *Store) forEachAuthorization(ctx context.Context, tx kv.Tx, pred kv.Curs } // UpdateAuthorization updates the status and description only of an authorization -func (s *Store) UpdateAuthorization(ctx context.Context, tx kv.Tx, id platform.ID, a *influxdb.Authorization) (*influxdb.Authorization, error) { +func (s *Store) UpdateAuthorization(ctx context.Context, tx kv.Tx, id platform.ID, a *influxdb.Authorization) (auth *influxdb.Authorization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpUpdateAuthorization)) + }() v, err := encodeAuthorization(a) if err != nil { - return nil, &errors.Error{ - Code: errors.EInvalid, - Err: err, - } + return nil, errors.ErrInternalServiceError(err, errors.WithErrorCode(errors.EInvalid)) } encodedID, err := a.ID.Encode() if err != nil { - return nil, &errors.Error{ - Code: errors.ENotFound, - Err: err, - } + return nil, errors.ErrInternalServiceError(err, errors.WithErrorCode(errors.ENotFound)) } idx, err := authIndexBucket(tx) @@ -246,10 +247,7 @@ func (s *Store) UpdateAuthorization(ctx context.Context, tx kv.Tx, id platform.I } if err := idx.Put(authIndexKey(a.Token), encodedID); err != nil { - return nil, &errors.Error{ - Code: errors.EInternal, - Err: err, - } + return nil, err } b, err := tx.Bucket(authBucket) @@ -258,9 +256,7 @@ func (s *Store) UpdateAuthorization(ctx context.Context, tx kv.Tx, id platform.I } if err := b.Put(encodedID, v); err != nil { - return nil, &errors.Error{ - Err: err, - } + return nil, err } return a, nil @@ -268,7 +264,10 @@ func (s *Store) UpdateAuthorization(ctx context.Context, tx kv.Tx, id platform.I } // DeleteAuthorization removes an authorization from storage -func (s *Store) DeleteAuthorization(ctx context.Context, tx kv.Tx, id platform.ID) error { +func (s *Store) DeleteAuthorization(ctx context.Context, tx kv.Tx, id platform.ID) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpDeleteAuthorization)) + }() a, err := s.GetAuthorizationByID(ctx, tx, id) if err != nil { return err @@ -290,11 +289,11 @@ func (s *Store) DeleteAuthorization(ctx context.Context, tx kv.Tx, id platform.I } if err := idx.Delete([]byte(a.Token)); err != nil { - return ErrInternalServiceError(err) + return err } if err := b.Delete(encodedID); err != nil { - return ErrInternalServiceError(err) + return err } return nil @@ -342,7 +341,7 @@ func uniqueID(ctx context.Context, tx kv.Tx, id platform.ID) error { b, err := tx.Bucket(authBucket) if err != nil { - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } _, err = b.Get(encodedID) diff --git a/bolt/kv.go b/bolt/kv.go index 85f99f08282..0676536839e 100644 --- a/bolt/kv.go +++ b/bolt/kv.go @@ -11,6 +11,7 @@ import ( "sync" "time" + errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kit/tracing" "github.com/influxdata/influxdb/v2/kv" "github.com/influxdata/influxdb/v2/kv/migration" @@ -161,12 +162,13 @@ func (s *KVStore) View(ctx context.Context, fn func(tx kv.Tx) error) error { span, ctx := tracing.StartSpanFromContext(ctx) defer span.Finish() - return s.DB().View(func(tx *bolt.Tx) error { + err := s.DB().View(func(tx *bolt.Tx) error { return fn(&Tx{ tx: tx, ctx: ctx, }) }) + return errors2.BoltToInfluxError(err) } // Update opens up an update transaction against the store. @@ -174,12 +176,13 @@ func (s *KVStore) Update(ctx context.Context, fn func(tx kv.Tx) error) error { span, ctx := tracing.StartSpanFromContext(ctx) defer span.Finish() - return s.DB().Update(func(tx *bolt.Tx) error { + err := s.DB().Update(func(tx *bolt.Tx) error { return fn(&Tx{ tx: tx, ctx: ctx, }) }) + return errors2.BoltToInfluxError(err) } // CreateBucket creates a bucket in the underlying boltdb store if it @@ -187,7 +190,7 @@ func (s *KVStore) Update(ctx context.Context, fn func(tx kv.Tx) error) error { func (s *KVStore) CreateBucket(ctx context.Context, name []byte) error { return s.DB().Update(func(tx *bolt.Tx) error { _, err := tx.CreateBucketIfNotExists(name) - return err + return errors2.BoltToInfluxError(err) }) } @@ -196,7 +199,7 @@ func (s *KVStore) CreateBucket(ctx context.Context, name []byte) error { func (s *KVStore) DeleteBucket(ctx context.Context, name []byte) error { return s.DB().Update(func(tx *bolt.Tx) error { if err := tx.DeleteBucket(name); err != nil && !errors.Is(err, bolt.ErrBucketNotFound) { - return err + return errors2.BoltToInfluxError(err) } return nil @@ -210,7 +213,7 @@ func (s *KVStore) Backup(ctx context.Context, w io.Writer) error { return s.DB().View(func(tx *bolt.Tx) error { _, err := tx.WriteTo(w) - return err + return errors2.BoltToInfluxError(err) }) } @@ -348,7 +351,7 @@ func (b *Bucket) Put(key []byte, value []byte) error { if err == bolt.ErrTxNotWritable { return kv.ErrTxNotWritable } - return err + return errors2.BoltToInfluxError(err) } // Delete removes the provided key. @@ -357,7 +360,7 @@ func (b *Bucket) Delete(key []byte) error { if err == bolt.ErrTxNotWritable { return kv.ErrTxNotWritable } - return err + return errors2.BoltToInfluxError(err) } // ForwardCursor retrieves a cursor for iterating through the entries diff --git a/bucket.go b/bucket.go index 095a9034f8e..1eaedfc0deb 100644 --- a/bucket.go +++ b/bucket.go @@ -2,12 +2,10 @@ package influxdb import ( "context" - "fmt" "strings" "time" "github.com/influxdata/influxdb/v2/kit/platform" - "github.com/influxdata/influxdb/v2/kit/platform/errors" ) const ( @@ -161,12 +159,3 @@ func (f BucketFilter) String() string { } return "[" + strings.Join(parts, ", ") + "]" } - -func ErrInternalBucketServiceError(op string, err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Msg: fmt.Sprintf("unexpected error in buckets; Err: %v", err), - Op: op, - Err: err, - } -} diff --git a/checks/service_external_test.go b/checks/service_external_test.go index a6abc97845a..d70552300f1 100644 --- a/checks/service_external_test.go +++ b/checks/service_external_test.go @@ -1030,7 +1030,7 @@ func FindChecks( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, _, opPrefix, done := init(tt.fields, t) + s, _, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1049,7 +1049,7 @@ func FindChecks( } checks, _, err := s.FindChecks(ctx, filter, tt.args.findOptions) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, t) if diff := cmp.Diff(checks, tt.wants.checks, checkCmpOptions...); diff != "" { t.Errorf("checks are different -got/+want\ndiff %s", diff) @@ -1536,14 +1536,14 @@ data = from(bucket: "telegraf") |> range(start: -1m)`, for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, _, opPrefix, done := init(tt.fields, t) + s, _, _, done := init(tt.fields, t) defer done() ctx := context.Background() checkCreate := influxdb.CheckCreate{Check: tt.args.check, Status: influxdb.Active} check, err := s.UpdateCheck(ctx, tt.args.id, checkCreate) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, t) if diff := cmp.Diff(check, tt.wants.check, checkCmpOptions...); diff != "" { t.Errorf("check is different -got/+want\ndiff %s", diff) @@ -1731,7 +1731,7 @@ func MustIDBase16(s string) platform.ID { return *id } -func diffPlatformErrors(name string, actual, expected error, opPrefix string, t *testing.T) { +func diffPlatformErrors(name string, actual, expected error, t *testing.T) { t.Helper() ErrorsEqual(t, actual, expected) } diff --git a/dashboards/testing/dashboards.go b/dashboards/testing/dashboards.go index 5a4c5aae473..77c1c639351 100644 --- a/dashboards/testing/dashboards.go +++ b/dashboards/testing/dashboards.go @@ -217,11 +217,11 @@ func CreateDashboard( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateDashboard(ctx, tt.args.dashboard) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) defer s.DeleteDashboard(ctx, tt.args.dashboard.ID) @@ -397,11 +397,11 @@ func AddDashboardCell( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.AddDashboardCell(ctx, tt.args.dashboardID, tt.args.cell, platform.AddDashboardCellOptions{}) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) defer s.DeleteDashboard(ctx, tt.args.dashboardID) @@ -493,12 +493,12 @@ func FindDashboardByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() dashboard, err := s.FindDashboardByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) if diff := cmp.Diff(dashboard, tt.wants.dashboard, dashboardCmpOptions...); diff != "" { t.Errorf("dashboard is different -got/+want\ndiff %s", diff) @@ -999,7 +999,7 @@ func FindDashboards( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx, err := feature.Annotate(context.Background(), mock.NewFlagger(map[feature.Flag]interface{}{ feature.EnforceOrganizationDashboardLimits(): true, @@ -1015,7 +1015,7 @@ func FindDashboards( } dashboards, _, err := s.FindDashboards(ctx, filter, tt.args.findOptions) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) if diff := cmp.Diff(dashboards, tt.wants.dashboards, dashboardCmpOptions...); diff != "" { t.Errorf("dashboards are different -got/+want\ndiff %s", diff) @@ -1115,11 +1115,11 @@ func DeleteDashboard( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteDashboard(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) filter := platform.DashboardFilter{} dashboards, _, err := s.FindDashboards(ctx, filter, platform.DefaultDashboardFindOptions) @@ -1351,7 +1351,7 @@ func UpdateDashboard( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1367,7 +1367,7 @@ func UpdateDashboard( } dashboard, err := s.UpdateDashboard(ctx, tt.args.id, upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) if diff := cmp.Diff(dashboard, tt.wants.dashboard, dashboardCmpOptions...); diff != "" { t.Errorf("dashboard is different -got/+want\ndiff %s", diff) @@ -1454,11 +1454,11 @@ func RemoveDashboardCell( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.RemoveDashboardCell(ctx, tt.args.dashboardID, tt.args.cellID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) defer s.DeleteDashboard(ctx, tt.args.dashboardID) @@ -1662,11 +1662,11 @@ func UpdateDashboardCell( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() _, err := s.UpdateDashboardCell(ctx, tt.args.dashboardID, tt.args.cellID, tt.args.cellUpdate) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) defer s.DeleteDashboard(ctx, tt.args.dashboardID) @@ -1852,11 +1852,11 @@ func ReplaceDashboardCells( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.ReplaceDashboardCells(ctx, tt.args.dashboardID, tt.args.cells) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) defer s.DeleteDashboard(ctx, tt.args.dashboardID) @@ -1952,12 +1952,12 @@ func GetDashboardCellView( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() view, err := s.GetDashboardCellView(ctx, tt.args.dashboardID, tt.args.cellID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) if diff := cmp.Diff(view, tt.wants.view); diff != "" { t.Errorf("dashboard cell views are different -got/+want\ndiff %s", diff) @@ -2125,7 +2125,7 @@ func UpdateDashboardCellView( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -2139,7 +2139,7 @@ func UpdateDashboardCellView( } view, err := s.UpdateDashboardCellView(ctx, tt.args.dashboardID, tt.args.cellID, upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(err, tt.wants.err, t) if diff := cmp.Diff(view, tt.wants.view); diff != "" { t.Errorf("dashboard cell views are different -got/+want\ndiff %s", diff) diff --git a/dashboards/testing/util.go b/dashboards/testing/util.go index a084a6b7053..8484fa007df 100644 --- a/dashboards/testing/util.go +++ b/dashboards/testing/util.go @@ -9,8 +9,7 @@ import ( "github.com/influxdata/influxdb/v2/kit/platform/errors" ) -// TODO(goller): remove opPrefix argument -func diffPlatformErrors(name string, actual, expected error, opPrefix string, t *testing.T) { +func diffPlatformErrors(actual, expected error, t *testing.T) { t.Helper() ErrorsEqual(t, actual, expected) } diff --git a/kit/errors/errors.go b/kit/errors/errors.go index 8c12720cd18..2bc48d4d03b 100644 --- a/kit/errors/errors.go +++ b/kit/errors/errors.go @@ -1,4 +1,4 @@ -package errors +package errors2 import ( "fmt" diff --git a/kit/errors/list.go b/kit/errors/list.go index c4636135c7b..d8271b7a6a0 100644 --- a/kit/errors/list.go +++ b/kit/errors/list.go @@ -1,4 +1,4 @@ -package errors +package errors2 import ( "errors" diff --git a/kit/platform/errors/errors.go b/kit/platform/errors/errors.go index 3d3f7e73664..f9c8cb8cc3d 100644 --- a/kit/platform/errors/errors.go +++ b/kit/platform/errors/errors.go @@ -7,9 +7,11 @@ import ( "fmt" "net/http" "strings" + + "go.etcd.io/bbolt" ) -// Some error code constant, ideally we want define common platform codes here +// Some error code constant, ideally we want to define common platform codes here // projects on use platform's error, should have their own central place like this. // Any time this set of constants changes, you must also update the swagger for Error.properties.code.enum. const ( @@ -81,6 +83,20 @@ func NewError(options ...func(*Error)) *Error { return err } +func (err *Error) Copy() *Error { + e := new(Error) + *e = *err + return e +} + +func (err *Error) Unwrap() error { + if err != nil { + return err.Err + } else { + return nil + } +} + // WithErrorErr sets the err on the error. func WithErrorErr(err error) func(*Error) { return func(e *Error) { @@ -111,7 +127,9 @@ func WithErrorOp(op string) func(*Error) { // Error implements the error interface by writing out the recursive messages. func (e *Error) Error() string { - if e.Msg != "" && e.Err != nil { + if e == nil { + return "" + } else if e.Msg != "" && e.Err != nil { var b strings.Builder b.WriteString(e.Msg) b.WriteString(": ") @@ -125,13 +143,22 @@ func (e *Error) Error() string { return fmt.Sprintf("<%s>", e.Code) } +func (e *Error) Is(err error) bool { + var errError *Error + + return errors.As(err, &errError) && + strings.Contains(e.Error(), err.Error()) && + e.Code == errError.Code +} + // ErrorCode returns the code of the root error, if available; otherwise returns EINTERNAL. func ErrorCode(err error) string { if err == nil { return "" } - e, ok := err.(*Error) + var e *Error + ok := errors.As(err, &e) if !ok { return EInternal } @@ -157,8 +184,8 @@ func ErrorOp(err error) string { return "" } - e, ok := err.(*Error) - if !ok { + var e *Error + if !errors.As(err, &e) { return "" } @@ -184,8 +211,8 @@ func ErrorMessage(err error) string { return "" } - e, ok := err.(*Error) - if !ok { + var e *Error + if !errors.As(err, &e) { return "An internal error has occurred." } @@ -193,12 +220,8 @@ func ErrorMessage(err error) string { return "" } - if e.Msg != "" { - return e.Msg - } - - if e.Err != nil { - return ErrorMessage(e.Err) + if msg := e.Error(); msg != "" { + return msg } return "An internal error has occurred." @@ -269,3 +292,53 @@ func decodeInternalError(target interface{}) error { type HTTPErrorHandler interface { HandleHTTPError(ctx context.Context, err error, w http.ResponseWriter) } + +func BoltToInfluxError(err error) error { + var e *Error + ok := errors.As(err, &e) + switch { + case err == nil: + return nil + case ok: + // Already an Influx error, we are good to go. + return e + case errors.Is(err, bbolt.ErrBucketNameRequired), errors.Is(err, bbolt.ErrKeyRequired): + return NewError(WithErrorErr(err), WithErrorCode(EEmptyValue)) + case errors.Is(err, bbolt.ErrIncompatibleValue): + return NewError(WithErrorErr(err), WithErrorCode(EConflict)) + case errors.Is(err, bbolt.ErrBucketNotFound): + return NewError(WithErrorErr(err), WithErrorCode(ENotFound)) + case errors.Is(err, bbolt.ErrBucketExists): + return NewError(WithErrorErr(err), WithErrorCode(EConflict)) + case errors.Is(err, bbolt.ErrKeyTooLarge), errors.Is(err, bbolt.ErrValueTooLarge): + return NewError(WithErrorErr(err), WithErrorCode(ETooLarge)) + default: + return err + } +} + +func ErrInternalServiceError(err error, options ...func(*Error)) error { + var e *Error + + if err == nil { + return nil + } else if !errors.As(err, &e) { + setters := make([]func(*Error), 0, len(options)+2) + // Defaults first, so they can be overridden by arguments. + setters = append(setters, WithErrorErr(err), WithErrorCode(EInternal)) + setters = append(setters, options...) + return NewError(setters...) + } else { + // Copy the Error struct because many are + // global variables/pseudo-constants we don't + // want to modify + e = e.Copy() + if e.Code == "" { + WithErrorCode(EInternal)(e) + } + for _, o := range options { + o(e) + } + return e + } +} diff --git a/kit/transport/http/error_handler.go b/kit/transport/http/error_handler.go index 8eb185fc985..3e4e0de4d8e 100644 --- a/kit/transport/http/error_handler.go +++ b/kit/transport/http/error_handler.go @@ -35,7 +35,8 @@ func (h ErrorHandler) HandleHTTPError(ctx context.Context, err error, w http.Res code := errors2.ErrorCode(err) var msg string - if _, ok := err.(*errors2.Error); ok { + var eTmp *errors2.Error + if ok := errors.As(err, &eTmp); ok { msg = err.Error() } else { msg = "An internal error has occurred - check server logs" diff --git a/label/error.go b/label/error.go index b71999e59d3..1d844491a5e 100644 --- a/label/error.go +++ b/label/error.go @@ -24,11 +24,3 @@ var ( Msg: "label not found", } ) - -// ErrInternalServiceError is used when the error comes from an internal system. -func ErrInternalServiceError(err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Err: err, - } -} diff --git a/notification/endpoint/endpoint_test.go b/notification/endpoint/endpoint_test.go index eec3cb8e6a2..1b44bf71c46 100644 --- a/notification/endpoint/endpoint_test.go +++ b/notification/endpoint/endpoint_test.go @@ -2,6 +2,7 @@ package endpoint_test import ( "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -10,7 +11,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/influxdata/influxdb/v2" - "github.com/influxdata/influxdb/v2/kit/errors" errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/mock" "github.com/influxdata/influxdb/v2/notification/endpoint" diff --git a/organization.go b/organization.go index 74f07c71716..50873cfc8da 100644 --- a/organization.go +++ b/organization.go @@ -2,7 +2,6 @@ package influxdb import ( "context" - "fmt" "github.com/influxdata/influxdb/v2/kit/platform" "github.com/influxdata/influxdb/v2/kit/platform/errors" @@ -78,12 +77,3 @@ type OrganizationFilter struct { ID *platform.ID UserID *platform.ID } - -func ErrInternalOrgServiceError(op string, err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Msg: fmt.Sprintf("unexpected error in organizations; Err: %v", err), - Op: op, - Err: err, - } -} diff --git a/query/control/controller.go b/query/control/controller.go index a461ad39635..1115fc1a7d3 100644 --- a/query/control/controller.go +++ b/query/control/controller.go @@ -19,6 +19,7 @@ package control import ( "context" + "errors" "fmt" "math" "runtime/debug" @@ -32,7 +33,7 @@ import ( "github.com/influxdata/flux/lang" "github.com/influxdata/flux/memory" "github.com/influxdata/flux/runtime" - "github.com/influxdata/influxdb/v2/kit/errors" + errors3 "github.com/influxdata/influxdb/v2/kit/errors" errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kit/prom" "github.com/influxdata/influxdb/v2/kit/tracing" @@ -154,7 +155,7 @@ func (c *Config) validate() error { if c.MaxMemoryBytes != 0 { // This is because we have to account for the per-query reserved memory and remove it from // the max total memory. If there is not a maximum number of queries this is not possible. - return errors.New("Cannot limit max memory when ConcurrencyQuota is unlimited") + return errors.New("cannot limit max memory when ConcurrencyQuota is unlimited") } } else { if c.QueueSize <= 0 { @@ -183,7 +184,7 @@ type QueryID uint64 func New(config Config, logger *zap.Logger) (*Controller, error) { c, err := config.complete(logger) if err != nil { - return nil, errors.Wrap(err, "invalid controller config") + return nil, errors3.Wrap(err, "invalid controller config") } metricLabelKeys := append(c.MetricLabelKeys, orgLabel) if logger == nil { diff --git a/query/stdlib/influxdata/influxdb/rules_test.go b/query/stdlib/influxdata/influxdb/rules_test.go index a069b366896..e3ce3e066bf 100644 --- a/query/stdlib/influxdata/influxdb/rules_test.go +++ b/query/stdlib/influxdata/influxdb/rules_test.go @@ -1163,9 +1163,7 @@ func meanProcedureSpec() *universe.MeanProcedureSpec { } } -// // Window Aggregate Testing -// func TestPushDownWindowAggregateRule(t *testing.T) { rules := []plan.Rule{ universe.AggregateWindowRule{}, @@ -2724,9 +2722,7 @@ func TestPushDownBareAggregateRule(t *testing.T) { } } -// // Group Aggregate Testing -// func TestPushDownGroupAggregateRule(t *testing.T) { readGroupAgg := func(aggregateMethod string) *influxdb.ReadGroupPhysSpec { return &influxdb.ReadGroupPhysSpec{ diff --git a/replications/remotewrite/writer_test.go b/replications/remotewrite/writer_test.go index 93c45d5a319..161a47cae9c 100644 --- a/replications/remotewrite/writer_test.go +++ b/replications/remotewrite/writer_test.go @@ -15,6 +15,7 @@ import ( "github.com/golang/mock/gomock" "github.com/influxdata/influxdb/v2" "github.com/influxdata/influxdb/v2/kit/platform" + errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kit/prom" "github.com/influxdata/influxdb/v2/kit/prom/promtest" ihttp "github.com/influxdata/influxdb/v2/kit/transport/http" @@ -419,9 +420,10 @@ func TestPostWrite(t *testing.T) { testData := []byte("some data") tests := []struct { - status int - bodyErr error - wantErr bool + status int + influxErr string + bodyErr error + wantErr bool }{ { status: http.StatusOK, @@ -432,14 +434,16 @@ func TestPostWrite(t *testing.T) { wantErr: false, }, { - status: http.StatusBadRequest, - wantErr: true, - bodyErr: fmt.Errorf("This is a terrible error: %w", errors.New("there are bad things here")), + status: http.StatusBadRequest, + influxErr: errors2.EEmptyValue, + wantErr: true, + bodyErr: fmt.Errorf("This is a terrible error: %w", errors.New("there are bad things here")), }, { - status: http.StatusMethodNotAllowed, - wantErr: true, - bodyErr: fmt.Errorf("method not allowed: %w", errors.New("what were you thinking")), + status: http.StatusMethodNotAllowed, + influxErr: errors2.EMethodNotAllowed, + wantErr: true, + bodyErr: fmt.Errorf("method not allowed: %w", errors.New("what were you thinking")), }, } @@ -451,8 +455,7 @@ func TestPostWrite(t *testing.T) { require.Equal(t, testData, recData) if tt.bodyErr != nil { - influxErrorCode := ihttp.StatusCodeToErrorCode(tt.status) - ihttp.WriteErrorResponse(context.Background(), w, influxErrorCode, tt.bodyErr.Error()) + ihttp.WriteErrorResponse(context.Background(), w, tt.influxErr, tt.bodyErr.Error()) } else { w.WriteHeader(tt.status) } diff --git a/sqlite/migrator_test.go b/sqlite/migrator_test.go index a7592627bce..bb5d318716e 100644 --- a/sqlite/migrator_test.go +++ b/sqlite/migrator_test.go @@ -8,8 +8,8 @@ import ( "os" "testing" - "github.com/influxdata/influxdb/v2/kit/errors" "github.com/influxdata/influxdb/v2/kit/migration" + "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/sqlite/test_migrations" "github.com/stretchr/testify/require" "go.uber.org/zap" diff --git a/storage/reads/aggregate_resultset.go b/storage/reads/aggregate_resultset.go index 92023f64795..1d41539457d 100644 --- a/storage/reads/aggregate_resultset.go +++ b/storage/reads/aggregate_resultset.go @@ -6,7 +6,7 @@ import ( "github.com/influxdata/flux/interval" "github.com/influxdata/flux/values" - "github.com/influxdata/influxdb/v2/kit/errors" + errors2 "github.com/influxdata/influxdb/v2/kit/errors" "github.com/influxdata/influxdb/v2/kit/tracing" "github.com/influxdata/influxdb/v2/models" "github.com/influxdata/influxdb/v2/storage/reads/datatypes" @@ -58,7 +58,7 @@ func NewWindowAggregateResultSet(ctx context.Context, req *datatypes.ReadWindowA } if nAggs := len(req.Aggregate); nAggs != 1 { - return nil, errors.Errorf(errors.InternalError, "attempt to create a windowAggregateResultSet with %v aggregate functions", nAggs) + return nil, errors2.Errorf(errors2.InternalError, "attempt to create a windowAggregateResultSet with %v aggregate functions", nAggs) } ascending := !IsLastDescendingAggregateOptimization(req) diff --git a/task/backend/analytical_storage.go b/task/backend/analytical_storage.go index 3c41170c2de..bd51f21ec2e 100644 --- a/task/backend/analytical_storage.go +++ b/task/backend/analytical_storage.go @@ -4,15 +4,15 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "time" "github.com/influxdata/flux" "github.com/influxdata/flux/lang" "github.com/influxdata/influxdb/v2" - "github.com/influxdata/influxdb/v2/kit/errors" "github.com/influxdata/influxdb/v2/kit/platform" - errors2 "github.com/influxdata/influxdb/v2/kit/platform/errors" + errors3 "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/query" "github.com/influxdata/influxdb/v2/storage" "github.com/influxdata/influxdb/v2/task/taskmodel" @@ -260,7 +260,7 @@ func (as *AnalyticalStorage) FindRunByID(ctx context.Context, taskID, runID plat // check the taskService to see if the run is on its list run, err := as.TaskService.FindRunByID(ctx, taskID, runID) if err != nil { - if err, ok := err.(*errors2.Error); !ok || err.Msg != "run not found" { + if err, ok := err.(*errors3.Error); !ok || err.Msg != "run not found" { return run, err } } @@ -332,9 +332,9 @@ func (as *AnalyticalStorage) FindRunByID(ctx context.Context, taskID, runID plat } if len(re.runs) != 1 { - return nil, &errors2.Error{ + return nil, &errors3.Error{ Msg: "found multiple runs with id " + runID.String(), - Code: errors2.EInternal, + Code: errors3.EInternal, } } @@ -344,7 +344,7 @@ func (as *AnalyticalStorage) FindRunByID(ctx context.Context, taskID, runID plat func (as *AnalyticalStorage) RetryRun(ctx context.Context, taskID, runID platform.ID) (*taskmodel.Run, error) { run, err := as.TaskService.RetryRun(ctx, taskID, runID) if err != nil { - if err, ok := err.(*errors2.Error); !ok || err.Msg != "run not found" { + if err, ok := err.(*errors3.Error); !ok || err.Msg != "run not found" { return run, err } } diff --git a/tenant/error.go b/tenant/error.go index 90ec96cb036..f38819e34b1 100644 --- a/tenant/error.go +++ b/tenant/error.go @@ -40,14 +40,6 @@ var ( } ) -// ErrInternalServiceError is used when the error comes from an internal system. -func ErrInternalServiceError(err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Err: err, - } -} - type errSlice []error func (e errSlice) Error() string { diff --git a/tenant/http_server_org_test.go b/tenant/http_server_org_test.go index af56ab9e8b5..9821038c376 100644 --- a/tenant/http_server_org_test.go +++ b/tenant/http_server_org_test.go @@ -55,5 +55,5 @@ func initHttpOrgService(f itesting.OrganizationFields, t *testing.T) (influxdb.O } func TestHTTPOrgService(t *testing.T) { - itesting.OrganizationService(initHttpOrgService, t) + itesting.OrganizationService(initHttpOrgService, true, t) } diff --git a/tenant/http_server_user_test.go b/tenant/http_server_user_test.go index 325395d534a..709759857e9 100644 --- a/tenant/http_server_user_test.go +++ b/tenant/http_server_user_test.go @@ -48,5 +48,5 @@ func initHttpUserService(f platformtesting.UserFields, t *testing.T) (platform.U func TestUserService(t *testing.T) { t.Parallel() - platformtesting.UserService(initHttpUserService, t) + platformtesting.UserService(initHttpUserService, true, t) } diff --git a/tenant/middleware_org_logging_test.go b/tenant/middleware_org_logging_test.go index ede755bf0ff..648dd046af3 100644 --- a/tenant/middleware_org_logging_test.go +++ b/tenant/middleware_org_logging_test.go @@ -10,7 +10,7 @@ import ( ) func TestOrganizationLoggingService(t *testing.T) { - influxdbtesting.OrganizationService(initBoltOrganizationLoggingService, t) + influxdbtesting.OrganizationService(initBoltOrganizationLoggingService, false, t) } func initBoltOrganizationLoggingService(f influxdbtesting.OrganizationFields, t *testing.T) (influxdb.OrganizationService, string, func()) { diff --git a/tenant/middleware_user_logging_test.go b/tenant/middleware_user_logging_test.go index 6ea0e24091b..2e1fd8b5348 100644 --- a/tenant/middleware_user_logging_test.go +++ b/tenant/middleware_user_logging_test.go @@ -10,7 +10,7 @@ import ( ) func TestUserLoggingService(t *testing.T) { - influxdbtesting.UserService(initBoltUserLoggingService, t) + influxdbtesting.UserService(initBoltUserLoggingService, false, t) } func initBoltUserLoggingService(f influxdbtesting.UserFields, t *testing.T) (influxdb.UserService, string, func()) { diff --git a/tenant/service_org_test.go b/tenant/service_org_test.go index 5d90aaba124..fa6bcc5d236 100644 --- a/tenant/service_org_test.go +++ b/tenant/service_org_test.go @@ -11,7 +11,7 @@ import ( ) func TestBoltOrganizationService(t *testing.T) { - influxdbtesting.OrganizationService(initBoltOrganizationService, t) + influxdbtesting.OrganizationService(initBoltOrganizationService, false, t) } func initBoltOrganizationService(f influxdbtesting.OrganizationFields, t *testing.T) (influxdb.OrganizationService, string, func()) { diff --git a/tenant/service_user_test.go b/tenant/service_user_test.go index a1106083ec9..a6b2f8bbc92 100644 --- a/tenant/service_user_test.go +++ b/tenant/service_user_test.go @@ -13,7 +13,7 @@ import ( ) func TestBoltUserService(t *testing.T) { - influxdbtesting.UserService(initBoltUserService, t) + influxdbtesting.UserService(initBoltUserService, false, t) } func initBoltUserService(f influxdbtesting.UserFields, t *testing.T) (influxdb.UserService, string, func()) { diff --git a/tenant/storage_bucket.go b/tenant/storage_bucket.go index 96c7c93c91b..15b50776054 100644 --- a/tenant/storage_bucket.go +++ b/tenant/storage_bucket.go @@ -8,6 +8,7 @@ import ( "github.com/influxdata/influxdb/v2/kit/platform" "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kv" + errors2 "github.com/influxdata/influxdb/v2/pkg/errors" ) var ( @@ -57,7 +58,7 @@ func (s *Store) uniqueBucketName(ctx context.Context, tx kv.Tx, oid platform.ID, } // any other error is some sort of internal server error - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } func unmarshalBucket(v []byte) (*influxdb.Bucket, error) { @@ -78,7 +79,10 @@ func marshalBucket(u *influxdb.Bucket) ([]byte, error) { return v, nil } -func (s *Store) GetBucket(ctx context.Context, tx kv.Tx, id platform.ID) (*influxdb.Bucket, error) { +func (s *Store) GetBucket(ctx context.Context, tx kv.Tx, id platform.ID) (bucket *influxdb.Bucket, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindBucketByID)) + }() encodedID, err := id.Encode() if err != nil { return nil, InvalidOrgIDError(err) @@ -95,19 +99,19 @@ func (s *Store) GetBucket(ctx context.Context, tx kv.Tx, id platform.ID) (*influ } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } return unmarshalBucket(v) } -func (s *Store) GetBucketByName(ctx context.Context, tx kv.Tx, orgID platform.ID, n string) (*influxdb.Bucket, error) { +func (s *Store) GetBucketByName(ctx context.Context, tx kv.Tx, orgID platform.ID, n string) (bucket *influxdb.Bucket, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindBucket)) + }() key, err := bucketIndexKey(orgID, n) if err != nil { - return nil, &errors.Error{ - Code: errors.EInvalid, - Err: err, - } + return nil, errors.ErrInternalServiceError(err, errors.WithErrorCode(errors.EInvalid)) } idx, err := tx.Bucket(bucketIndex) @@ -117,7 +121,7 @@ func (s *Store) GetBucketByName(ctx context.Context, tx kv.Tx, orgID platform.ID buf, err := idx.Get(key) - // allow for hard coded bucket names that dont exist in the system + // allow for hard coded bucket names that don't exist in the system if kv.IsNotFound(err) { return nil, ErrBucketNotFoundByName(n) } @@ -128,9 +132,7 @@ func (s *Store) GetBucketByName(ctx context.Context, tx kv.Tx, orgID platform.ID var id platform.ID if err := id.Decode(buf); err != nil { - return nil, &errors.Error{ - Err: err, - } + return nil, err } return s.GetBucket(ctx, tx, id) } @@ -140,7 +142,10 @@ type BucketFilter struct { OrganizationID *platform.ID } -func (s *Store) ListBuckets(ctx context.Context, tx kv.Tx, filter BucketFilter, opt ...influxdb.FindOptions) ([]*influxdb.Bucket, error) { +func (s *Store) ListBuckets(ctx context.Context, tx kv.Tx, filter BucketFilter, opt ...influxdb.FindOptions) (buckets []*influxdb.Bucket, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindBuckets)) + }() // this isn't a list action its a `GetBucketByName` if (filter.OrganizationID != nil && filter.OrganizationID.Valid()) && filter.Name != nil { return nil, invalidBucketListRequest @@ -179,7 +184,7 @@ func (s *Store) ListBuckets(ctx context.Context, tx kv.Tx, filter BucketFilter, if err != nil { return nil, err } - defer cursor.Close() + defer errors2.Capture(&retErr, cursor.Close)() count := 0 bs := []*influxdb.Bucket{} @@ -206,14 +211,11 @@ func (s *Store) ListBuckets(ctx context.Context, tx kv.Tx, filter BucketFilter, return bs, cursor.Err() } -func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.ID, o influxdb.FindOptions) ([]*influxdb.Bucket, error) { +func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.ID, o influxdb.FindOptions) (buckets []*influxdb.Bucket, retErr error) { // get the prefix key (org id with an empty name) key, err := bucketIndexKey(orgID, "") if err != nil { - return nil, &errors.Error{ - Code: errors.EInvalid, - Err: err, - } + return nil, errors.ErrInternalServiceError(err, errors.WithErrorCode(errors.EInvalid)) } idx, err := tx.Bucket(bucketIndex) @@ -233,7 +235,7 @@ func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.I if err != nil { return nil, err } - defer cursor.Close() + defer errors2.Capture(&retErr, cursor.Close)() lastKey := start for k, _ := cursor.Next(); k != nil; k, _ = cursor.Next() { @@ -251,8 +253,7 @@ func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.I if err != nil { return nil, err } - defer cursor.Close() - + defer errors2.Capture(&retErr, cursor.Close)() count := 0 bs := []*influxdb.Bucket{} searchingForAfter := o.After != nil @@ -268,9 +269,7 @@ func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.I var id platform.ID if err := id.Decode(v); err != nil { - return nil, &errors.Error{ - Err: err, - } + return nil, err } if searchingForAfter { searchingForAfter = id != *o.After @@ -292,6 +291,9 @@ func (s *Store) listBucketsByOrg(ctx context.Context, tx kv.Tx, orgID platform.I } func (s *Store) CreateBucket(ctx context.Context, tx kv.Tx, bucket *influxdb.Bucket) (err error) { + defer func() { + err = errors.ErrInternalServiceError(err, errors.WithErrorOp(influxdb.OpCreateBucket)) + }() // generate new bucket ID bucket.ID, err = s.generateSafeID(ctx, tx, bucketBucket, s.BucketIDGen) if err != nil { @@ -330,17 +332,20 @@ func (s *Store) CreateBucket(ctx context.Context, tx kv.Tx, bucket *influxdb.Buc } if err := idx.Put(ikey, encodedID); err != nil { - return ErrInternalServiceError(err) + return err } if err := b.Put(encodedID, v); err != nil { - return ErrInternalServiceError(err) + return err } return nil } -func (s *Store) UpdateBucket(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.BucketUpdate) (*influxdb.Bucket, error) { +func (s *Store) UpdateBucket(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.BucketUpdate) (retBucket *influxdb.Bucket, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpUpdateBucket)) + }() encodedID, err := id.Encode() if err != nil { return nil, err @@ -377,7 +382,7 @@ func (s *Store) UpdateBucket(ctx context.Context, tx kv.Tx, id platform.ID, upd } if err := idx.Delete(oldIkey); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } bucket.Name = *upd.Name @@ -387,7 +392,7 @@ func (s *Store) UpdateBucket(ctx context.Context, tx kv.Tx, id platform.ID, upd } if err := idx.Put(newIkey, encodedID); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } } @@ -412,13 +417,16 @@ func (s *Store) UpdateBucket(ctx context.Context, tx kv.Tx, id platform.ID, upd return nil, err } if err := b.Put(encodedID, v); err != nil { - return nil, ErrInternalServiceError(err) + return nil, errors.ErrInternalServiceError(err) } return bucket, nil } -func (s *Store) DeleteBucket(ctx context.Context, tx kv.Tx, id platform.ID) error { +func (s *Store) DeleteBucket(ctx context.Context, tx kv.Tx, id platform.ID) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpDeleteBucket)) + }() bucket, err := s.GetBucket(ctx, tx, id) if err != nil { return err @@ -439,7 +447,7 @@ func (s *Store) DeleteBucket(ctx context.Context, tx kv.Tx, id platform.ID) erro return err } if err := idx.Delete(ikey); err != nil { - return ErrInternalServiceError(err) + return err } b, err := tx.Bucket(bucketBucket) @@ -448,7 +456,7 @@ func (s *Store) DeleteBucket(ctx context.Context, tx kv.Tx, id platform.ID) erro } if err := b.Delete(encodedID); err != nil { - return ErrInternalServiceError(err) + return err } return nil diff --git a/tenant/storage_bucket_test.go b/tenant/storage_bucket_test.go index 1777d9d5852..3bf9368ce1f 100644 --- a/tenant/storage_bucket_test.go +++ b/tenant/storage_bucket_test.go @@ -2,6 +2,7 @@ package tenant_test import ( "context" + "errors" "fmt" "reflect" "testing" @@ -142,7 +143,7 @@ func TestBucket(t *testing.T) { require.NoError(t, err) assert.Equal(t, expected, bucket) - if _, err := store.GetBucket(context.Background(), tx, 11); err != tenant.ErrBucketNotFound { + if _, err := store.GetBucket(context.Background(), tx, 11); !errors.Is(err, tenant.ErrBucketNotFound) { t.Fatal("failed to get correct error when looking for non present bucket by id") } @@ -323,7 +324,7 @@ func TestBucket(t *testing.T) { update: func(t *testing.T, store *tenant.Store, tx kv.Tx) { bucket5 := "bucket5" _, err := store.UpdateBucket(context.Background(), tx, thirdBucketID, influxdb.BucketUpdate{Name: &bucket5}) - if err != tenant.ErrBucketNameNotUnique { + if !errors.Is(err, tenant.ErrBucketNameNotUnique) { t.Fatal("failed to error on duplicate bucketname") } @@ -355,7 +356,7 @@ func TestBucket(t *testing.T) { require.NoError(t, err) err = store.DeleteBucket(context.Background(), tx, firstBucketID) - if err != tenant.ErrBucketNotFound { + if !errors.Is(err, tenant.ErrBucketNotFound) { t.Fatal("invalid error when deleting bucket that has already been deleted", err) } diff --git a/tenant/storage_org.go b/tenant/storage_org.go index e71db36700c..6f37f860f9f 100644 --- a/tenant/storage_org.go +++ b/tenant/storage_org.go @@ -7,7 +7,9 @@ import ( "github.com/influxdata/influxdb/v2" "github.com/influxdata/influxdb/v2/kit/platform" + "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kv" + errors2 "github.com/influxdata/influxdb/v2/pkg/errors" ) var ( @@ -24,7 +26,7 @@ func (s *Store) uniqueOrgName(ctx context.Context, tx kv.Tx, uname string) error idx, err := tx.Bucket(organizationIndex) if err != nil { - return err + return errors.ErrInternalServiceError(err) } _, err = idx.Get(key) @@ -39,7 +41,7 @@ func (s *Store) uniqueOrgName(ctx context.Context, tx kv.Tx, uname string) error } // any other error is some sort of internal server error - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } func organizationIndexKey(n string) []byte { @@ -64,7 +66,10 @@ func marshalOrg(u *influxdb.Organization) ([]byte, error) { return v, nil } -func (s *Store) GetOrg(ctx context.Context, tx kv.Tx, id platform.ID) (*influxdb.Organization, error) { +func (s *Store) GetOrg(ctx context.Context, tx kv.Tx, id platform.ID) (org *influxdb.Organization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindOrganizationByID)) + }() encodedID, err := id.Encode() if err != nil { return nil, InvalidOrgIDError(err) @@ -81,13 +86,16 @@ func (s *Store) GetOrg(ctx context.Context, tx kv.Tx, id platform.ID) (*influxdb } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } return unmarshalOrg(v) } -func (s *Store) GetOrgByName(ctx context.Context, tx kv.Tx, n string) (*influxdb.Organization, error) { +func (s *Store) GetOrgByName(ctx context.Context, tx kv.Tx, n string) (org *influxdb.Organization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindOrganization)) + }() b, err := tx.Bucket(organizationIndex) if err != nil { return nil, err @@ -99,7 +107,7 @@ func (s *Store) GetOrgByName(ctx context.Context, tx kv.Tx, n string) (*influxdb } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } var id platform.ID @@ -109,8 +117,11 @@ func (s *Store) GetOrgByName(ctx context.Context, tx kv.Tx, n string) (*influxdb return s.GetOrg(ctx, tx, id) } -func (s *Store) ListOrgs(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOptions) ([]*influxdb.Organization, error) { - // if we dont have any options it would be irresponsible to just give back all orgs in the system +func (s *Store) ListOrgs(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOptions) (orgs []*influxdb.Organization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindOrganizations)) + }() + // if we don't have any options it would be irresponsible to just give back all orgs in the system if len(opt) == 0 { opt = append(opt, influxdb.FindOptions{}) } @@ -125,7 +136,7 @@ func (s *Store) ListOrgs(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOpti if err != nil { return nil, err } - defer cursor.Close() + defer errors2.Capture(&retErr, cursor.Close)() count := 0 us := []*influxdb.Organization{} @@ -150,6 +161,9 @@ func (s *Store) ListOrgs(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOpti } func (s *Store) CreateOrg(ctx context.Context, tx kv.Tx, o *influxdb.Organization) (err error) { + defer func() { + err = errors.ErrInternalServiceError(err, errors.WithErrorOp(influxdb.OpCreateOrganization)) + }() // if ID is provided then ensure it is unique // generate new bucket ID o.ID, err = s.generateSafeID(ctx, tx, organizationBucket, s.OrgIDGen) @@ -184,17 +198,20 @@ func (s *Store) CreateOrg(ctx context.Context, tx kv.Tx, o *influxdb.Organizatio } if err := idx.Put(organizationIndexKey(o.Name), encodedID); err != nil { - return ErrInternalServiceError(err) + return err } if err := b.Put(encodedID, v); err != nil { - return ErrInternalServiceError(err) + return err } return nil } -func (s *Store) UpdateOrg(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.OrganizationUpdate) (*influxdb.Organization, error) { +func (s *Store) UpdateOrg(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.OrganizationUpdate) (org *influxdb.Organization, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpUpdateOrganization)) + }() encodedID, err := id.Encode() if err != nil { return nil, err @@ -217,13 +234,13 @@ func (s *Store) UpdateOrg(ctx context.Context, tx kv.Tx, id platform.ID, upd inf } if err := idx.Delete(organizationIndexKey(u.Name)); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } u.Name = *upd.Name if err := idx.Put(organizationIndexKey(*upd.Name), encodedID); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } } @@ -241,13 +258,16 @@ func (s *Store) UpdateOrg(ctx context.Context, tx kv.Tx, id platform.ID, upd inf return nil, err } if err := b.Put(encodedID, v); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } return u, nil } -func (s *Store) DeleteOrg(ctx context.Context, tx kv.Tx, id platform.ID) error { +func (s *Store) DeleteOrg(ctx context.Context, tx kv.Tx, id platform.ID) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpDeleteOrganization)) + }() u, err := s.GetOrg(ctx, tx, id) if err != nil { return err @@ -264,7 +284,7 @@ func (s *Store) DeleteOrg(ctx context.Context, tx kv.Tx, id platform.ID) error { } if err := idx.Delete([]byte(u.Name)); err != nil { - return ErrInternalServiceError(err) + return err } b, err := tx.Bucket(organizationBucket) @@ -273,7 +293,7 @@ func (s *Store) DeleteOrg(ctx context.Context, tx kv.Tx, id platform.ID) error { } if err := b.Delete(encodedID); err != nil { - return ErrInternalServiceError(err) + return err } return nil diff --git a/tenant/storage_org_test.go b/tenant/storage_org_test.go index aab8533637c..d6a71c68f84 100644 --- a/tenant/storage_org_test.go +++ b/tenant/storage_org_test.go @@ -2,6 +2,7 @@ package tenant_test import ( "context" + "errors" "fmt" "testing" "time" @@ -121,7 +122,7 @@ func TestOrg(t *testing.T) { } require.Equal(t, expected, org) - if _, err := store.GetOrg(context.Background(), tx, 500); err != tenant.ErrOrgNotFound { + if _, err := store.GetOrg(context.Background(), tx, 500); !errors.Is(err, tenant.ErrOrgNotFound) { t.Fatal("failed to get correct error when looking for invalid org by id") } @@ -205,7 +206,7 @@ func TestOrg(t *testing.T) { require.NoError(t, err) err = store.DeleteOrg(context.Background(), tx, firstOrgID) - if err != tenant.ErrOrgNotFound { + if !errors.Is(err, tenant.ErrOrgNotFound) { t.Fatal("invalid error when deleting org that has already been deleted", err) } diff --git a/tenant/storage_user.go b/tenant/storage_user.go index c937bdcb549..0d813519e5d 100644 --- a/tenant/storage_user.go +++ b/tenant/storage_user.go @@ -6,7 +6,9 @@ import ( "github.com/influxdata/influxdb/v2" "github.com/influxdata/influxdb/v2/kit/platform" + "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kv" + errors2 "github.com/influxdata/influxdb/v2/pkg/errors" ) var ( @@ -38,7 +40,7 @@ func (s *Store) uniqueUserName(tx kv.Tx, uname string) error { idx, err := tx.Bucket(userIndex) if err != nil { - return err + return errors.ErrInternalServiceError(err) } _, err = idx.Get([]byte(uname)) @@ -61,7 +63,7 @@ func (s *Store) uniqueUserID(tx kv.Tx, id platform.ID) error { b, err := tx.Bucket(userBucket) if err != nil { - return err + return errors.ErrInternalServiceError(err) } _, err = b.Get(encodedID) @@ -76,7 +78,10 @@ func (s *Store) uniqueUserID(tx kv.Tx, id platform.ID) error { return ErrUnprocessableUser(err) } -func (s *Store) GetUser(ctx context.Context, tx kv.Tx, id platform.ID) (*influxdb.User, error) { +func (s *Store) GetUser(ctx context.Context, tx kv.Tx, id platform.ID) (user *influxdb.User, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindUserByID)) + }() encodedID, err := id.Encode() if err != nil { return nil, InvalidUserIDError(err) @@ -93,13 +98,16 @@ func (s *Store) GetUser(ctx context.Context, tx kv.Tx, id platform.ID) (*influxd } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } return unmarshalUser(v) } -func (s *Store) GetUserByName(ctx context.Context, tx kv.Tx, n string) (*influxdb.User, error) { +func (s *Store) GetUserByName(ctx context.Context, tx kv.Tx, n string) (user *influxdb.User, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindUser)) + }() b, err := tx.Bucket(userIndex) if err != nil { return nil, err @@ -111,7 +119,7 @@ func (s *Store) GetUserByName(ctx context.Context, tx kv.Tx, n string) (*influxd } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } var id platform.ID @@ -121,7 +129,10 @@ func (s *Store) GetUserByName(ctx context.Context, tx kv.Tx, n string) (*influxd return s.GetUser(ctx, tx, id) } -func (s *Store) ListUsers(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOptions) ([]*influxdb.User, error) { +func (s *Store) ListUsers(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOptions) (users []*influxdb.User, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpFindUsers)) + }() if len(opt) == 0 { opt = append(opt, influxdb.FindOptions{}) } @@ -150,7 +161,7 @@ func (s *Store) ListUsers(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOpt if err != nil { return nil, err } - defer cursor.Close() + defer errors2.Capture(&retErr, cursor.Close)() count := 0 us := []*influxdb.User{} @@ -174,7 +185,10 @@ func (s *Store) ListUsers(ctx context.Context, tx kv.Tx, opt ...influxdb.FindOpt return us, cursor.Err() } -func (s *Store) CreateUser(ctx context.Context, tx kv.Tx, u *influxdb.User) error { +func (s *Store) CreateUser(ctx context.Context, tx kv.Tx, u *influxdb.User) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpCreateUser)) + }() if !u.ID.Valid() { u.ID = s.IDGen.ID() } @@ -208,17 +222,20 @@ func (s *Store) CreateUser(ctx context.Context, tx kv.Tx, u *influxdb.User) erro } if err := idx.Put([]byte(u.Name), encodedID); err != nil { - return ErrInternalServiceError(err) + return err } if err := b.Put(encodedID, v); err != nil { - return ErrInternalServiceError(err) + return err } return nil } -func (s *Store) UpdateUser(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.UserUpdate) (*influxdb.User, error) { +func (s *Store) UpdateUser(ctx context.Context, tx kv.Tx, id platform.ID, upd influxdb.UserUpdate) (user *influxdb.User, retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpUpdateUser)) + }() encodedID, err := id.Encode() if err != nil { return nil, err @@ -240,13 +257,13 @@ func (s *Store) UpdateUser(ctx context.Context, tx kv.Tx, id platform.ID, upd in } if err := idx.Delete([]byte(u.Name)); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } u.Name = *upd.Name if err := idx.Put([]byte(u.Name), encodedID); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } } @@ -264,13 +281,16 @@ func (s *Store) UpdateUser(ctx context.Context, tx kv.Tx, id platform.ID, upd in return nil, err } if err := b.Put(encodedID, v); err != nil { - return nil, ErrInternalServiceError(err) + return nil, err } return u, nil } -func (s *Store) DeleteUser(ctx context.Context, tx kv.Tx, id platform.ID) error { +func (s *Store) DeleteUser(ctx context.Context, tx kv.Tx, id platform.ID) (retErr error) { + defer func() { + retErr = errors.ErrInternalServiceError(retErr, errors.WithErrorOp(influxdb.OpDeleteUser)) + }() u, err := s.GetUser(ctx, tx, id) if err != nil { return err @@ -287,7 +307,7 @@ func (s *Store) DeleteUser(ctx context.Context, tx kv.Tx, id platform.ID) error } if err := idx.Delete([]byte(u.Name)); err != nil { - return ErrInternalServiceError(err) + return err } b, err := tx.Bucket(userBucket) @@ -296,10 +316,10 @@ func (s *Store) DeleteUser(ctx context.Context, tx kv.Tx, id platform.ID) error } if err := b.Delete(encodedID); err != nil { - return ErrInternalServiceError(err) + return err } - // Clean up users password. + // Clean up user's password. ub, err := tx.Bucket(userpasswordBucket) if err != nil { return UnavailablePasswordServiceError(err) diff --git a/tenant/storage_user_test.go b/tenant/storage_user_test.go index a7e35d6de55..9f3ef3eac0f 100644 --- a/tenant/storage_user_test.go +++ b/tenant/storage_user_test.go @@ -2,6 +2,7 @@ package tenant_test import ( "context" + "errors" "fmt" "reflect" "testing" @@ -118,11 +119,11 @@ func TestUser(t *testing.T) { t.Fatalf("expected identical user: \n%+v\n%+v", user, expected) } - if _, err := store.GetUser(context.Background(), tx, 500); err != tenant.ErrUserNotFound { + if _, err := store.GetUser(context.Background(), tx, 500); !errors.Is(err, tenant.ErrUserNotFound) { t.Fatal("failed to get correct error when looking for invalid user by id") } - if _, err := store.GetUserByName(context.Background(), tx, "notauser"); err != tenant.ErrUserNotFound { + if _, err := store.GetUserByName(context.Background(), tx, "notauser"); !errors.Is(err, tenant.ErrUserNotFound) { t.Fatal("failed to get correct error when looking for invalid user by name") } @@ -249,7 +250,7 @@ func TestUser(t *testing.T) { } err = store.DeleteUser(context.Background(), tx, 1) - if err != tenant.ErrUserNotFound { + if !errors.Is(err, tenant.ErrUserNotFound) { t.Fatal("invalid error when deleting user that has already been deleted", err) } diff --git a/testing/auth.go b/testing/auth.go index 35e1aec7fb8..d1badb8cbc4 100644 --- a/testing/auth.go +++ b/testing/auth.go @@ -316,7 +316,7 @@ func CreateAuthorization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateAuthorization(ctx, tt.args.authorization) @@ -324,7 +324,7 @@ func CreateAuthorization( t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) } - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) defer s.DeleteAuthorization(ctx, tt.args.authorization.ID) @@ -416,13 +416,13 @@ func FindAuthorizationByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() for i := range tt.fields.Authorizations { authorization, err := s.FindAuthorizationByID(ctx, tt.fields.Authorizations[i].ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(authorization, tt.wants.authorizations[i], authorizationCmpOptions...); diff != "" { t.Errorf("authorization is different -got/+want\ndiff %s", diff) @@ -670,12 +670,12 @@ func UpdateAuthorization( } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() updatedAuth, err := s.UpdateAuthorization(ctx, tt.args.id, tt.args.upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if tt.wants.err == nil { authorization, err := s.FindAuthorizationByID(ctx, tt.args.id) @@ -848,12 +848,12 @@ func FindAuthorizationByToken( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() authorization, err := s.FindAuthorizationByToken(ctx, tt.args.token) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(authorization, tt.wants.authorization, authorizationCmpOptions...); diff != "" { t.Errorf("authorization is different -got/+want\ndiff %s", diff) @@ -1158,7 +1158,7 @@ func FindAuthorizations( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1177,7 +1177,7 @@ func FindAuthorizations( } authorizations, _, err := s.FindAuthorizations(ctx, filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(authorizations, tt.wants.authorizations, authorizationCmpOptions...); diff != "" { t.Errorf("authorizations are different -got/+want\ndiff %s", diff) } @@ -1325,11 +1325,11 @@ func DeleteAuthorization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteAuthorization(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) filter := influxdb.AuthorizationFilter{} authorizations, _, err := s.FindAuthorizations(ctx, filter) diff --git a/testing/bucket_service.go b/testing/bucket_service.go index cd81bb0cb78..fcdb5970b57 100644 --- a/testing/bucket_service.go +++ b/testing/bucket_service.go @@ -375,11 +375,11 @@ func CreateBucket( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateBucket(ctx, tt.args.bucket) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) // Delete only newly created buckets - ie., with a not nil ID // if tt.args.bucket.ID.Valid() { @@ -500,12 +500,12 @@ func FindBucketByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() bucket, err := s.FindBucketByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(bucket, tt.wants.bucket, bucketCmpOptions...); diff != "" { t.Errorf("bucket is different -got/+want\ndiff %s", diff) @@ -888,7 +888,7 @@ func FindBuckets( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -907,7 +907,7 @@ func FindBuckets( } buckets, _, err := s.FindBuckets(ctx, filter, tt.args.findOptions) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) // remove system buckets filteredBuckets := []*influxdb.Bucket{} @@ -1070,11 +1070,11 @@ func DeleteBucket( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteBucket(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) filter := influxdb.BucketFilter{} buckets, _, err := s.FindBuckets(ctx, filter) @@ -1228,7 +1228,7 @@ func FindBucket( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() filter := influxdb.BucketFilter{} @@ -1243,7 +1243,7 @@ func FindBucket( } bucket, err := s.FindBucket(ctx, filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(bucket, tt.wants.bucket, bucketCmpOptions...); diff != "" { t.Errorf("buckets are different -got/+want\ndiff %s", diff) @@ -1718,7 +1718,7 @@ func UpdateBucket( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1738,7 +1738,7 @@ func UpdateBucket( upd.Description = tt.args.description bucket, err := s.UpdateBucket(ctx, tt.args.id, upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(bucket, tt.wants.bucket, bucketCmpOptions...); diff != "" { t.Errorf("bucket is different -got/+want\ndiff %s", diff) diff --git a/testing/label_service.go b/testing/label_service.go index cd09988ee1e..5fca8054ea6 100644 --- a/testing/label_service.go +++ b/testing/label_service.go @@ -271,11 +271,11 @@ func CreateLabel( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateLabel(ctx, tt.args.label) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) defer s.DeleteLabel(ctx, tt.args.label.ID) @@ -410,11 +410,11 @@ func FindLabels( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() labels, err := s.FindLabels(ctx, tt.args.filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(labels, tt.wants.labels, labelCmpOptions...); diff != "" { t.Errorf("labels are different -got/+want\ndiff %s", diff) @@ -488,11 +488,11 @@ func FindLabelByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() label, err := s.FindLabelByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(label, tt.wants.label, labelCmpOptions...); diff != "" { t.Errorf("labels are different -got/+want\ndiff %s", diff) @@ -762,11 +762,11 @@ func UpdateLabel( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() _, err := s.UpdateLabel(ctx, tt.args.labelID, tt.args.update) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) labels, err := s.FindLabels(ctx, influxdb.LabelFilter{}) if err != nil { @@ -859,11 +859,11 @@ func DeleteLabel( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteLabel(ctx, tt.args.labelID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) labels, err := s.FindLabels(ctx, influxdb.LabelFilter{}) if err != nil { @@ -949,11 +949,11 @@ func CreateLabelMapping( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateLabelMapping(ctx, tt.args.mapping) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) defer s.DeleteLabelMapping(ctx, tt.args.mapping) @@ -1025,11 +1025,11 @@ func DeleteLabelMapping( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteLabelMapping(ctx, tt.args.mapping) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) labels, err := s.FindResourceLabels(ctx, tt.args.filter) if err != nil { diff --git a/testing/organization_service.go b/testing/organization_service.go index e61f1400520..da10bf6d862 100644 --- a/testing/organization_service.go +++ b/testing/organization_service.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "sort" + "strings" "testing" "time" @@ -12,6 +13,7 @@ import ( "github.com/influxdata/influxdb/v2/kit/platform" "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/mock" + bolt "go.etcd.io/bbolt" ) var orgBucketsIDGenerator = mock.NewMockIDGenerator() @@ -47,12 +49,11 @@ type OrganizationFields struct { } // OrganizationService tests all the service functions. -func OrganizationService( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), t *testing.T, -) { +func OrganizationService(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { tests := []struct { name string fn func(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), + isInMem bool, t *testing.T) }{ { @@ -84,21 +85,19 @@ func OrganizationService( t.Run(tt.name, func(t *testing.T) { tt := tt t.Parallel() - tt.fn(init, t) + tt.fn(init, isInMem, t) }) } } // CreateOrganization testing -func CreateOrganization( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func CreateOrganization(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { organization *influxdb.Organization } type wants struct { err error + inMemNoError bool organizations []*influxdb.Organization } @@ -173,6 +172,36 @@ func CreateOrganization( }, }, }, + { + name: "huge name", + fields: OrganizationFields{ + IDGenerator: mock.NewMockIDGenerator(), + OrgBucketIDs: orgBucketsIDGenerator, + TimeGenerator: mock.TimeGenerator{FakeValue: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC)}, + Organizations: []*influxdb.Organization{ + { + ID: idOne, + Name: "organization1", + }, + }, + }, + args: args{ + organization: &influxdb.Organization{ + ID: idTwo, + Name: strings.Repeat("A Huge Organization Name", 10_000), + }, + }, + wants: wants{ + organizations: []*influxdb.Organization{ + { + ID: idOne, + Name: "organization1", + }, + }, + err: &errors.Error{Code: errors.ETooLarge, Err: bolt.ErrKeyTooLarge}, + inMemNoError: true, + }, + }, { name: "empty name", fields: OrganizationFields{ @@ -302,18 +331,25 @@ func CreateOrganization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() - err := s.CreateOrganization(ctx, tt.args.organization) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + errCreate := s.CreateOrganization(ctx, tt.args.organization) + diffPlatformErrors(tt.name, errCreate, tt.wants.err, tt.wants.inMemNoError, isInMem, t) // Delete only newly created organizations // if tt.args.organization.ID != nil { defer s.DeleteOrganization(ctx, tt.args.organization.ID) organizations, _, err := s.FindOrganizations(ctx, influxdb.OrganizationFilter{}) - diffPlatformErrors(tt.name, err, nil, opPrefix, t) + diffPlatformErrors(tt.name, err, nil, tt.wants.inMemNoError, false, t) + + // Our test cases wants list is for the case when the error occurs + // If the error does not occur (usually because of the inmem store) + // our list of wanted organizations will be wrong. + if tt.wants.inMemNoError && errCreate == nil { + return + } if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) } @@ -322,10 +358,7 @@ func CreateOrganization( } // FindOrganizationByID testing -func FindOrganizationByID( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func FindOrganizationByID(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { id platform.ID } @@ -396,12 +429,12 @@ func FindOrganizationByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() organization, err := s.FindOrganizationByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organization is different -got/+want\ndiff %s", diff) @@ -411,10 +444,7 @@ func FindOrganizationByID( } // FindOrganizations testing -func FindOrganizations( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func FindOrganizations(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { ID platform.ID name string @@ -610,7 +640,7 @@ func FindOrganizations( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -623,7 +653,7 @@ func FindOrganizations( } organizations, _, err := s.FindOrganizations(ctx, filter, tt.args.findOptions) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) @@ -633,10 +663,7 @@ func FindOrganizations( } // DeleteOrganization testing -func DeleteOrganization( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func DeleteOrganization(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { ID platform.ID } @@ -719,15 +746,15 @@ func DeleteOrganization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteOrganization(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) filter := influxdb.OrganizationFilter{} organizations, _, err := s.FindOrganizations(ctx, filter) - diffPlatformErrors(tt.name, err, nil, opPrefix, t) + diffPlatformErrors(tt.name, err, nil, false, false, t) if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) @@ -737,10 +764,7 @@ func DeleteOrganization( } // FindOrganization testing -func FindOrganization( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func FindOrganization(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { name string id platform.ID @@ -843,7 +867,7 @@ func FindOrganization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() filter := influxdb.OrganizationFilter{} @@ -855,7 +879,7 @@ func FindOrganization( } organization, err := s.FindOrganization(ctx, filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) @@ -865,10 +889,7 @@ func FindOrganization( } // UpdateOrganization testing -func UpdateOrganization( - init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), - t *testing.T, -) { +func UpdateOrganization(init func(OrganizationFields, *testing.T) (influxdb.OrganizationService, string, func()), isInMem bool, t *testing.T) { type args struct { id platform.ID name *string @@ -1085,7 +1106,7 @@ func UpdateOrganization( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1094,7 +1115,7 @@ func UpdateOrganization( upd.Description = tt.args.description organization, err := s.UpdateOrganization(ctx, tt.args.id, upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organization is different -got/+want\ndiff %s", diff) diff --git a/testing/scraper_target.go b/testing/scraper_target.go index 20a4a837ff3..52e2c9359e4 100644 --- a/testing/scraper_target.go +++ b/testing/scraper_target.go @@ -300,11 +300,11 @@ func AddTarget( } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.AddTarget(ctx, tt.args.target, tt.args.userID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) defer s.RemoveTarget(ctx, tt.args.target.ID) targets, err := s.ListTargets(ctx, influxdb.ScraperTargetFilter{}) @@ -488,11 +488,11 @@ func ListTargets( } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() targets, err := s.ListTargets(ctx, tt.args.filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(targets, tt.wants.targets, targetCmpOptions...); diff != "" { t.Errorf("targets are different -got/+want\ndiff %s", diff) @@ -585,12 +585,12 @@ func GetTargetByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() target, err := s.GetTargetByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(target, tt.wants.target, targetCmpOptions...); diff != "" { t.Errorf("target is different -got/+want\ndiff %s", diff) @@ -691,11 +691,11 @@ func RemoveTarget(init func(TargetFields, *testing.T) (influxdb.ScraperTargetSto } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.RemoveTarget(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) targets, err := s.ListTargets(ctx, influxdb.ScraperTargetFilter{}) if err != nil { @@ -826,7 +826,7 @@ func UpdateTarget( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -836,7 +836,7 @@ func UpdateTarget( } target, err := s.UpdateTarget(ctx, upd, tt.args.userID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(target, tt.wants.target, targetCmpOptions...); diff != "" { t.Errorf("scraper target is different -got/+want\ndiff %s", diff) diff --git a/testing/session.go b/testing/session.go index 19631b6e249..2d6cf2d7101 100644 --- a/testing/session.go +++ b/testing/session.go @@ -132,11 +132,11 @@ func CreateSession( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() session, err := s.CreateSession(ctx, tt.args.user) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(session, tt.wants.session, sessionCmpOptions...); diff != "" { t.Errorf("sessions are different -got/+want\ndiff %s", diff) @@ -207,12 +207,12 @@ func FindSession( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() session, err := s.FindSession(ctx, tt.args.key) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(session, tt.wants.session, sessionCmpOptions...); diff != "" { t.Errorf("session is different -got/+want\ndiff %s", diff) @@ -270,12 +270,12 @@ func ExpireSession( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.ExpireSession(ctx, tt.args.key) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) session, err := s.FindSession(ctx, tt.args.key) if err.Error() != influxdb.ErrSessionExpired && err.Error() != influxdb.ErrSessionNotFound { @@ -413,12 +413,12 @@ func RenewSession( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.RenewSession(ctx, tt.args.session, tt.args.expireAt) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) session, err := s.FindSession(ctx, tt.args.key) if err != nil { diff --git a/testing/source.go b/testing/source.go index c81a3c13dc3..6151596afe6 100644 --- a/testing/source.go +++ b/testing/source.go @@ -91,11 +91,11 @@ func CreateSource( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateSource(ctx, tt.args.source) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) defer s.DeleteSource(ctx, tt.args.source.ID) sources, _, err := s.FindSources(ctx, platform.FindOptions{}) @@ -174,11 +174,11 @@ func FindSourceByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() source, err := s.FindSourceByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(source, tt.wants.source, sourceCmpOptions...); diff != "" { t.Errorf("sources are different -got/+want\ndiff %s", diff) @@ -250,11 +250,11 @@ func FindSources( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() sources, _, err := s.FindSources(ctx, tt.args.opts) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(sources, tt.wants.sources, sourceCmpOptions...); diff != "" { t.Errorf("sources are different -got/+want\ndiff %s", diff) @@ -339,11 +339,11 @@ func DeleteSource( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteSource(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) sources, _, err := s.FindSources(ctx, platform.FindOptions{}) if err != nil { diff --git a/testing/user_service.go b/testing/user_service.go index e684fdb5411..d7c08895b12 100644 --- a/testing/user_service.go +++ b/testing/user_service.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "sort" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -11,6 +12,7 @@ import ( "github.com/influxdata/influxdb/v2/kit/platform" "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/mock" + bolt "go.etcd.io/bbolt" ) const ( @@ -48,12 +50,11 @@ type UserFields struct { } // UserService tests all the service functions. -func UserService( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), t *testing.T, -) { +func UserService(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { tests := []struct { name string fn func(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), + isInMem bool, t *testing.T) }{ { @@ -89,22 +90,20 @@ func UserService( t.Run(tt.name, func(t *testing.T) { tt := tt t.Parallel() - tt.fn(init, t) + tt.fn(init, isInMem, t) }) } } // CreateUser testing -func CreateUser( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func CreateUser(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { user *influxdb.User } type wants struct { - err error - users []*influxdb.User + err error + inMemNoError bool + users []*influxdb.User } tests := []struct { @@ -204,16 +203,54 @@ func CreateUser( }, }, }, + { + name: "names can't be huge", + fields: UserFields{ + IDGenerator: &mock.IDGenerator{ + IDFn: func() platform.ID { + return MustIDBase16(userOneID) + }, + }, + Users: []*influxdb.User{ + { + ID: MustIDBase16(userOneID), + Name: "user1", + Status: influxdb.Active, + }, + }, + }, + args: args{ + user: &influxdb.User{ + ID: MustIDBase16(userTwoID), + Name: strings.Repeat("ATrulyEnormousUserName", 10_000), + Status: influxdb.Active, + }, + }, + wants: wants{ + users: []*influxdb.User{ + { + ID: MustIDBase16(userOneID), + Name: "user1", + Status: influxdb.Active, + }, + }, + err: &errors.Error{ + Code: errors.ETooLarge, + Op: influxdb.OpCreateUser, + Err: bolt.ErrKeyTooLarge, + }, + inMemNoError: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() - err := s.CreateUser(ctx, tt.args.user) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) - + errCreate := s.CreateUser(ctx, tt.args.user) + diffPlatformErrors(tt.name, errCreate, tt.wants.err, tt.wants.inMemNoError, isInMem, t) // Delete only created users - ie., having a not nil ID if tt.args.user.ID.Valid() { defer s.DeleteUser(ctx, tt.args.user.ID) @@ -223,6 +260,13 @@ func CreateUser( if err != nil { t.Fatalf("failed to retrieve users: %v", err) } + + // If the operation succeeded against expectations (because is the inmem store) + // our wants list will be wrong, so don't compare it. + + if isInMem && tt.wants.inMemNoError && errCreate == nil { + return + } if diff := cmp.Diff(users, tt.wants.users, userCmpOptions...); diff != "" { t.Errorf("users are different -got/+want\ndiff %s", diff) } @@ -231,10 +275,7 @@ func CreateUser( } // FindUserByID testing -func FindUserByID( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func FindUserByID(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { id platform.ID } @@ -307,12 +348,12 @@ func FindUserByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() user, err := s.FindUserByID(ctx, tt.args.id) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, isInMem, t) if diff := cmp.Diff(user, tt.wants.user, userCmpOptions...); diff != "" { t.Errorf("user is different -got/+want\ndiff %s", diff) @@ -322,10 +363,7 @@ func FindUserByID( } // FindUsers testing -func FindUsers( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func FindUsers(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { ID platform.ID name string @@ -607,7 +645,7 @@ func FindUsers( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -620,7 +658,7 @@ func FindUsers( } users, _, err := s.FindUsers(ctx, filter, tt.args.findOptions) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, isInMem, t) if diff := cmp.Diff(users, tt.wants.users, userCmpOptions...); diff != "" { t.Errorf("users are different -got/+want\ndiff %s", diff) @@ -630,10 +668,7 @@ func FindUsers( } // DeleteUser testing -func DeleteUser( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func DeleteUser(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { ID platform.ID } @@ -720,11 +755,11 @@ func DeleteUser( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteUser(ctx, tt.args.ID) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, isInMem, t) filter := influxdb.UserFilter{} users, _, err := s.FindUsers(ctx, filter) @@ -739,10 +774,7 @@ func DeleteUser( } // FindUser testing -func FindUser( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func FindUser(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { filter influxdb.UserFilter } @@ -931,11 +963,11 @@ func FindUser( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() user, err := s.FindUser(ctx, tt.args.filter) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, isInMem, t) if diff := cmp.Diff(user, tt.wants.user, userCmpOptions...); diff != "" { t.Errorf("users are different -got/+want\ndiff %s", diff) @@ -945,10 +977,7 @@ func FindUser( } // UpdateUser testing -func UpdateUser( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func UpdateUser(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { type args struct { name string id platform.ID @@ -1079,7 +1108,7 @@ func UpdateUser( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx := context.Background() @@ -1098,7 +1127,7 @@ func UpdateUser( } user, err := s.UpdateUser(ctx, tt.args.id, upd) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, isInMem, t) if diff := cmp.Diff(user, tt.wants.user, userCmpOptions...); diff != "" { t.Errorf("user is different -got/+want\ndiff %s", diff) @@ -1107,10 +1136,7 @@ func UpdateUser( } } -func UpdateUser_IndexHygiene( - init func(UserFields, *testing.T) (influxdb.UserService, string, func()), - t *testing.T, -) { +func UpdateUser_IndexHygiene(init func(UserFields, *testing.T) (influxdb.UserService, string, func()), isInMem bool, t *testing.T) { oldUserName := "user1" users := UserFields{ diff --git a/testing/util.go b/testing/util.go index b2ed351a9eb..29f3a813ab1 100644 --- a/testing/util.go +++ b/testing/util.go @@ -46,10 +46,13 @@ func NewTestInmemStore(t *testing.T) kv.SchemaStore { return s } -// TODO(goller): remove opPrefix argument -func diffPlatformErrors(name string, actual, expected error, opPrefix string, t *testing.T) { +func diffPlatformErrors(name string, actual, expected error, inMemNoError, isInMem bool, t *testing.T) { t.Helper() - ErrorsEqual(t, actual, expected) + if inMemNoError && isInMem { + ErrorsEqual(t, actual, nil) + } else { + ErrorsEqual(t, actual, expected) + } } // ErrorsEqual checks to see if the provided errors are equivalent. diff --git a/testing/variable.go b/testing/variable.go index 34325f2855f..09afaaaa44d 100644 --- a/testing/variable.go +++ b/testing/variable.go @@ -830,7 +830,7 @@ func FindVariables(init func(VariableFields, *testing.T) (influxdb.VariableServi for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, opPrefix, done := init(tt.fields, t) + s, _, done := init(tt.fields, t) defer done() ctx, cancel := context.WithTimeout(context.Background(), time.Second) @@ -842,7 +842,7 @@ func FindVariables(init func(VariableFields, *testing.T) (influxdb.VariableServi } variables, err := s.FindVariables(ctx, filter, tt.args.findOpts) - diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) + diffPlatformErrors(tt.name, err, tt.wants.err, false, false, t) if diff := cmp.Diff(variables, tt.wants.variables, variableCmpOptions...); diff != "" { t.Errorf("variables are different -got/+want\ndiff %s", diff) diff --git a/tsdb/engine/tsm1/engine_test.go b/tsdb/engine/tsm1/engine_test.go index e8fd76c5165..0da4886d9f6 100644 --- a/tsdb/engine/tsm1/engine_test.go +++ b/tsdb/engine/tsm1/engine_test.go @@ -1196,7 +1196,7 @@ func TestIndex_SeriesIDSet(t *testing.T) { } // Ensures that deleting series from TSM files with multiple fields removes all the -/// series +// / series func TestEngine_DeleteSeries(t *testing.T) { for _, index := range tsdb.RegisteredIndexes() { t.Run(index, func(t *testing.T) { diff --git a/tsdb/index_test.go b/tsdb/index_test.go index 2cd2fd26d3a..b43ab2f3561 100644 --- a/tsdb/index_test.go +++ b/tsdb/index_test.go @@ -564,7 +564,7 @@ func BenchmarkIndexSet_TagSets(b *testing.B) { // This benchmark concurrently writes series to the index and fetches cached bitsets. // The idea is to emphasize the performance difference when bitset caching is on and off. // -// Typical results for an i7 laptop +// # Typical results for an i7 laptop // // BenchmarkIndex_ConcurrentWriteQuery/tsi1/queries_100000/cache-8 1 1645048376 ns/op 2215402840 B/op 23048978 allocs/op // BenchmarkIndex_ConcurrentWriteQuery/tsi1/queries_100000/no_cache-8 1 22242155616 ns/op 28277544136 B/op 79620463 allocs/op diff --git a/v1/authorization/error.go b/v1/authorization/error.go index a9a658f2745..a4f600c138d 100644 --- a/v1/authorization/error.go +++ b/v1/authorization/error.go @@ -56,14 +56,6 @@ func ErrInvalidAuthIDError(err error) *errors.Error { } } -// ErrInternalServiceError is used when the error comes from an internal system. -func ErrInternalServiceError(err error) *errors.Error { - return &errors.Error{ - Code: errors.EInternal, - Err: err, - } -} - // UnexpectedAuthIndexError is used when the error comes from an internal system. func UnexpectedAuthIndexError(err error) *errors.Error { return &errors.Error{ diff --git a/v1/authorization/storage_authorization.go b/v1/authorization/storage_authorization.go index f0635f65e7b..130762b1100 100644 --- a/v1/authorization/storage_authorization.go +++ b/v1/authorization/storage_authorization.go @@ -3,6 +3,7 @@ package authorization import ( "context" "encoding/json" + errors2 "errors" "github.com/buger/jsonparser" "github.com/influxdata/influxdb/v2" @@ -75,7 +76,7 @@ func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.A continue } _, err := ts.GetBucket(ctx, tx, *p.Resource.ID) - if err == tenant.ErrBucketNotFound { + if errors2.Is(err, tenant.ErrBucketNotFound) { return ErrBucketNotFound } } @@ -132,7 +133,7 @@ func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform. b, err := tx.Bucket(authBucket) if err != nil { - return nil, ErrInternalServiceError(err) + return nil, errors.ErrInternalServiceError(err) } v, err := b.Get(encodedID) @@ -141,7 +142,7 @@ func (s *Store) GetAuthorizationByID(ctx context.Context, tx kv.Tx, id platform. } if err != nil { - return nil, ErrInternalServiceError(err) + return nil, errors.ErrInternalServiceError(err) } a := &influxdb.Authorization{} @@ -302,11 +303,11 @@ func (s *Store) DeleteAuthorization(ctx context.Context, tx kv.Tx, id platform.I } if err := idx.Delete([]byte(a.Token)); err != nil { - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } if err := b.Delete(encodedID); err != nil { - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } return nil @@ -354,7 +355,7 @@ func uniqueID(ctx context.Context, tx kv.Tx, id platform.ID) error { b, err := tx.Bucket(authBucket) if err != nil { - return ErrInternalServiceError(err) + return errors.ErrInternalServiceError(err) } _, err = b.Get(encodedID) diff --git a/v1/authorization/storage_authorization_test.go b/v1/authorization/storage_authorization_test.go index 4c10beb47b0..da9ff43dc31 100644 --- a/v1/authorization/storage_authorization_test.go +++ b/v1/authorization/storage_authorization_test.go @@ -2,6 +2,7 @@ package authorization import ( "context" + "errors" "fmt" "reflect" "testing" @@ -307,7 +308,7 @@ func TestAuthBucketNotExists(t *testing.T) { return err }) - if err == nil || err != ErrBucketNotFound { + if err == nil || !errors.Is(err, ErrBucketNotFound) { t.Fatalf("Authorization creating should have failed with ErrBucketNotFound [Error]: %v", err) } }