diff --git a/bolt/source.go b/bolt/source.go index c0dcdf8c6a..a14e9c3f67 100644 --- a/bolt/source.go +++ b/bolt/source.go @@ -5,7 +5,7 @@ import ( "encoding/json" "fmt" - "github.com/coreos/bbolt" + bolt "github.com/coreos/bbolt" "github.com/influxdata/platform" ) @@ -42,12 +42,12 @@ func (c *Client) initializeSources(ctx context.Context, tx *bolt.Tx) error { return err } - _, err := c.findSourceByID(ctx, tx, DefaultSource.ID) - if err != nil && err != platform.ErrSourceNotFound { - return err + _, pe := c.findSourceByID(ctx, tx, DefaultSource.ID) + if pe != nil && platform.ErrorCode(pe) != platform.ENotFound { + return pe } - if err == platform.ErrSourceNotFound { + if platform.ErrorCode(pe) == platform.ENotFound { if err := c.putSource(ctx, tx, &DefaultSource); err != nil { return err } @@ -72,11 +72,17 @@ func (c *Client) DefaultSource(ctx context.Context) (*platform.Source, error) { return nil } } - return fmt.Errorf("no default source found") + return &platform.Error{ + Code: platform.ENotFound, + Msg: "no default source found", + } }) if err != nil { - return nil, err + return nil, &platform.Error{ + Op: getOp(platform.OpDefaultSource), + Err: err, + } } return s, nil @@ -87,35 +93,40 @@ func (c *Client) FindSourceByID(ctx context.Context, id platform.ID) (*platform. var s *platform.Source err := c.db.View(func(tx *bolt.Tx) error { - src, err := c.findSourceByID(ctx, tx, id) - if err != nil { - return err + src, pe := c.findSourceByID(ctx, tx, id) + if pe != nil { + return &platform.Error{ + Err: pe, + Op: getOp(platform.OpFindSourceByID), + } } s = src return nil }) - - if err != nil { - return nil, err - } - - return s, nil + return s, err } -func (c *Client) findSourceByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Source, error) { +func (c *Client) findSourceByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Source, *platform.Error) { encodedID, err := id.Encode() if err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } v := tx.Bucket(sourceBucket).Get(encodedID) if len(v) == 0 { - return nil, platform.ErrSourceNotFound + return nil, &platform.Error{ + Code: platform.ENotFound, + Msg: platform.ErrSourceNotFound, + } } var s platform.Source if err := json.Unmarshal(v, &s); err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } return &s, nil @@ -136,7 +147,10 @@ func (c *Client) FindSources(ctx context.Context, opt platform.FindOptions) ([]* }) if err != nil { - return nil, 0, err + return nil, 0, &platform.Error{ + Op: platform.OpFindSources, + Err: err, + } } return ss, len(ss), nil @@ -159,7 +173,7 @@ func (c *Client) findSources(ctx context.Context, tx *bolt.Tx, opt platform.Find // CreateSource creates a platform source and sets s.ID. func (c *Client) CreateSource(ctx context.Context, s *platform.Source) error { - return c.db.Update(func(tx *bolt.Tx) error { + err := c.db.Update(func(tx *bolt.Tx) error { s.ID = c.IDGenerator.ID() // Generating an organization id if it missing or invalid @@ -169,6 +183,13 @@ func (c *Client) CreateSource(ctx context.Context, s *platform.Source) error { return c.putSource(ctx, tx, s) }) + if err != nil { + return &platform.Error{ + Err: err, + Op: getOp(platform.OpCreateSource), + } + } + return nil } // PutSource will put a source without setting an ID. @@ -218,7 +239,10 @@ func (c *Client) UpdateSource(ctx context.Context, id platform.ID, upd platform. err := c.db.Update(func(tx *bolt.Tx) error { src, err := c.updateSource(ctx, tx, id, upd) if err != nil { - return err + return &platform.Error{ + Err: err, + Op: getOp(platform.OpUpdateSource), + } } s = src return nil @@ -228,9 +252,9 @@ func (c *Client) UpdateSource(ctx context.Context, id platform.ID, upd platform. } func (c *Client) updateSource(ctx context.Context, tx *bolt.Tx, id platform.ID, upd platform.SourceUpdate) (*platform.Source, error) { - s, err := c.findSourceByID(ctx, tx, id) - if err != nil { - return nil, err + s, pe := c.findSourceByID(ctx, tx, id) + if pe != nil { + return nil, pe } if err := upd.Apply(s); err != nil { @@ -247,23 +271,40 @@ func (c *Client) updateSource(ctx context.Context, tx *bolt.Tx, id platform.ID, // DeleteSource deletes a source and prunes it from the index. func (c *Client) DeleteSource(ctx context.Context, id platform.ID) error { return c.db.Update(func(tx *bolt.Tx) error { - return c.deleteSource(ctx, tx, id) + pe := c.deleteSource(ctx, tx, id) + if pe != nil { + return &platform.Error{ + Err: pe, + Op: getOp(platform.OpDeleteSource), + } + } + return nil }) } -func (c *Client) deleteSource(ctx context.Context, tx *bolt.Tx, id platform.ID) error { +func (c *Client) deleteSource(ctx context.Context, tx *bolt.Tx, id platform.ID) *platform.Error { if id == DefaultSource.ID { - return fmt.Errorf("cannot delete autogen source") + return &platform.Error{ + Code: platform.EForbidden, + Msg: "cannot delete autogen source", + } } - _, err := c.findSourceByID(ctx, tx, id) - if err != nil { - return err + _, pe := c.findSourceByID(ctx, tx, id) + if pe != nil { + return pe } encodedID, err := id.Encode() if err != nil { - return err + return &platform.Error{ + Err: err, + } } - return tx.Bucket(sourceBucket).Delete(encodedID) + if err = tx.Bucket(sourceBucket).Delete(encodedID); err != nil { + return &platform.Error{ + Err: err, + } + } + return nil } diff --git a/bolt/source_test.go b/bolt/source_test.go index 8de69c3fcb..4c74d2967b 100644 --- a/bolt/source_test.go +++ b/bolt/source_test.go @@ -5,10 +5,11 @@ import ( "testing" "github.com/influxdata/platform" + "github.com/influxdata/platform/bolt" platformtesting "github.com/influxdata/platform/testing" ) -func initSourceService(f platformtesting.SourceFields, t *testing.T) (platform.SourceService, func()) { +func initSourceService(f platformtesting.SourceFields, t *testing.T) (platform.SourceService, string, func()) { c, closeFn, err := NewTestClient() if err != nil { t.Fatalf("failed to create new bolt client: %v", err) @@ -20,7 +21,7 @@ func initSourceService(f platformtesting.SourceFields, t *testing.T) (platform.S t.Fatalf("failed to populate buckets") } } - return c, func() { + return c, bolt.OpPrefix, func() { defer closeFn() for _, b := range f.Sources { if err := c.DeleteSource(ctx, b.ID); err != nil { diff --git a/http/source_service.go b/http/source_service.go index fb1e8354d5..bf6e6f3af4 100644 --- a/http/source_service.go +++ b/http/source_service.go @@ -14,7 +14,6 @@ import ( "github.com/influxdata/flux/csv" "github.com/influxdata/flux/lang" "github.com/influxdata/platform" - kerrors "github.com/influxdata/platform/kit/errors" "github.com/influxdata/platform/query" "github.com/influxdata/platform/query/influxql" "github.com/julienschmidt/httprouter" @@ -342,7 +341,10 @@ func decodeGetSourceRequest(ctx context.Context, r *http.Request) (*getSourceReq params := httprouter.ParamsFromContext(ctx) id := params.ByName("id") if id == "" { - return nil, kerrors.InvalidDataf("url missing id") + return nil, &platform.Error{ + Code: platform.EInvalid, + Msg: "url missing id", + } } var i platform.ID @@ -382,7 +384,10 @@ func decodeDeleteSourceRequest(ctx context.Context, r *http.Request) (*deleteSou params := httprouter.ParamsFromContext(ctx) id := params.ByName("id") if id == "" { - return nil, kerrors.InvalidDataf("url missing id") + return nil, &platform.Error{ + Code: platform.EInvalid, + Msg: "url missing id", + } } var i platform.ID @@ -460,7 +465,10 @@ func decodePatchSourceRequest(ctx context.Context, r *http.Request) (*patchSourc params := httprouter.ParamsFromContext(ctx) id := params.ByName("id") if id == "" { - return nil, kerrors.InvalidDataf("url missing id") + return nil, &platform.Error{ + Code: platform.EInvalid, + Msg: "url missing id", + } } var i platform.ID diff --git a/source.go b/source.go index caac708d64..f1ce13dc95 100644 --- a/source.go +++ b/source.go @@ -3,8 +3,8 @@ package platform import "context" const ( - // ErrSourceNotFound is a chronograf error when a source does not exist. - ErrSourceNotFound = ChronografError("source not found") + // ErrSourceNotFound is an error message when a source does not exist. + ErrSourceNotFound = "source not found" ) // SourceType is a string for types of sources. @@ -49,6 +49,16 @@ type SourceFields struct { Token string `json:"token"` // Token is the 2.0 authorization token associated with a source } +// ops for sources. +const ( + OpDefaultSource = "DefaultSource" + OpFindSourceByID = "FindSourceByID" + OpFindSources = "FindSources" + OpCreateSource = "CreateSource" + OpUpdateSource = "UpdateSource" + OpDeleteSource = "DeleteSource" +) + // SourceService is a service for managing sources. type SourceService interface { // DefaultSource retrieves the default source. diff --git a/testing/source.go b/testing/source.go index 85cde9d852..08cb653318 100644 --- a/testing/source.go +++ b/testing/source.go @@ -3,7 +3,6 @@ package testing import ( "bytes" "context" - "fmt" "sort" "testing" @@ -41,7 +40,7 @@ type SourceFields struct { // CreateSource testing func CreateSource( - init func(SourceFields, *testing.T) (platform.SourceService, func()), + init func(SourceFields, *testing.T) (platform.SourceService, string, func()), t *testing.T, ) { type args struct { @@ -90,19 +89,11 @@ func CreateSource( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.CreateSource(ctx, tt.args.source) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) - } - - if err != nil && tt.wants.err != nil { - if err.Error() != tt.wants.err.Error() { - t.Fatalf("expected error messages to match '%v' got '%v'", tt.wants.err, err.Error()) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) defer s.DeleteSource(ctx, tt.args.source.ID) sources, _, err := s.FindSources(ctx, platform.FindOptions{}) @@ -118,7 +109,7 @@ func CreateSource( // FindSourceByID testing func FindSourceByID( - init func(SourceFields, *testing.T) (platform.SourceService, func()), + init func(SourceFields, *testing.T) (platform.SourceService, string, func()), t *testing.T, ) { type args struct { @@ -181,19 +172,11 @@ func FindSourceByID( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.Background() source, err := s.FindSourceByID(ctx, tt.args.id) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) - } - - if err != nil && tt.wants.err != nil { - if err.Error() != tt.wants.err.Error() { - t.Fatalf("expected error messages to match '%v' got '%v'", tt.wants.err, err.Error()) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(source, tt.wants.source, sourceCmpOptions...); diff != "" { t.Errorf("sources are different -got/+want\ndiff %s", diff) @@ -204,7 +187,7 @@ func FindSourceByID( // FindSources testing func FindSources( - init func(SourceFields, *testing.T) (platform.SourceService, func()), + init func(SourceFields, *testing.T) (platform.SourceService, string, func()), t *testing.T, ) { type args struct { @@ -265,19 +248,11 @@ func FindSources( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.Background() sources, _, err := s.FindSources(ctx, tt.args.opts) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) - } - - if err != nil && tt.wants.err != nil { - if err.Error() != tt.wants.err.Error() { - t.Fatalf("expected error messages to match '%v' got '%v'", tt.wants.err, err.Error()) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(sources, tt.wants.sources, sourceCmpOptions...); diff != "" { t.Errorf("sources are different -got/+want\ndiff %s", diff) @@ -288,7 +263,7 @@ func FindSources( // DeleteSource testing func DeleteSource( - init func(SourceFields, *testing.T) (platform.SourceService, func()), + init func(SourceFields, *testing.T) (platform.SourceService, string, func()), t *testing.T, ) { type args struct { @@ -342,7 +317,11 @@ func DeleteSource( id: MustIDBase16(defaultSourceID), }, wants: wants{ - err: fmt.Errorf("cannot delete autogen source"), + err: &platform.Error{ + Code: platform.EForbidden, + Op: platform.OpDeleteSource, + Msg: "cannot delete autogen source", + }, sources: []*platform.Source{ { Name: "autogen", @@ -358,19 +337,11 @@ func DeleteSource( for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.Background() err := s.DeleteSource(ctx, tt.args.id) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) - } - - if err != nil && tt.wants.err != nil { - if err.Error() != tt.wants.err.Error() { - t.Fatalf("expected error messages to match '%v' got '%v'", tt.wants.err, err.Error()) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) sources, _, err := s.FindSources(ctx, platform.FindOptions{}) if err != nil {