From 8522c93063e976f7203b1aa4e51d0f83029929cb Mon Sep 17 00:00:00 2001 From: zhulongcheng Date: Thu, 13 Dec 2018 22:57:00 +0800 Subject: [PATCH] fix(http): convert macro errors --- bolt/macro.go | 130 +++++++++++++++++++++++++++++++----------- bolt/macro_test.go | 21 ++----- http/macro_service.go | 12 ++-- http/macro_test.go | 4 +- inmem/macro.go | 55 +++++++++++++++--- inmem/macro_test.go | 16 ++++-- macro.go | 10 ++++ testing/macro.go | 48 ++++++++++------ testing/util.go | 14 ----- 9 files changed, 205 insertions(+), 105 deletions(-) diff --git a/bolt/macro.go b/bolt/macro.go index b9b9f4aae5..85d0eafd9b 100644 --- a/bolt/macro.go +++ b/bolt/macro.go @@ -6,7 +6,6 @@ import ( bolt "github.com/coreos/bbolt" "github.com/influxdata/platform" - kerrors "github.com/influxdata/platform/kit/errors" ) var ( @@ -22,8 +21,8 @@ func (c *Client) initializeMacros(ctx context.Context, tx *bolt.Tx) error { // FindMacros returns all macros in the store func (c *Client) FindMacros(ctx context.Context) ([]*platform.Macro, error) { + op := getOp(platform.OpFindMacros) macros := []*platform.Macro{} - err := c.db.View(func(tx *bolt.Tx) error { b := tx.Bucket(macroBucket) c := b.Cursor() @@ -33,7 +32,10 @@ func (c *Client) FindMacros(ctx context.Context) ([]*platform.Macro, error) { err := json.Unmarshal(v, ¯o) if err != nil { - return err + return &platform.Error{ + Err: err, + Op: op, + } } macros = append(macros, ¯o) @@ -50,11 +52,15 @@ func (c *Client) FindMacros(ctx context.Context) ([]*platform.Macro, error) { // FindMacroByID finds a single macro in the store by its ID func (c *Client) FindMacroByID(ctx context.Context, id platform.ID) (*platform.Macro, error) { + op := getOp(platform.OpFindMacroByID) var macro *platform.Macro err := c.db.View(func(tx *bolt.Tx) error { - m, err := c.findMacroByID(ctx, tx, id) - if err != nil { - return err + m, pe := c.findMacroByID(ctx, tx, id) + if pe != nil { + return &platform.Error{ + Op: op, + Err: pe, + } } macro = m return nil @@ -66,21 +72,29 @@ func (c *Client) FindMacroByID(ctx context.Context, id platform.ID) (*platform.M return macro, nil } -func (c *Client) findMacroByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Macro, error) { +func (c *Client) findMacroByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Macro, *platform.Error) { encID, err := id.Encode() if err != nil { - return nil, err + return nil, &platform.Error{ + Code: platform.EInvalid, + Err: err, + } } d := tx.Bucket(macroBucket).Get(encID) if d == nil { - return nil, kerrors.Errorf(kerrors.NotFound, "macro with ID %v not found", id) + return nil, &platform.Error{ + Code: platform.ENotFound, + Msg: "macro not found", + } } macro := &platform.Macro{} err = json.Unmarshal(d, ¯o) if err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } return macro, nil @@ -88,74 +102,122 @@ func (c *Client) findMacroByID(ctx context.Context, tx *bolt.Tx, id platform.ID) // CreateMacro creates a new macro and assigns it an ID func (c *Client) CreateMacro(ctx context.Context, macro *platform.Macro) error { + op := getOp(platform.OpCreateMacro) return c.db.Update(func(tx *bolt.Tx) error { macro.ID = c.IDGenerator.ID() + if pe := c.putMacro(ctx, tx, macro); pe != nil { + return &platform.Error{ + Op: op, + Err: pe, + } - return c.putMacro(ctx, tx, macro) + } + return nil }) } // ReplaceMacro puts a macro in the store func (c *Client) ReplaceMacro(ctx context.Context, macro *platform.Macro) error { + op := getOp(platform.OpReplaceMacro) return c.db.Update(func(tx *bolt.Tx) error { - return c.putMacro(ctx, tx, macro) + if pe := c.putMacro(ctx, tx, macro); pe != nil { + return &platform.Error{ + Op: op, + Err: pe, + } + } + return nil }) } -func (c *Client) putMacro(ctx context.Context, tx *bolt.Tx, macro *platform.Macro) error { +func (c *Client) putMacro(ctx context.Context, tx *bolt.Tx, macro *platform.Macro) *platform.Error { j, err := json.Marshal(macro) if err != nil { - return err - } - encID, err := macro.ID.Encode() - if err != nil { - return err + return &platform.Error{ + Err: err, + } } - return tx.Bucket(macroBucket).Put(encID, j) + encID, err := macro.ID.Encode() + if err != nil { + return &platform.Error{ + Code: platform.EInvalid, + Err: err, + } + } + + if err := tx.Bucket(macroBucket).Put(encID, j); err != nil { + return &platform.Error{ + Err: err, + } + } + + return nil } // UpdateMacro updates a single macro in the store with a changeset func (c *Client) UpdateMacro(ctx context.Context, id platform.ID, update *platform.MacroUpdate) (*platform.Macro, error) { + op := getOp(platform.OpUpdateMacro) var macro *platform.Macro - err := c.db.Update(func(tx *bolt.Tx) error { - m, err := c.findMacroByID(ctx, tx, id) - if err != nil { - return err + m, pe := c.findMacroByID(ctx, tx, id) + if pe != nil { + return &platform.Error{ + Op: op, + Err: pe, + } } - if err = update.Apply(m); err != nil { - return err + if err := update.Apply(m); err != nil { + return &platform.Error{ + Op: op, + Err: err, + } } macro = m - - return c.putMacro(ctx, tx, macro) + if pe = c.putMacro(ctx, tx, macro); pe != nil { + return &platform.Error{ + Op: op, + Err: pe, + } + } + return nil }) - if err != nil { - return nil, err - } - return macro, nil + return macro, err } // DeleteMacro removes a single macro from the store by its ID func (c *Client) DeleteMacro(ctx context.Context, id platform.ID) error { + op := getOp(platform.OpDeleteMacro) return c.db.Update(func(tx *bolt.Tx) error { b := tx.Bucket(macroBucket) encID, err := id.Encode() if err != nil { - return err + return &platform.Error{ + Code: platform.EInvalid, + Op: op, + Err: err, + } } d := b.Get(encID) if d == nil { - return kerrors.Errorf(kerrors.NotFound, "macro with ID %v not found", id) + return &platform.Error{ + Code: platform.ENotFound, + Op: op, + Msg: "macro not found", + } } - b.Delete(encID) + if err := b.Delete(encID); err != nil { + return &platform.Error{ + Op: op, + Err: err, + } + } return nil }) diff --git a/bolt/macro_test.go b/bolt/macro_test.go index ee007746ca..0fcecb7e24 100644 --- a/bolt/macro_test.go +++ b/bolt/macro_test.go @@ -2,13 +2,14 @@ package bolt_test import ( "context" + "github.com/influxdata/platform/bolt" "testing" "github.com/influxdata/platform" platformtesting "github.com/influxdata/platform/testing" ) -func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, func()) { +func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, string, func()) { c, closeFn, err := NewTestClient() if err != nil { t.Fatalf("failed to create new bolt test client: %v", err) @@ -33,21 +34,9 @@ func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.Mac } } - return c, done + return c, bolt.OpPrefix, done } -func TestMacroService_CreateMacro(t *testing.T) { - platformtesting.CreateMacro(initMacroService, t) -} - -func TestMacroService_FindMacroByID(t *testing.T) { - platformtesting.FindMacroByID(initMacroService, t) -} - -func TestMacroService_UpdateMacro(t *testing.T) { - platformtesting.UpdateMacro(initMacroService, t) -} - -func TestMacroService_DeleteMacro(t *testing.T) { - platformtesting.DeleteMacro(initMacroService, t) +func TestMacroService(t *testing.T) { + platformtesting.MacroService(initMacroService, t) } diff --git a/http/macro_service.go b/http/macro_service.go index 376919fbe7..7d469c7984 100644 --- a/http/macro_service.go +++ b/http/macro_service.go @@ -346,7 +346,7 @@ func (s *MacroService) FindMacroByID(ctx context.Context, id platform.ID) (*plat return nil, err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return nil, err } @@ -379,7 +379,7 @@ func (s *MacroService) FindMacros(ctx context.Context) ([]*platform.Macro, error return nil, err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return nil, err } @@ -423,7 +423,7 @@ func (s *MacroService) CreateMacro(ctx context.Context, m *platform.Macro) error return err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return err } @@ -457,7 +457,7 @@ func (s *MacroService) UpdateMacro(ctx context.Context, id platform.ID, update * return nil, err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return nil, err } @@ -497,7 +497,7 @@ func (s *MacroService) ReplaceMacro(ctx context.Context, macro *platform.Macro) return err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return err } @@ -527,7 +527,7 @@ func (s *MacroService) DeleteMacro(ctx context.Context, id platform.ID) error { if err != nil { return err } - return CheckError(resp) + return CheckError(resp, true) } func macroIDPath(id platform.ID) string { diff --git a/http/macro_test.go b/http/macro_test.go index 0f8e626920..ee9c6f1bc7 100644 --- a/http/macro_test.go +++ b/http/macro_test.go @@ -515,7 +515,7 @@ func TestMacroService_handleDeleteMacro(t *testing.T) { } } -func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, func()) { +func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, string, func()) { t.Helper() svc := inmem.NewService() svc.IDGenerator = f.IDGenerator @@ -535,7 +535,7 @@ func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.Mac } done := server.Close - return &client, done + return &client, inmem.OpPrefix, done } func TestMacroService(t *testing.T) { diff --git a/inmem/macro.go b/inmem/macro.go index 3ea23a3ef5..3b13577bf7 100644 --- a/inmem/macro.go +++ b/inmem/macro.go @@ -5,19 +5,26 @@ import ( "fmt" "github.com/influxdata/platform" - kerrors "github.com/influxdata/platform/kit/errors" ) // FindMacroByID implements the platform.MacroService interface func (s *Service) FindMacroByID(ctx context.Context, id platform.ID) (*platform.Macro, error) { + op := OpPrefix + platform.OpFindMacroByID i, ok := s.macroKV.Load(id.String()) if !ok { - return nil, kerrors.Errorf(kerrors.NotFound, "macro with ID %v not found", id) + return nil, &platform.Error{ + Op: op, + Code: platform.ENotFound, + Msg: "macro not found", + } } macro, ok := i.(*platform.Macro) if !ok { - return nil, fmt.Errorf("type %T is not a macro", i) + return nil, &platform.Error{ + Op: op, + Msg: fmt.Sprintf("type %T is not a macro", i), + } } return macro, nil @@ -25,12 +32,16 @@ func (s *Service) FindMacroByID(ctx context.Context, id platform.ID) (*platform. // FindMacros implements the platform.MacroService interface func (s *Service) FindMacros(ctx context.Context) ([]*platform.Macro, error) { + op := OpPrefix + platform.OpFindMacros var err error var macros []*platform.Macro s.macroKV.Range(func(k, v interface{}) bool { macro, ok := v.(*platform.Macro) if !ok { - err = fmt.Errorf("type %T is not a macro", v) + err = &platform.Error{ + Op: op, + Msg: fmt.Sprintf("type %T is not a macro", v), + } return false } @@ -47,23 +58,43 @@ func (s *Service) FindMacros(ctx context.Context) ([]*platform.Macro, error) { // CreateMacro implements the platform.MacroService interface func (s *Service) CreateMacro(ctx context.Context, m *platform.Macro) error { + op := OpPrefix + platform.OpCreateMacro m.ID = s.IDGenerator.ID() - return s.ReplaceMacro(ctx, m) + err := s.ReplaceMacro(ctx, m) + if err != nil { + return &platform.Error{ + Op: op, + Err: err, + } + } + return nil } // UpdateMacro implements the platform.MacroService interface func (s *Service) UpdateMacro(ctx context.Context, id platform.ID, update *platform.MacroUpdate) (*platform.Macro, error) { + op := OpPrefix + platform.OpUpdateMacro macro, err := s.FindMacroByID(ctx, id) if err != nil { - return nil, err + return nil, &platform.Error{ + Op: op, + Code: platform.ENotFound, + Msg: "macro not found", + Err: err, + } } if err := update.Apply(macro); err != nil { - return nil, err + return nil, &platform.Error{ + Op: op, + Err: err, + } } if err := s.ReplaceMacro(ctx, macro); err != nil { - return nil, err + return nil, &platform.Error{ + Op: op, + Err: err, + } } return macro, nil @@ -71,9 +102,15 @@ func (s *Service) UpdateMacro(ctx context.Context, id platform.ID, update *platf // DeleteMacro implements the platform.MacroService interface func (s *Service) DeleteMacro(ctx context.Context, id platform.ID) error { + op := OpPrefix + platform.OpDeleteMacro _, err := s.FindMacroByID(ctx, id) if err != nil { - return err + return &platform.Error{ + Op: op, + Code: platform.ENotFound, + Msg: "macro not found", + Err: err, + } } s.macroKV.Delete(id.String()) diff --git a/inmem/macro_test.go b/inmem/macro_test.go index 0cc2ec6a20..c586cec54c 100644 --- a/inmem/macro_test.go +++ b/inmem/macro_test.go @@ -8,7 +8,7 @@ import ( platformtesting "github.com/influxdata/platform/testing" ) -func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, func()) { +func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.MacroService, string, func()) { s := NewService() s.IDGenerator = f.IDGenerator @@ -19,12 +19,18 @@ func initMacroService(f platformtesting.MacroFields, t *testing.T) (platform.Mac } } - done := func() {} - return s, done + done := func() { + for _, macro := range f.Macros { + if err := s.DeleteMacro(ctx, macro.ID); err != nil { + t.Fatalf("failed to clean up macros bolt test: %v", err) + } + } + } + return s, OpPrefix, done } -func TestMacroService_CreateMacro(t *testing.T) { - platformtesting.CreateMacro(initMacroService, t) +func TestMacroService(t *testing.T) { + platformtesting.MacroService(initMacroService, t) } func TestMacroService_FindMacroByID(t *testing.T) { diff --git a/macro.go b/macro.go index 10843b5bd6..535bfb8789 100644 --- a/macro.go +++ b/macro.go @@ -6,6 +6,16 @@ import ( "fmt" ) +// ops for macro error. +const ( + OpFindMacroByID = "FindMacroByID" + OpFindMacros = "FindMacros" + OpCreateMacro = "CreateMacro" + OpUpdateMacro = "UpdateMacro" + OpReplaceMacro = "ReplaceMacro" + OpDeleteMacro = "DeleteMacro" +) + // MacroService describes a service for managing Macros type MacroService interface { // FindMacro finds a single macro from the store by its ID diff --git a/testing/macro.go b/testing/macro.go index 0811d5f284..6d2d913a31 100644 --- a/testing/macro.go +++ b/testing/macro.go @@ -3,13 +3,11 @@ package testing import ( "bytes" "context" - "fmt" "sort" "testing" "github.com/google/go-cmp/cmp" "github.com/influxdata/platform" - kerrors "github.com/influxdata/platform/kit/errors" "github.com/influxdata/platform/mock" ) @@ -39,11 +37,11 @@ type MacroFields struct { // MacroService tests all the service functions. func MacroService( - init func(MacroFields, *testing.T) (platform.MacroService, func()), t *testing.T, + init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T, ) { tests := []struct { name string - fn func(init func(MacroFields, *testing.T) (platform.MacroService, func()), + fn func(init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T) }{ { @@ -71,7 +69,7 @@ func MacroService( } // CreateMacro tests platform.MacroService CreateMacro interface method -func CreateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func()), t *testing.T) { +func CreateMacro(init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T) { type args struct { macro *platform.Macro } @@ -144,12 +142,12 @@ func CreateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func } for _, tt := range tests { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.TODO() err := s.CreateMacro(ctx, tt.args.macro) - diffErrors(err, tt.wants.err, t) + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) macros, err := s.FindMacros(ctx) if err != nil { @@ -162,7 +160,7 @@ func CreateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func } // FindMacroByID tests platform.MacroService FindMacroByID interface method -func FindMacroByID(init func(MacroFields, *testing.T) (platform.MacroService, func()), t *testing.T) { +func FindMacroByID(init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T) { type args struct { id platform.ID } @@ -223,19 +221,23 @@ func FindMacroByID(init func(MacroFields, *testing.T) (platform.MacroService, fu id: MustIDBase16(idA), }, wants: wants{ - err: kerrors.Errorf(kerrors.NotFound, "macro with ID %s not found", idA), + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpFindMacroByID, + Msg: "macro not found", + }, macro: nil, }, }, } for _, tt := range tests { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.TODO() macro, err := s.FindMacroByID(ctx, tt.args.id) - diffErrors(err, tt.wants.err, t) + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(macro, tt.wants.macro); diff != "" { t.Fatalf("found unexpected macro -got/+want\ndiff %s", diff) @@ -244,7 +246,7 @@ func FindMacroByID(init func(MacroFields, *testing.T) (platform.MacroService, fu } // UpdateMacro tests platform.MacroService UpdateMacro interface method -func UpdateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func()), t *testing.T) { +func UpdateMacro(init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T) { type args struct { id platform.ID update *platform.MacroUpdate @@ -322,7 +324,11 @@ func UpdateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func }, }, wants: wants{ - err: fmt.Errorf("macro with ID %s not found", idA), + err: &platform.Error{ + Op: platform.OpUpdateMacro, + Msg: "macro not found", + Code: platform.ENotFound, + }, macros: []*platform.Macro{}, }, }, @@ -330,12 +336,12 @@ func UpdateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func 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.TODO() macro, err := s.UpdateMacro(ctx, tt.args.id, tt.args.update) - diffErrors(err, tt.wants.err, t) + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if macro != nil { if tt.args.update.Name != "" && macro.Name != tt.args.update.Name { @@ -355,7 +361,7 @@ func UpdateMacro(init func(MacroFields, *testing.T) (platform.MacroService, func } // DeleteMacro tests platform.MacroService DeleteMacro interface method -func DeleteMacro(init func(MacroFields, *testing.T) (platform.MacroService, func()), t *testing.T) { +func DeleteMacro(init func(MacroFields, *testing.T) (platform.MacroService, string, func()), t *testing.T) { type args struct { id platform.ID } @@ -410,7 +416,11 @@ func DeleteMacro(init func(MacroFields, *testing.T) (platform.MacroService, func id: MustIDBase16(idB), }, wants: wants{ - err: kerrors.Errorf(kerrors.NotFound, "macro with ID %s not found", idB), + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpDeleteMacro, + Msg: "macro not found", + }, macros: []*platform.Macro{ { ID: MustIDBase16(idA), @@ -426,7 +436,7 @@ func DeleteMacro(init func(MacroFields, *testing.T) (platform.MacroService, func } for _, tt := range tests { - s, done := init(tt.fields, t) + s, opPrefix, done := init(tt.fields, t) defer done() ctx := context.TODO() @@ -434,7 +444,7 @@ func DeleteMacro(init func(MacroFields, *testing.T) (platform.MacroService, func defer s.ReplaceMacro(ctx, &platform.Macro{ ID: tt.args.id, }) - diffErrors(err, tt.wants.err, t) + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) macros, err := s.FindMacros(ctx) if err != nil { diff --git a/testing/util.go b/testing/util.go index 01a1dfce9e..46eb07d41c 100644 --- a/testing/util.go +++ b/testing/util.go @@ -6,20 +6,6 @@ import ( "github.com/influxdata/platform" ) -func diffErrors(actual, expected error, t *testing.T) { - if expected == nil && actual != nil { - t.Fatalf("unexpected error %q", actual.Error()) - } - - if expected != nil && actual == nil { - t.Fatalf("expected error %q but received nil", expected.Error()) - } - - if expected != nil && actual != nil && expected.Error() != actual.Error() { - t.Fatalf("expected error %q but received error %q", expected.Error(), actual.Error()) - } -} - func diffPlatformErrors(name string, actual, expected error, opPrefix string, t *testing.T) { if expected == nil && actual == nil { return