Merge pull request #13677 from influxdata/org_name_not_empty

fix(kv): fix empty org name
pull/13659/head
Jade McGough 2019-04-26 15:57:19 -07:00 committed by GitHub
commit e02c859992
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 179 additions and 23 deletions

View File

@ -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 {

View File

@ -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()

View File

@ -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
}

View File

@ -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 {

View File

@ -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"

View File

@ -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)
}

View File

@ -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)