diff --git a/bolt/organizations.go b/bolt/organizations.go index e62aa01883..e8bfefdc5e 100644 --- a/bolt/organizations.go +++ b/bolt/organizations.go @@ -43,8 +43,21 @@ func (s *OrganizationsStore) Migrate(ctx context.Context) error { }) } +func (s *OrganizationsStore) nameIsUnique(ctx context.Context, name string) bool { + _, err := s.Get(ctx, chronograf.OrganizationQuery{Name: &name}) + switch err { + case chronograf.ErrOrganizationNotFound: + return true + default: + return false + } +} + // Add creates a new Organization in the OrganizationsStore func (s *OrganizationsStore) Add(ctx context.Context, o *chronograf.Organization) (*chronograf.Organization, error) { + if !s.nameIsUnique(ctx, o.Name) { + return nil, chronograf.ErrOrganizationNameTaken + } if err := s.client.db.Update(func(tx *bolt.Tx) error { b := tx.Bucket(OrganizationsBucket) seq, err := b.NextSequence() @@ -228,6 +241,9 @@ func (s *OrganizationsStore) Update(ctx context.Context, o *chronograf.Organizat if err != nil { return err } + if o.Name != org.Name && !s.nameIsUnique(ctx, o.Name) { + return chronograf.ErrOrganizationNameTaken + } return s.client.db.Update(func(tx *bolt.Tx) error { org.Name = o.Name if v, err := internal.MarshalOrganization(org); err != nil { diff --git a/bolt/organizations_test.go b/bolt/organizations_test.go index e518131e30..e73a4b09a6 100644 --- a/bolt/organizations_test.go +++ b/bolt/organizations_test.go @@ -231,6 +231,9 @@ func TestOrganizationsStore_All(t *testing.T) { } func TestOrganizationsStore_Update(t *testing.T) { + type fields struct { + orgs []chronograf.Organization + } type args struct { ctx context.Context org *chronograf.Organization @@ -238,13 +241,15 @@ func TestOrganizationsStore_Update(t *testing.T) { } tests := []struct { name string + fields fields args args addFirst bool want *chronograf.Organization wantErr bool }{ { - name: "No such organization", + name: "No such organization", + fields: fields{}, args: args{ ctx: context.Background(), org: &chronograf.Organization{ @@ -255,7 +260,8 @@ func TestOrganizationsStore_Update(t *testing.T) { wantErr: true, }, { - name: "Update organization name", + name: "Update organization name", + fields: fields{}, args: args{ ctx: context.Background(), org: &chronograf.Organization{ @@ -268,6 +274,25 @@ func TestOrganizationsStore_Update(t *testing.T) { }, addFirst: true, }, + { + name: "Update organization name - name already taken", + fields: fields{ + orgs: []chronograf.Organization{ + { + Name: "The Bad Place", + }, + }, + }, + args: args{ + ctx: context.Background(), + org: &chronograf.Organization{ + Name: "The Good Place", + }, + name: "The Bad Place", + }, + wantErr: true, + addFirst: true, + }, } for _, tt := range tests { client, err := NewTestClient() @@ -280,13 +305,17 @@ func TestOrganizationsStore_Update(t *testing.T) { defer client.Close() s := client.OrganizationsStore - if tt.addFirst { - tt.args.org, err = s.Add(tt.args.ctx, tt.args.org) + for _, org := range tt.fields.orgs { + _, err = s.Add(tt.args.ctx, &org) if err != nil { t.Fatal(err) } } + if tt.addFirst { + tt.args.org, err = s.Add(tt.args.ctx, tt.args.org) + } + if tt.args.name != "" { tt.args.org.Name = tt.args.name } @@ -361,3 +390,75 @@ func TestOrganizationStore_Delete(t *testing.T) { } } } + +func TestOrganizationsStore_Add(t *testing.T) { + type fields struct { + orgs []chronograf.Organization + } + type args struct { + ctx context.Context + org *chronograf.Organization + } + tests := []struct { + name string + fields fields + args args + want *chronograf.Organization + wantErr bool + }{ + { + name: "Add organization - name already taken", + fields: fields{ + orgs: []chronograf.Organization{ + { + Name: "The Good Place", + }, + }, + }, + args: args{ + ctx: context.Background(), + org: &chronograf.Organization{ + Name: "The Good Place", + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + client, err := NewTestClient() + if err != nil { + t.Fatal(err) + } + if err := client.Open(context.TODO()); err != nil { + t.Fatal(err) + } + defer client.Close() + s := client.OrganizationsStore + + for _, org := range tt.fields.orgs { + _, err = s.Add(tt.args.ctx, &org) + if err != nil { + t.Fatal(err) + } + } + + _, err = s.Add(tt.args.ctx, tt.args.org) + + if (err != nil) != tt.wantErr { + t.Errorf("%q. OrganizationsStore.Update() error = %v, wantErr %v", tt.name, err, tt.wantErr) + } + + // for the empty test + if tt.want == nil { + continue + } + + got, err := s.Get(tt.args.ctx, chronograf.OrganizationQuery{Name: &tt.args.org.Name}) + if err != nil { + t.Fatalf("failed to get organization: %v", err) + } + if diff := cmp.Diff(got, tt.want, orgCmpOptions...); diff != "" { + t.Errorf("%q. OrganizationsStore.Update():\n-got/+want\ndiff %s", tt.name, diff) + } + } +} diff --git a/chronograf.go b/chronograf.go index 0491f1472f..ce8d326893 100644 --- a/chronograf.go +++ b/chronograf.go @@ -20,18 +20,19 @@ import ( // General errors. const ( - ErrUpstreamTimeout = Error("request to backend timed out") - ErrSourceNotFound = Error("source not found") - ErrServerNotFound = Error("server not found") - ErrLayoutNotFound = Error("layout not found") - ErrDashboardNotFound = Error("dashboard not found") - ErrUserNotFound = Error("user not found") - ErrOrganizationNotFound = Error("organization not found") - ErrLayoutInvalid = Error("layout is invalid") - ErrAlertNotFound = Error("alert not found") - ErrAuthentication = Error("user not authenticated") - ErrUninitialized = Error("client uninitialized. Call Open() method") - ErrInvalidAxis = Error("Unexpected axis in cell. Valid axes are 'x', 'y', and 'y2'") + ErrUpstreamTimeout = Error("request to backend timed out") + ErrSourceNotFound = Error("source not found") + ErrServerNotFound = Error("server not found") + ErrLayoutNotFound = Error("layout not found") + ErrDashboardNotFound = Error("dashboard not found") + ErrUserNotFound = Error("user not found") + ErrOrganizationNotFound = Error("organization not found") + ErrLayoutInvalid = Error("layout is invalid") + ErrAlertNotFound = Error("alert not found") + ErrAuthentication = Error("user not authenticated") + ErrUninitialized = Error("client uninitialized. Call Open() method") + ErrInvalidAxis = Error("Unexpected axis in cell. Valid axes are 'x', 'y', and 'y2'") + ErrOrganizationNameTaken = Error("organization name is taken") ) // Error is a domain error encountered while processing chronograf requests @@ -786,6 +787,11 @@ type OrganizationQuery struct { } // OrganizationsStore is the storage and retrieval of Organizations +// +// While not necessary for the app to function correctly, it is +// expected that Implementors of the OrganizationsStore will take +// care to guarantee that the Organization.Name is unqiue. Allowing +// for duplicate names creates a confusing UX experience for the User. type OrganizationsStore interface { // Add creates a new Organization. // The Created organization is returned back to the user with the