From 40428588f2b8d7c341de9c49d9c8f9088f421f27 Mon Sep 17 00:00:00 2001 From: Michael de Sa Date: Thu, 12 Oct 2017 18:53:33 -0400 Subject: [PATCH] Refactor bolt UsersStore to use ID instead of name Signed-off-by: Jared Scheib --- bolt/users.go | 57 +++++++++++++++++++--------------------------- bolt/users_test.go | 32 +++++++++++++++++--------- 2 files changed, 44 insertions(+), 45 deletions(-) diff --git a/bolt/users.go b/bolt/users.go index 5eb9b05b2f..ea92a034c8 100644 --- a/bolt/users.go +++ b/bolt/users.go @@ -2,6 +2,7 @@ package bolt import ( "context" + "strconv" "github.com/boltdb/bolt" "github.com/influxdata/chronograf" @@ -19,42 +20,34 @@ type UsersStore struct { client *Client } -// get searches the UsersStore for user with name and returns the bolt representation -func (s *UsersStore) get(ctx context.Context, name string) (*chronograf.User, error) { - found := false - var returnUser chronograf.User +// get searches the UsersStore for user with id and returns the bolt representation +func (s *UsersStore) get(ctx context.Context, id uint64) (*chronograf.User, error) { + var u chronograf.User err := s.client.db.View(func(tx *bolt.Tx) error { - err := tx.Bucket(UsersBucket).ForEach(func(k, v []byte) error { - var u chronograf.User - if err := internal.UnmarshalUser(v, &u); err != nil { - return err - } else if u.Name != name { - return nil - } - found = true - if err := internal.UnmarshalUser(v, &returnUser); err != nil { - return err - } - return nil - }) - if err != nil { - return err - } - if found == false { + v := tx.Bucket(UsersBucket).Get(u64tob(id)) + if v == nil { return chronograf.ErrUserNotFound } + if err := internal.UnmarshalUser(v, &u); err != nil { + return err + } return nil }) + if err != nil { return nil, err } - return &returnUser, nil + return &u, nil } // Get searches the UsersStore for user with name -func (s *UsersStore) Get(ctx context.Context, name string) (*chronograf.User, error) { - u, err := s.get(ctx, name) +func (s *UsersStore) Get(ctx context.Context, id string) (*chronograf.User, error) { + uid, err := strconv.ParseUint(id, 10, 64) + if err != nil { + return nil, err + } + u, err := s.get(ctx, uid) if err != nil { return nil, err } @@ -84,13 +77,13 @@ func (s *UsersStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.U } // Delete the users from the UsersStore -func (s *UsersStore) Delete(ctx context.Context, user *chronograf.User) error { - u, err := s.get(ctx, user.Name) +func (s *UsersStore) Delete(ctx context.Context, usr *chronograf.User) error { + _, err := s.get(ctx, usr.ID) if err != nil { return err } if err := s.client.db.Update(func(tx *bolt.Tx) error { - if err := tx.Bucket(UsersBucket).Delete(u64tob(u.ID)); err != nil { + if err := tx.Bucket(UsersBucket).Delete(u64tob(usr.ID)); err != nil { return err } return nil @@ -103,18 +96,14 @@ func (s *UsersStore) Delete(ctx context.Context, user *chronograf.User) error { // Update a user func (s *UsersStore) Update(ctx context.Context, usr *chronograf.User) error { - u, err := s.get(ctx, usr.Name) + _, err := s.get(ctx, usr.ID) if err != nil { return err } if err := s.client.db.Update(func(tx *bolt.Tx) error { - u.Name = usr.Name - u.Provider = usr.Provider - u.Scheme = usr.Scheme - u.Roles = usr.Roles - if v, err := internal.MarshalUser(u); err != nil { + if v, err := internal.MarshalUser(usr); err != nil { return err - } else if err := tx.Bucket(UsersBucket).Put(u64tob(u.ID), v); err != nil { + } else if err := tx.Bucket(UsersBucket).Put(u64tob(usr.ID), v); err != nil { return err } return nil diff --git a/bolt/users_test.go b/bolt/users_test.go index 6a2801e780..7bd3a7af51 100644 --- a/bolt/users_test.go +++ b/bolt/users_test.go @@ -2,6 +2,7 @@ package bolt_test import ( "context" + "fmt" "reflect" "testing" @@ -10,8 +11,8 @@ import ( func TestUsersStore_Get(t *testing.T) { type args struct { - ctx context.Context - name string + ctx context.Context + ID string } tests := []struct { name string @@ -22,8 +23,8 @@ func TestUsersStore_Get(t *testing.T) { { name: "User not found", args: args{ - ctx: context.Background(), - name: "unknown", + ctx: context.Background(), + ID: "unknown", }, wantErr: true, }, @@ -39,7 +40,7 @@ func TestUsersStore_Get(t *testing.T) { defer client.Close() s := client.UsersStore - got, err := s.Get(tt.args.ctx, tt.args.name) + got, err := s.Get(tt.args.ctx, tt.args.ID) if (err != nil) != tt.wantErr { t.Errorf("%q. UsersStore.Get() error = %v, wantErr %v", tt.name, err, tt.wantErr) continue @@ -100,7 +101,10 @@ func TestUsersStore_Add(t *testing.T) { continue } - got, _ = s.Get(tt.args.ctx, got.Name) + got, err = s.Get(tt.args.ctx, fmt.Sprintf("%d", got.ID)) + if err != nil { + t.Fatalf("failed to get user: %v", err) + } if got.Name != tt.want.Name { t.Errorf("%q. UsersStore.Add() .Name:\ngot %v, want %v", tt.name, got.Name, tt.want.Name) } @@ -141,7 +145,7 @@ func TestUsersStore_Delete(t *testing.T) { args: args{ ctx: context.Background(), user: &chronograf.User{ - Name: "noone", + ID: 10, }, }, wantErr: true, @@ -169,7 +173,7 @@ func TestUsersStore_Delete(t *testing.T) { s := client.UsersStore if tt.addFirst { - s.Add(tt.args.ctx, tt.args.user) + tt.args.user, _ = s.Add(tt.args.ctx, tt.args.user) } if err := s.Delete(tt.args.ctx, tt.args.user); (err != nil) != tt.wantErr { t.Errorf("%q. UsersStore.Delete() error = %v, wantErr %v", tt.name, err, tt.wantErr) @@ -198,7 +202,7 @@ func TestUsersStore_Update(t *testing.T) { args: args{ ctx: context.Background(), usr: &chronograf.User{ - Name: "noone", + ID: 10, }, }, wantErr: true, @@ -261,7 +265,10 @@ func TestUsersStore_Update(t *testing.T) { s := client.UsersStore if tt.addFirst { - tt.args.usr, _ = s.Add(tt.args.ctx, tt.args.usr) + tt.args.usr, err = s.Add(tt.args.ctx, tt.args.usr) + if err != nil { + t.Fatal(err) + } } if tt.args.roles != nil { @@ -285,7 +292,10 @@ func TestUsersStore_Update(t *testing.T) { continue } - got, _ := s.Get(tt.args.ctx, tt.args.usr.Name) + got, err := s.Get(tt.args.ctx, fmt.Sprintf("%d", tt.args.usr.ID)) + if err != nil { + t.Fatalf("failed to get user: %v", err) + } if got.Name != tt.want.Name { t.Errorf("%q. UsersStore.Update() .Name:\ngot %v, want %v", tt.name, got.Name, tt.want.Name) }