From 2b602bfcfe6cf78ce339212d28715f758b1b8253 Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Wed, 5 Dec 2018 09:57:26 -0500 Subject: [PATCH] fix(http): convert org errors endpoint --- bolt/organization.go | 227 +++++++++++++++++++++++--------- bolt/organization_test.go | 29 +--- cmd/influx/organization.go | 5 +- http/org_service.go | 30 ++++- http/org_test.go | 7 +- inmem/organization_service.go | 123 +++++++++++++---- inmem/organization_test.go | 28 +--- organization.go | 10 ++ testing/organization_service.go | 214 ++++++++++++++++++++---------- 9 files changed, 456 insertions(+), 217 deletions(-) diff --git a/bolt/organization.go b/bolt/organization.go index 9bc12415b7..376c2c4dd3 100644 --- a/bolt/organization.go +++ b/bolt/organization.go @@ -6,7 +6,7 @@ import ( "fmt" "time" - "github.com/coreos/bbolt" + bolt "github.com/coreos/bbolt" "github.com/influxdata/platform" platformcontext "github.com/influxdata/platform/context" ) @@ -32,11 +32,13 @@ func (c *Client) initializeOrganizations(ctx context.Context, tx *bolt.Tx) error // FindOrganizationByID retrieves a organization by id. func (c *Client) FindOrganizationByID(ctx context.Context, id platform.ID) (*platform.Organization, error) { var o *platform.Organization - err := c.db.View(func(tx *bolt.Tx) error { - org, err := c.findOrganizationByID(ctx, tx, id) - if err != nil { - return err + org, pe := c.findOrganizationByID(ctx, tx, id) + if pe != nil { + return &platform.Error{ + Op: getOp(platform.OpFindOrganizationByID), + Err: pe, + } } o = org return nil @@ -49,21 +51,28 @@ func (c *Client) FindOrganizationByID(ctx context.Context, id platform.ID) (*pla return o, nil } -func (c *Client) findOrganizationByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Organization, error) { +func (c *Client) findOrganizationByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (*platform.Organization, *platform.Error) { encodedID, err := id.Encode() if err != nil { - return nil, err + return nil, &platform.Error{ + Code: platform.EInvalid, + Err: err, + } } v := tx.Bucket(organizationBucket).Get(encodedID) if len(v) == 0 { - // TODO: Make standard error - return nil, fmt.Errorf("organization not found") + return nil, &platform.Error{ + Code: platform.ENotFound, + Msg: "organization not found", + } } var o platform.Organization if err := json.Unmarshal(v, &o); err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } return &o, nil @@ -74,9 +83,9 @@ func (c *Client) FindOrganizationByName(ctx context.Context, n string) (*platfor var o *platform.Organization err := c.db.View(func(tx *bolt.Tx) error { - org, err := c.findOrganizationByName(ctx, tx, n) - if err != nil { - return err + org, pe := c.findOrganizationByName(ctx, tx, n) + if pe != nil { + return pe } o = org return nil @@ -85,16 +94,21 @@ func (c *Client) FindOrganizationByName(ctx context.Context, n string) (*platfor return o, err } -func (c *Client) findOrganizationByName(ctx context.Context, tx *bolt.Tx, n string) (*platform.Organization, error) { +func (c *Client) findOrganizationByName(ctx context.Context, tx *bolt.Tx, n string) (*platform.Organization, *platform.Error) { o := tx.Bucket(organizationIndex).Get(organizationIndexKey(n)) if o == nil { - // TODO: Make standard error - return nil, fmt.Errorf("organization not found") + return nil, &platform.Error{ + Code: platform.ENotFound, + Msg: "organization not found", + } } var id platform.ID if err := id.Decode(o); err != nil { - return nil, err + return nil, &platform.Error{ + Code: platform.EInvalid, + Err: err, + } } return c.findOrganizationByID(ctx, tx, id) } @@ -103,12 +117,27 @@ func (c *Client) findOrganizationByName(ctx context.Context, tx *bolt.Tx, n stri // Filters using ID, or Name should be efficient. // Other filters will do a linear scan across organizations until it finds a match. func (c *Client) FindOrganization(ctx context.Context, filter platform.OrganizationFilter) (*platform.Organization, error) { + op := getOp(platform.OpFindOrganization) if filter.ID != nil { - return c.FindOrganizationByID(ctx, *filter.ID) + o, err := c.FindOrganizationByID(ctx, *filter.ID) + if err != nil { + return nil, &platform.Error{ + Err: err, + Op: op, + } + } + return o, nil } if filter.Name != nil { - return c.FindOrganizationByName(ctx, *filter.Name) + o, err := c.FindOrganizationByName(ctx, *filter.Name) + if err != nil { + return nil, &platform.Error{ + Err: err, + Op: op, + } + } + return o, nil } filterFn := filterOrganizationsFn(filter) @@ -125,11 +154,18 @@ func (c *Client) FindOrganization(ctx context.Context, filter platform.Organizat }) if err != nil { - return nil, err + return nil, &platform.Error{ + Op: op, + Err: err, + } } if o == nil { - return nil, fmt.Errorf("organization not found") + return nil, &platform.Error{ + Code: platform.ENotFound, + Op: op, + Msg: "organization not found", + } } return o, nil @@ -155,10 +191,14 @@ func filterOrganizationsFn(filter platform.OrganizationFilter) func(o *platform. // Filters using ID, or Name should be efficient. // Other filters will do a linear scan across all organizations searching for a match. func (c *Client) FindOrganizations(ctx context.Context, filter platform.OrganizationFilter, opt ...platform.FindOptions) ([]*platform.Organization, int, error) { + op := getOp(platform.OpFindOrganizations) if filter.ID != nil { o, err := c.FindOrganizationByID(ctx, *filter.ID) if err != nil { - return nil, 0, err + return nil, 0, &platform.Error{ + Err: err, + Op: op, + } } return []*platform.Organization{o}, 1, nil @@ -167,7 +207,10 @@ func (c *Client) FindOrganizations(ctx context.Context, filter platform.Organiza if filter.Name != nil { o, err := c.FindOrganizationByName(ctx, *filter.Name) if err != nil { - return nil, 0, err + return nil, 0, &platform.Error{ + Err: err, + Op: op, + } } return []*platform.Organization{o}, 1, nil @@ -185,7 +228,10 @@ func (c *Client) FindOrganizations(ctx context.Context, filter platform.Organiza }) if err != nil { - return nil, 0, err + return nil, 0, &platform.Error{ + Err: err, + Op: op, + } } return os, len(os), nil @@ -193,43 +239,71 @@ func (c *Client) FindOrganizations(ctx context.Context, filter platform.Organiza // CreateOrganization creates a platform organization and sets b.ID. func (c *Client) CreateOrganization(ctx context.Context, o *platform.Organization) error { + op := getOp(platform.OpCreateOrganization) return c.db.Update(func(tx *bolt.Tx) error { unique := c.uniqueOrganizationName(ctx, tx, o) - if !unique { - // TODO: make standard error - return fmt.Errorf("organization with name %s already exists", o.Name) + return &platform.Error{ + Code: platform.EConflict, + Op: op, + Msg: fmt.Sprintf("organization with name %s already exists", o.Name), + } } o.ID = c.IDGenerator.ID() if err := c.appendOrganizationEventToLog(ctx, tx, o.ID, organizationCreatedEvent); err != nil { - return err + return &platform.Error{ + Err: err, + Op: op, + } } - return c.putOrganization(ctx, tx, o) + if err := c.putOrganization(ctx, tx, o); err != nil { + return &platform.Error{ + Err: err, + Op: op, + } + } + return nil }) } // PutOrganization will put a organization without setting an ID. func (c *Client) PutOrganization(ctx context.Context, o *platform.Organization) error { + var err error return c.db.Update(func(tx *bolt.Tx) error { - return c.putOrganization(ctx, tx, o) + if pe := c.putOrganization(ctx, tx, o); pe != nil { + err = pe + } + return err }) } -func (c *Client) putOrganization(ctx context.Context, tx *bolt.Tx, o *platform.Organization) error { +func (c *Client) putOrganization(ctx context.Context, tx *bolt.Tx, o *platform.Organization) *platform.Error { v, err := json.Marshal(o) if err != nil { - return err + return &platform.Error{ + Err: err, + } } encodedID, err := o.ID.Encode() if err != nil { - return err + return &platform.Error{ + Code: platform.EInvalid, + Err: err, + } } if err := tx.Bucket(organizationIndex).Put(organizationIndexKey(o.Name), encodedID); err != nil { - return err + return &platform.Error{ + Err: err, + } } - return tx.Bucket(organizationBucket).Put(encodedID, v) + if err = tx.Bucket(organizationBucket).Put(encodedID, v); err != nil { + return &platform.Error{ + Err: err, + } + } + return nil } func organizationIndexKey(n string) []byte { @@ -261,9 +335,12 @@ func (c *Client) uniqueOrganizationName(ctx context.Context, tx *bolt.Tx, o *pla func (c *Client) UpdateOrganization(ctx context.Context, id platform.ID, upd platform.OrganizationUpdate) (*platform.Organization, error) { var o *platform.Organization err := c.db.Update(func(tx *bolt.Tx) error { - org, err := c.updateOrganization(ctx, tx, id, upd) - if err != nil { - return err + org, pe := c.updateOrganization(ctx, tx, id, upd) + if pe != nil { + return &platform.Error{ + Err: pe, + Op: getOp(platform.OpUpdateOrganization), + } } o = org return nil @@ -272,27 +349,31 @@ func (c *Client) UpdateOrganization(ctx context.Context, id platform.ID, upd pla return o, err } -func (c *Client) updateOrganization(ctx context.Context, tx *bolt.Tx, id platform.ID, upd platform.OrganizationUpdate) (*platform.Organization, error) { - o, err := c.findOrganizationByID(ctx, tx, id) - if err != nil { - return nil, err +func (c *Client) updateOrganization(ctx context.Context, tx *bolt.Tx, id platform.ID, upd platform.OrganizationUpdate) (*platform.Organization, *platform.Error) { + o, pe := c.findOrganizationByID(ctx, tx, id) + if pe != nil { + return nil, pe } if upd.Name != nil { // Organizations are indexed by name and so the organization index must be pruned // when name is modified. if err := tx.Bucket(organizationIndex).Delete(organizationIndexKey(o.Name)); err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } o.Name = *upd.Name } if err := c.appendOrganizationEventToLog(ctx, tx, o.ID, organizationUpdatedEvent); err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } - if err := c.putOrganization(ctx, tx, o); err != nil { - return nil, err + if pe := c.putOrganization(ctx, tx, o); pe != nil { + return nil, pe } return o, nil @@ -300,40 +381,60 @@ func (c *Client) updateOrganization(ctx context.Context, tx *bolt.Tx, id platfor // DeleteOrganization deletes a organization and prunes it from the index. func (c *Client) DeleteOrganization(ctx context.Context, id platform.ID) error { - return c.db.Update(func(tx *bolt.Tx) error { - if err := c.deleteOrganizationsBuckets(ctx, tx, id); err != nil { - return err + err := c.db.Update(func(tx *bolt.Tx) error { + if pe := c.deleteOrganizationsBuckets(ctx, tx, id); pe != nil { + return pe } - return c.deleteOrganization(ctx, tx, id) + if pe := c.deleteOrganization(ctx, tx, id); pe != nil { + return pe + } + return nil }) + if err != nil { + return &platform.Error{ + Op: getOp(platform.OpDeleteOrganization), + Err: err, + } + } + return nil } -func (c *Client) deleteOrganization(ctx context.Context, tx *bolt.Tx, id platform.ID) error { - o, err := c.findOrganizationByID(ctx, tx, id) - if err != nil { - return err +func (c *Client) deleteOrganization(ctx context.Context, tx *bolt.Tx, id platform.ID) *platform.Error { + o, pe := c.findOrganizationByID(ctx, tx, id) + if pe != nil { + return pe } if err := tx.Bucket(organizationIndex).Delete(organizationIndexKey(o.Name)); err != nil { - return err + return &platform.Error{ + Err: err, + } } encodedID, err := id.Encode() if err != nil { - return err + return &platform.Error{ + Code: platform.EInvalid, + Err: err, + } } - return tx.Bucket(organizationBucket).Delete(encodedID) + if err = tx.Bucket(organizationBucket).Delete(encodedID); err != nil { + return &platform.Error{ + Err: err, + } + } + return nil } -func (c *Client) deleteOrganizationsBuckets(ctx context.Context, tx *bolt.Tx, id platform.ID) error { +func (c *Client) deleteOrganizationsBuckets(ctx context.Context, tx *bolt.Tx, id platform.ID) *platform.Error { filter := platform.BucketFilter{ OrganizationID: &id, } - bs, err := c.findBuckets(ctx, tx, filter) - if err != nil { - return err + bs, pe := c.findBuckets(ctx, tx, filter) + if pe != nil { + return pe } for _, b := range bs { - if err := c.deleteBucket(ctx, tx, b.ID); err != nil { - return err + if pe := c.deleteBucket(ctx, tx, b.ID); pe != nil { + return pe } } return nil diff --git a/bolt/organization_test.go b/bolt/organization_test.go index 2082af4077..e8296b1d3b 100644 --- a/bolt/organization_test.go +++ b/bolt/organization_test.go @@ -5,10 +5,11 @@ import ( "testing" "github.com/influxdata/platform" + "github.com/influxdata/platform/bolt" platformtesting "github.com/influxdata/platform/testing" ) -func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, func()) { +func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, string, func()) { c, closeFn, err := NewTestClient() if err != nil { t.Fatalf("failed to create new bolt client: %v", err) @@ -20,7 +21,7 @@ func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) t.Fatalf("failed to populate organizations") } } - return c, func() { + return c, bolt.OpPrefix, func() { defer closeFn() for _, o := range f.Organizations { if err := c.DeleteOrganization(ctx, o.ID); err != nil { @@ -30,26 +31,6 @@ func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) } } -func TestOrganizationService_CreateOrganization(t *testing.T) { - platformtesting.CreateOrganization(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganizationByID(t *testing.T) { - platformtesting.FindOrganizationByID(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganizations(t *testing.T) { - platformtesting.FindOrganizations(initOrganizationService, t) -} - -func TestOrganizationService_DeleteOrganization(t *testing.T) { - platformtesting.DeleteOrganization(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganization(t *testing.T) { - platformtesting.FindOrganization(initOrganizationService, t) -} - -func TestOrganizationService_UpdateOrganization(t *testing.T) { - platformtesting.UpdateOrganization(initOrganizationService, t) +func TestOrganizationService(t *testing.T) { + platformtesting.OrganizationService(initOrganizationService, t) } diff --git a/cmd/influx/organization.go b/cmd/influx/organization.go index 820dab0def..49bbae7c96 100644 --- a/cmd/influx/organization.go +++ b/cmd/influx/organization.go @@ -60,8 +60,9 @@ func newOrganizationService(f Flags) (platform.OrganizationService, error) { return c, nil } return &http.OrganizationService{ - Addr: flags.host, - Token: flags.token, + Addr: flags.host, + Token: flags.token, + OpPrefix: bolt.OpPrefix, }, nil } diff --git a/http/org_service.go b/http/org_service.go index 51082aa65b..a7840fb85e 100644 --- a/http/org_service.go +++ b/http/org_service.go @@ -486,23 +486,39 @@ type OrganizationService struct { Addr string Token string InsecureSkipVerify bool + // OpPrefix is for not found errors. + OpPrefix string } // FindOrganizationByID gets a single organization with a given id using HTTP. func (s *OrganizationService) FindOrganizationByID(ctx context.Context, id platform.ID) (*platform.Organization, error) { filter := platform.OrganizationFilter{ID: &id} - return s.FindOrganization(ctx, filter) + o, err := s.FindOrganization(ctx, filter) + if err != nil { + return nil, &platform.Error{ + Err: err, + Op: s.OpPrefix + platform.OpFindOrganizationByID, + } + } + return o, nil } // FindOrganization gets a single organization matching the filter using HTTP. func (s *OrganizationService) FindOrganization(ctx context.Context, filter platform.OrganizationFilter) (*platform.Organization, error) { os, n, err := s.FindOrganizations(ctx, filter) if err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + Op: s.OpPrefix + platform.OpFindOrganization, + } } if n == 0 { - return nil, ErrNotFound + return nil, &platform.Error{ + Code: platform.ENotFound, + Op: s.OpPrefix + platform.OpFindOrganization, + Msg: "organization not found", + } } return os[0], nil @@ -537,7 +553,7 @@ func (s *OrganizationService) FindOrganizations(ctx context.Context, filter plat return nil, 0, err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return nil, 0, err } @@ -583,7 +599,7 @@ func (s *OrganizationService) CreateOrganization(ctx context.Context, o *platfor } // TODO(jsternberg): Should this check for a 201 explicitly? - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return err } @@ -621,7 +637,7 @@ func (s *OrganizationService) UpdateOrganization(ctx context.Context, id platfor return nil, err } - if err := CheckError(resp); err != nil { + if err := CheckError(resp, true); err != nil { return nil, err } @@ -653,7 +669,7 @@ func (s *OrganizationService) DeleteOrganization(ctx context.Context, id platfor return err } - return CheckErrorStatus(http.StatusNoContent, resp) + return CheckErrorStatus(http.StatusNoContent, resp, true) } func organizationIDPath(id platform.ID) string { diff --git a/http/org_test.go b/http/org_test.go index 2ec29af285..185999ba99 100644 --- a/http/org_test.go +++ b/http/org_test.go @@ -16,7 +16,7 @@ import ( platformtesting "github.com/influxdata/platform/testing" ) -func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, func()) { +func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, string, func()) { t.Helper() svc := inmem.NewService() svc.IDGenerator = f.IDGenerator @@ -34,11 +34,12 @@ func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) handler.BucketService = svc server := httptest.NewServer(handler) client := OrganizationService{ - Addr: server.URL, + Addr: server.URL, + OpPrefix: inmem.OpPrefix, } done := server.Close - return &client, done + return &client, inmem.OpPrefix, done } func TestOrganizationService(t *testing.T) { t.Parallel() diff --git a/inmem/organization_service.go b/inmem/organization_service.go index bad5c227e4..d35c0fb3bf 100644 --- a/inmem/organization_service.go +++ b/inmem/organization_service.go @@ -7,19 +7,25 @@ import ( "github.com/influxdata/platform" ) -var ( - errOrganizationNotFound = fmt.Errorf("organization not found") +const ( + errOrganizationNotFound = "organization not found" ) -func (s *Service) loadOrganization(id platform.ID) (*platform.Organization, error) { +func (s *Service) loadOrganization(id platform.ID) (*platform.Organization, *platform.Error) { i, ok := s.organizationKV.Load(id.String()) if !ok { - return nil, errOrganizationNotFound + return nil, &platform.Error{ + Code: platform.ENotFound, + Msg: errOrganizationNotFound, + } } b, ok := i.(*platform.Organization) if !ok { - return nil, fmt.Errorf("type %T is not a organization", i) + return nil, &platform.Error{ + Code: platform.EInternal, + Msg: fmt.Sprintf("type %T is not a organization", i), + } } return b, nil } @@ -39,7 +45,7 @@ func (s *Service) forEachOrganization(ctx context.Context, fn func(b *platform.O return err } -func (s *Service) filterOrganizations(ctx context.Context, fn func(b *platform.Organization) bool) ([]*platform.Organization, error) { +func (s *Service) filterOrganizations(ctx context.Context, fn func(b *platform.Organization) bool) ([]*platform.Organization, *platform.Error) { orgs := []*platform.Organization{} err := s.forEachOrganization(ctx, func(o *platform.Organization) bool { if fn(o) { @@ -49,7 +55,9 @@ func (s *Service) filterOrganizations(ctx context.Context, fn func(b *platform.O }) if err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + } } return orgs, nil @@ -57,35 +65,66 @@ func (s *Service) filterOrganizations(ctx context.Context, fn func(b *platform.O // FindOrganizationByID returns a single organization by ID. func (s *Service) FindOrganizationByID(ctx context.Context, id platform.ID) (*platform.Organization, error) { - return s.loadOrganization(id) + o, pe := s.loadOrganization(id) + if pe != nil { + return nil, &platform.Error{ + Op: OpPrefix + platform.OpFindOrganizationByID, + Err: pe, + } + } + return o, nil } // FindOrganization returns the first organization that matches a filter. func (s *Service) FindOrganization(ctx context.Context, filter platform.OrganizationFilter) (*platform.Organization, error) { + op := OpPrefix + platform.OpFindOrganization if filter.ID == nil && filter.Name == nil { - return nil, fmt.Errorf("no filter parameters provided") + return nil, &platform.Error{ + Code: platform.EInvalid, + Op: op, + Msg: "no filter parameters provided", + } } if filter.ID != nil { - return s.FindOrganizationByID(ctx, *filter.ID) + o, err := s.FindOrganizationByID(ctx, *filter.ID) + if err != nil { + return nil, &platform.Error{ + Op: op, + Err: err, + } + } + return o, nil } orgs, n, err := s.FindOrganizations(ctx, filter) if err != nil { - return nil, err + return nil, &platform.Error{ + Op: op, + Err: err, + } } if n < 1 { - return nil, fmt.Errorf("organization not found") + return nil, &platform.Error{ + Code: platform.ENotFound, + Op: op, + Msg: errOrganizationNotFound, + } } return orgs[0], nil } +// FindOrganizations returns a list of organizations that match filter and the total count of matching organizations. func (s *Service) FindOrganizations(ctx context.Context, filter platform.OrganizationFilter, opt ...platform.FindOptions) ([]*platform.Organization, int, error) { + op := OpPrefix + platform.OpFindOrganizations if filter.ID != nil { o, err := s.FindOrganizationByID(ctx, *filter.ID) if err != nil { - return nil, 0, err + return nil, 0, &platform.Error{ + Op: op, + Err: err, + } } return []*platform.Organization{o}, 1, nil @@ -98,35 +137,71 @@ func (s *Service) FindOrganizations(ctx context.Context, filter platform.Organiz } } - orgs, err := s.filterOrganizations(ctx, filterFunc) - if err != nil { - return nil, 0, err + orgs, pe := s.filterOrganizations(ctx, filterFunc) + if pe != nil { + return nil, 0, &platform.Error{ + Err: pe, + Op: op, + } + } + + if len(orgs) == 0 { + return orgs, 0, &platform.Error{ + Code: platform.ENotFound, + Op: op, + Msg: OpPrefix + platform.OpFindOrganizations, + } } return orgs, len(orgs), nil } -func (c *Service) findOrganizationByName(ctx context.Context, n string) (*platform.Organization, error) { - return c.FindOrganization(ctx, platform.OrganizationFilter{Name: &n}) +func (s *Service) findOrganizationByName(ctx context.Context, n string) (*platform.Organization, *platform.Error) { + o, err := s.FindOrganization(ctx, platform.OrganizationFilter{Name: &n}) + if err != nil { + return nil, &platform.Error{ + Err: err, + } + } + return o, nil } +// CreateOrganization creates a new organization and sets b.ID with the new identifier. func (s *Service) CreateOrganization(ctx context.Context, o *platform.Organization) error { + op := OpPrefix + platform.OpCreateOrganization if _, err := s.FindOrganization(ctx, platform.OrganizationFilter{Name: &o.Name}); err == nil { - return fmt.Errorf("organization with name %s already exists", o.Name) + return &platform.Error{ + Code: platform.EConflict, + Op: op, + Msg: fmt.Sprintf("organization with name %s already exists", o.Name), + } + } o.ID = s.IDGenerator.ID() - return s.PutOrganization(ctx, o) + err := s.PutOrganization(ctx, o) + if err != nil { + return &platform.Error{ + Op: op, + Err: err, + } + } + return nil } +// PutOrganization will put a organization without setting an ID. func (s *Service) PutOrganization(ctx context.Context, o *platform.Organization) error { s.organizationKV.Store(o.ID.String(), o) return nil } +// UpdateOrganization updates a organization according the parameters set on upd. func (s *Service) UpdateOrganization(ctx context.Context, id platform.ID, upd platform.OrganizationUpdate) (*platform.Organization, error) { o, err := s.FindOrganizationByID(ctx, id) if err != nil { - return nil, err + return nil, &platform.Error{ + Err: err, + Op: OpPrefix + platform.OpUpdateOrganization, + } } if upd.Name != nil { @@ -138,9 +213,13 @@ func (s *Service) UpdateOrganization(ctx context.Context, id platform.ID, upd pl return o, nil } +// DeleteOrganization deletes a organization and prunes it from the index. func (s *Service) DeleteOrganization(ctx context.Context, id platform.ID) error { if _, err := s.FindOrganizationByID(ctx, id); err != nil { - return err + return &platform.Error{ + Err: err, + Op: OpPrefix + platform.OpDeleteOrganization, + } } s.organizationKV.Delete(id.String()) return nil diff --git a/inmem/organization_test.go b/inmem/organization_test.go index 0838664be7..f927fb497b 100644 --- a/inmem/organization_test.go +++ b/inmem/organization_test.go @@ -8,7 +8,7 @@ import ( platformtesting "github.com/influxdata/platform/testing" ) -func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, func()) { +func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) (platform.OrganizationService, string, func()) { s := NewService() s.IDGenerator = f.IDGenerator ctx := context.TODO() @@ -17,29 +17,9 @@ func initOrganizationService(f platformtesting.OrganizationFields, t *testing.T) t.Fatalf("failed to populate organizations") } } - return s, func() {} + return s, OpPrefix, func() {} } -func TestOrganizationService_CreateOrganization(t *testing.T) { - platformtesting.CreateOrganization(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganizationByID(t *testing.T) { - platformtesting.FindOrganizationByID(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganizations(t *testing.T) { - platformtesting.FindOrganizations(initOrganizationService, t) -} - -func TestOrganizationService_DeleteOrganization(t *testing.T) { - platformtesting.DeleteOrganization(initOrganizationService, t) -} - -func TestOrganizationService_FindOrganization(t *testing.T) { - platformtesting.FindOrganization(initOrganizationService, t) -} - -func TestOrganizationService_UpdateOrganization(t *testing.T) { - platformtesting.UpdateOrganization(initOrganizationService, t) +func TestOrganizationService(t *testing.T) { + platformtesting.OrganizationService(initOrganizationService, t) } diff --git a/organization.go b/organization.go index 0c469223d6..6e69b0eacc 100644 --- a/organization.go +++ b/organization.go @@ -8,6 +8,16 @@ type Organization struct { Name string `json:"name"` } +// ops for orgs error and orgs op logs. +const ( + OpFindOrganizationByID = "FindOrganizationByID" + OpFindOrganization = "FindOrganization" + OpFindOrganizations = "FindOrganizations" + OpCreateOrganization = "CreateOrganization" + OpUpdateOrganization = "UpdateOrganization" + OpDeleteOrganization = "DeleteOrganization" +) + // OrganizationService represents a service for managing organization data. type OrganizationService interface { // Returns a single organization by ID. diff --git a/testing/organization_service.go b/testing/organization_service.go index e2e8901a97..cb84dd2843 100644 --- a/testing/organization_service.go +++ b/testing/organization_service.go @@ -3,7 +3,6 @@ package testing import ( "bytes" "context" - "fmt" "sort" "testing" @@ -38,11 +37,11 @@ type OrganizationFields struct { // OrganizationService tests all the service functions. func OrganizationService( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), t *testing.T, + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { tests := []struct { name string - fn func(init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + fn func(init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T) }{ { @@ -79,7 +78,7 @@ func OrganizationService( // CreateOrganization testing func CreateOrganization( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -171,7 +170,11 @@ func CreateOrganization( Name: "organization1", }, }, - err: fmt.Errorf("organization with name organization1 already exists"), + err: &platform.Error{ + Code: platform.EConflict, + Op: platform.OpCreateOrganization, + Msg: "organization with name organization1 already exists", + }, }, }, { @@ -207,19 +210,11 @@ func CreateOrganization( 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() err := s.CreateOrganization(ctx, tt.args.organization) - 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) // Delete only newly created organizations // if tt.args.organization.ID != nil { @@ -227,9 +222,7 @@ func CreateOrganization( // } organizations, _, err := s.FindOrganizations(ctx, platform.OrganizationFilter{}) - if err != nil { - t.Fatalf("failed to retrieve organizations: %v", err) - } + diffPlatformErrors(tt.name, err, nil, opPrefix, t) if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) } @@ -239,7 +232,7 @@ func CreateOrganization( // FindOrganizationByID testing func FindOrganizationByID( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -280,24 +273,42 @@ func FindOrganizationByID( }, }, }, + { + name: "didn't find organization by id", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "organization2", + }, + }, + }, + args: args{ + id: MustIDBase16(threeID), + }, + wants: wants{ + organization: nil, + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpFindOrganizationByID, + Msg: "", + }, + }, + }, } 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() organization, err := s.FindOrganizationByID(ctx, tt.args.id) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected errors to be equal '%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 '%v' got '%v'", tt.wants.err, err) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organization is different -got/+want\ndiff %s", diff) @@ -308,7 +319,7 @@ func FindOrganizationByID( // FindOrganizations testing func FindOrganizations( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -406,11 +417,63 @@ func FindOrganizations( }, }, }, + { + name: "find organization by id not exists", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "abc", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "xyz", + }, + }, + }, + args: args{ + ID: MustIDBase16(threeID), + }, + wants: wants{ + organizations: []*platform.Organization{}, + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpFindOrganizations, + Msg: "organization not found", + }, + }, + }, + { + name: "find organization by name not exists", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "abc", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "xyz", + }, + }, + }, + args: args{ + name: "na", + }, + wants: wants{ + organizations: []*platform.Organization{}, + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpFindOrganizations, + Msg: "organization not found", + }, + }, + }, } 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() @@ -423,15 +486,7 @@ func FindOrganizations( } organizations, _, err := s.FindOrganizations(ctx, filter) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected errors to be equal '%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 '%v' got '%v'", tt.wants.err, err) - } - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) @@ -442,7 +497,7 @@ func FindOrganizations( // DeleteOrganization testing func DeleteOrganization( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -503,7 +558,11 @@ func DeleteOrganization( ID: "1234567890654321", }, wants: wants{ - err: fmt.Errorf("organization not found"), + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpDeleteOrganization, + Msg: "organization not found", + }, organizations: []*platform.Organization{ { Name: "orgA", @@ -520,25 +579,16 @@ func DeleteOrganization( 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() err := s.DeleteOrganization(ctx, MustIDBase16(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) filter := platform.OrganizationFilter{} organizations, _, err := s.FindOrganizations(ctx, filter) - if err != nil { - t.Fatalf("failed to retrieve organizations: %v", err) - } + diffPlatformErrors(tt.name, err, nil, opPrefix, t) + if diff := cmp.Diff(organizations, tt.wants.organizations, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) } @@ -548,7 +598,7 @@ func DeleteOrganization( // FindOrganization testing func FindOrganization( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -599,14 +649,18 @@ func FindOrganization( name: "abc", }, wants: wants{ - err: fmt.Errorf("organization not found"), + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpFindOrganization, + Msg: "organization not found", + }, }, }, } 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() filter := platform.OrganizationFilter{} @@ -615,9 +669,7 @@ func FindOrganization( } organization, err := s.FindOrganization(ctx, filter) - if (err != nil) != (tt.wants.err != nil) { - t.Fatalf("expected error '%v' got '%v'", tt.wants.err, err) - } + diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t) if diff := cmp.Diff(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organizations are different -got/+want\ndiff %s", diff) @@ -628,7 +680,7 @@ func FindOrganization( // UpdateOrganization testing func UpdateOrganization( - init func(OrganizationFields, *testing.T) (platform.OrganizationService, func()), + init func(OrganizationFields, *testing.T) (platform.OrganizationService, string, func()), t *testing.T, ) { type args struct { @@ -646,6 +698,32 @@ func UpdateOrganization( args args wants wants }{ + { + name: "update id not exists", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "organization2", + }, + }, + }, + args: args{ + id: MustIDBase16(threeID), + name: "changed", + }, + wants: wants{ + err: &platform.Error{ + Code: platform.ENotFound, + Op: platform.OpUpdateOrganization, + Msg: "organization not found", + }, + }, + }, { name: "update name", fields: OrganizationFields{ @@ -675,7 +753,7 @@ func UpdateOrganization( 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() @@ -685,15 +763,7 @@ func UpdateOrganization( } organization, err := s.UpdateOrganization(ctx, tt.args.id, upd) - 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(organization, tt.wants.organization, organizationCmpOptions...); diff != "" { t.Errorf("organization is different -got/+want\ndiff %s", diff)