diff --git a/bolt/organization.go b/bolt/organization.go index 3cc246b6ce..3183223093 100644 --- a/bolt/organization.go +++ b/bolt/organization.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "time" bolt "github.com/coreos/bbolt" @@ -224,12 +225,10 @@ func (c *Client) FindOrganizations(ctx context.Context, filter influxdb.Organiza func (c *Client) CreateOrganization(ctx context.Context, o *influxdb.Organization) error { op := getOp(influxdb.OpCreateOrganization) return c.db.Update(func(tx *bolt.Tx) error { - unique := c.uniqueOrganizationName(ctx, tx, o) - if !unique { + if err := c.validOrganizationName(ctx, tx, o); err != nil { return &influxdb.Error{ - Code: influxdb.EConflict, - Op: op, - Msg: fmt.Sprintf("organization with name %s already exists", o.Name), + Op: op, + Err: err, } } @@ -309,9 +308,18 @@ func forEachOrganization(ctx context.Context, tx *bolt.Tx, fn func(*influxdb.Org return nil } -func (c *Client) uniqueOrganizationName(ctx context.Context, tx *bolt.Tx, o *influxdb.Organization) bool { +func (c *Client) validOrganizationName(ctx context.Context, tx *bolt.Tx, o *influxdb.Organization) error { + if o.Name = strings.TrimSpace(o.Name); o.Name == "" { + return influxdb.ErrOrgNameisEmpty + } v := tx.Bucket(organizationIndex).Get(organizationIndexKey(o.Name)) - return len(v) == 0 + if len(v) != 0 { + return &influxdb.Error{ + Code: influxdb.EConflict, + Msg: fmt.Sprintf("organization with name %s already exists", o.Name), + } + } + return nil } // UpdateOrganization updates a organization according the parameters set on upd. @@ -347,6 +355,11 @@ func (c *Client) updateOrganization(ctx context.Context, tx *bolt.Tx, id influxd } } o.Name = *upd.Name + if err := c.validOrganizationName(ctx, tx, o); err != nil { + return nil, &influxdb.Error{ + Err: err, + } + } } if err := c.appendOrganizationEventToLog(ctx, tx, o.ID, organizationUpdatedEvent); err != nil { diff --git a/http/org_service.go b/http/org_service.go index 9615590715..99901fef45 100644 --- a/http/org_service.go +++ b/http/org_service.go @@ -681,12 +681,6 @@ func (s *OrganizationService) FindOrganizations(ctx context.Context, filter infl // CreateOrganization creates an organization. func (s *OrganizationService) CreateOrganization(ctx context.Context, o *influxdb.Organization) error { - if o.Name == "" { - return &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "organization name is required", - } - } span, _ := tracing.StartSpanFromContext(ctx) defer span.Finish() diff --git a/inmem/organization_service.go b/inmem/organization_service.go index d22c7917e1..ecaccb89db 100644 --- a/inmem/organization_service.go +++ b/inmem/organization_service.go @@ -3,6 +3,7 @@ package inmem import ( "context" "fmt" + "strings" platform "github.com/influxdata/influxdb" ) @@ -179,13 +180,15 @@ func (s *Service) findOrganizationByName(ctx context.Context, n string) (*platfo // 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 o.Name = strings.TrimSpace(o.Name); o.Name == "" { + return platform.ErrOrgNameisEmpty + } if _, err := s.FindOrganization(ctx, platform.OrganizationFilter{Name: &o.Name}); err == nil { return &platform.Error{ Code: platform.EConflict, Op: op, Msg: fmt.Sprintf("organization with name %s already exists", o.Name), } - } o.ID = s.IDGenerator.ID() err := s.PutOrganization(ctx, o) @@ -215,6 +218,15 @@ func (s *Service) UpdateOrganization(ctx context.Context, id platform.ID, upd pl } if upd.Name != nil { + if *upd.Name = strings.TrimSpace(*upd.Name); *upd.Name == "" { + return nil, platform.ErrOrgNameisEmpty + } + if _, err := s.FindOrganization(ctx, platform.OrganizationFilter{Name: upd.Name}); err == nil { + return nil, &platform.Error{ + Code: platform.EConflict, + Msg: fmt.Sprintf("organization with name %s already exists", *upd.Name), + } + } o.Name = *upd.Name } diff --git a/kv/org.go b/kv/org.go index a6a7bf5a51..ff115379eb 100644 --- a/kv/org.go +++ b/kv/org.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "time" "go.uber.org/zap" @@ -246,7 +247,7 @@ func (s *Service) addOrgOwner(ctx context.Context, tx Tx, orgID influxdb.ID) err } func (s *Service) createOrganization(ctx context.Context, tx Tx, o *influxdb.Organization) error { - if err := s.uniqueOrganizationName(ctx, tx, o); err != nil { + if err := s.validOrganizationName(ctx, tx, o); err != nil { return err } @@ -344,7 +345,10 @@ func forEachOrganization(ctx context.Context, tx Tx, fn func(*influxdb.Organizat return nil } -func (s *Service) uniqueOrganizationName(ctx context.Context, tx Tx, o *influxdb.Organization) error { +func (s *Service) validOrganizationName(ctx context.Context, tx Tx, o *influxdb.Organization) error { + if o.Name = strings.TrimSpace(o.Name); o.Name == "" { + return influxdb.ErrOrgNameisEmpty + } key := organizationIndexKey(o.Name) // if the name is not unique across all organizations, then, do not @@ -392,6 +396,9 @@ func (s *Service) updateOrganization(ctx context.Context, tx Tx, id influxdb.ID, } } o.Name = *upd.Name + if err := s.validOrganizationName(ctx, tx, o); err != nil { + return nil, err + } } if err := s.appendOrganizationEventToLog(ctx, tx, o.ID, organizationUpdatedEvent); err != nil { diff --git a/organization.go b/organization.go index 6f7899c5e3..2436596de5 100644 --- a/organization.go +++ b/organization.go @@ -9,6 +9,15 @@ type Organization struct { Description string `json:"description"` } +// errors of org +var ( + // ErrOrgNameisEmpty is error when org name is empty + ErrOrgNameisEmpty = &Error{ + Code: EInvalid, + Msg: "org name is empty", + } +) + // ops for orgs error and orgs op logs. const ( OpFindOrganizationByID = "FindOrganizationByID" diff --git a/storage/bucket_service_test.go b/storage/bucket_service_test.go index 88e5fdd8ac..ee4692a36e 100644 --- a/storage/bucket_service_test.go +++ b/storage/bucket_service_test.go @@ -28,7 +28,7 @@ func TestBucketService(t *testing.T) { t.Fatal("expected error, got nil") } - org := &platform.Organization{} + org := &platform.Organization{Name: "org1"} if err := inmemService.CreateOrganization(context.TODO(), org); err != nil { panic(err) } diff --git a/testing/organization_service.go b/testing/organization_service.go index 9a446966cc..6f2926c3a0 100644 --- a/testing/organization_service.go +++ b/testing/organization_service.go @@ -148,6 +148,59 @@ func CreateOrganization( }, }, }, + { + name: "empty name", + fields: OrganizationFields{ + IDGenerator: mock.NewIDGenerator(orgTwoID, t), + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + }, + }, + args: args{ + organization: &platform.Organization{ + ID: MustIDBase16(orgOneID), + }, + }, + wants: wants{ + organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + }, + err: platform.ErrOrgNameisEmpty, + }, + }, + { + name: "name only have spaces", + fields: OrganizationFields{ + IDGenerator: mock.NewIDGenerator(orgTwoID, t), + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + }, + }, + args: args{ + organization: &platform.Organization{ + ID: MustIDBase16(orgOneID), + Name: " ", + }, + }, + wants: wants{ + organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + }, + err: platform.ErrOrgNameisEmpty, + }, + }, { name: "names should be unique", fields: OrganizationFields{ @@ -734,7 +787,7 @@ func UpdateOrganization( t *testing.T, ) { type args struct { - name string + name *string id platform.ID } type wants struct { @@ -764,7 +817,7 @@ func UpdateOrganization( }, args: args{ id: MustIDBase16(threeID), - name: "changed", + name: strPtr("changed"), }, wants: wants{ err: &platform.Error{ @@ -790,7 +843,7 @@ func UpdateOrganization( }, args: args{ id: MustIDBase16(orgOneID), - name: "changed", + name: strPtr("changed"), }, wants: wants{ organization: &platform.Organization{ @@ -799,6 +852,76 @@ func UpdateOrganization( }, }, }, + { + name: "update name not unique", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "organization2", + }, + }, + }, + args: args{ + id: MustIDBase16(orgOneID), + name: strPtr("organization2"), + }, + wants: wants{ + err: &platform.Error{ + Code: platform.EConflict, + Op: platform.OpUpdateOrganization, + Msg: "organization with name organization2 already exists", + }, + }, + }, + { + name: "update name is empty", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "organization2", + }, + }, + }, + args: args{ + id: MustIDBase16(orgOneID), + name: strPtr(""), + }, + wants: wants{ + err: platform.ErrOrgNameisEmpty, + }, + }, + { + name: "update name only has space", + fields: OrganizationFields{ + Organizations: []*platform.Organization{ + { + ID: MustIDBase16(orgOneID), + Name: "organization1", + }, + { + ID: MustIDBase16(orgTwoID), + Name: "organization2", + }, + }, + }, + args: args{ + id: MustIDBase16(orgOneID), + name: strPtr(" "), + }, + wants: wants{ + err: platform.ErrOrgNameisEmpty, + }, + }, } for _, tt := range tests { @@ -808,9 +931,7 @@ func UpdateOrganization( ctx := context.Background() upd := platform.OrganizationUpdate{} - if tt.args.name != "" { - upd.Name = &tt.args.name - } + upd.Name = tt.args.name organization, err := s.UpdateOrganization(ctx, tt.args.id, upd) diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t)