Generalize chronograf.UsersStore Get method

The `Get` method on the UsersStore was generalize by changing the second
parameter to a struct. This allows the Store to retrieve users by more
than simply their name.

-Get(ctx context.Context, name string) (*User, error)
+Get(ctx context.Context, q UserQuery) (*User, error)
pull/10616/head
Michael Desa 2017-10-18 14:17:42 -04:00
parent 0517a87954
commit 246e65e598
13 changed files with 120 additions and 60 deletions

View File

@ -2,7 +2,7 @@ package bolt
import (
"context"
"strconv"
"fmt"
"github.com/boltdb/bolt"
"github.com/influxdata/chronograf"
@ -38,17 +38,48 @@ func (s *UsersStore) get(ctx context.Context, id uint64) (*chronograf.User, erro
return &u, nil
}
func (s *UsersStore) each(ctx context.Context, fn func(*chronograf.User)) error {
return s.client.db.View(func(tx *bolt.Tx) error {
return tx.Bucket(UsersBucket).ForEach(func(k, v []byte) error {
var user chronograf.User
if err := internal.UnmarshalUser(v, &user); err != nil {
return err
}
fn(&user)
return nil
})
})
}
// Get searches the UsersStore for user with 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
func (s *UsersStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
if q.ID != nil {
return s.get(ctx, *q.ID)
}
u, err := s.get(ctx, uid)
if err != nil {
return nil, err
if q.Name != nil && q.Provider != nil {
var user *chronograf.User
err := s.each(ctx, func(u *chronograf.User) {
if user != nil {
return
}
if u.Name == *q.Name && u.Provider == *q.Provider {
user = u
}
})
if err != nil {
return nil, err
}
if user == nil {
return nil, fmt.Errorf("user not found")
}
return user, nil
}
return u, nil
return nil, fmt.Errorf("must specify ID or Provider and Scheme in UserQuery")
}
// Add a new Users in the UsersStore.

View File

@ -2,7 +2,6 @@ package bolt_test
import (
"context"
"fmt"
"reflect"
"testing"
@ -21,7 +20,7 @@ var cmpOptions = cmp.Options{
func TestUsersStore_Get(t *testing.T) {
type args struct {
ctx context.Context
ID string
ID uint64
}
tests := []struct {
name string
@ -33,7 +32,7 @@ func TestUsersStore_Get(t *testing.T) {
name: "User not found",
args: args{
ctx: context.Background(),
ID: "1337",
ID: 1337,
},
wantErr: true,
},
@ -49,7 +48,7 @@ func TestUsersStore_Get(t *testing.T) {
defer client.Close()
s := client.UsersStore
got, err := s.Get(tt.args.ctx, tt.args.ID)
got, err := s.Get(tt.args.ctx, chronograf.UserQuery{ID: &tt.args.ID})
if (err != nil) != tt.wantErr {
t.Errorf("%q. UsersStore.Get() error = %v, wantErr %v", tt.name, err, tt.wantErr)
continue
@ -114,7 +113,7 @@ func TestUsersStore_Add(t *testing.T) {
continue
}
got, err = s.Get(tt.args.ctx, fmt.Sprintf("%d", got.ID))
got, err = s.Get(tt.args.ctx, chronograf.UserQuery{ID: &got.ID})
if err != nil {
t.Fatalf("failed to get user: %v", err)
}
@ -298,7 +297,7 @@ func TestUsersStore_Update(t *testing.T) {
continue
}
got, err := s.Get(tt.args.ctx, fmt.Sprintf("%d", tt.args.usr.ID))
got, err := s.Get(tt.args.ctx, chronograf.UserQuery{ID: &tt.args.usr.ID})
if err != nil {
t.Fatalf("failed to get user: %v", err)
}

View File

@ -606,6 +606,12 @@ type User struct {
Scheme string `json:"scheme,omitempty"`
}
type UserQuery struct {
ID *uint64
Name *string
Provider *string
}
// UsersStore is the Storage and retrieval of authentication information
type UsersStore interface {
// All lists all users from the UsersStore
@ -615,7 +621,7 @@ type UsersStore interface {
// Delete the User from the UsersStore
Delete(context.Context, *User) error
// Get retrieves a user if name exists.
Get(ctx context.Context, name string) (*User, error)
Get(ctx context.Context, q UserQuery) (*User, error)
// Update the user's permissions or roles
Update(context.Context, *User) error
}

View File

@ -2,6 +2,7 @@ package enterprise
import (
"context"
"fmt"
"github.com/influxdata/chronograf"
)
@ -28,7 +29,7 @@ func (c *UserStore) Add(ctx context.Context, u *chronograf.User) (*chronograf.Us
}
}
return c.Get(ctx, u.Name)
return c.Get(ctx, chronograf.UserQuery{Name: &u.Name})
}
// Delete the User from Influx Enterprise
@ -37,8 +38,11 @@ func (c *UserStore) Delete(ctx context.Context, u *chronograf.User) error {
}
// Get retrieves a user if name exists.
func (c *UserStore) Get(ctx context.Context, name string) (*chronograf.User, error) {
u, err := c.Ctrl.User(ctx, name)
func (c *UserStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
if q.Name == nil {
return nil, fmt.Errorf("query must specify name")
}
u, err := c.Ctrl.User(ctx, *q.Name)
if err != nil {
return nil, err
}
@ -48,7 +52,7 @@ func (c *UserStore) Get(ctx context.Context, name string) (*chronograf.User, err
return nil, err
}
role := ur[name]
role := ur[*q.Name]
cr := role.ToChronograf()
// For now we are removing all users from a role being returned.
for i, r := range cr {

View File

@ -375,7 +375,7 @@ func TestClient_Get(t *testing.T) {
Ctrl: tt.fields.Ctrl,
Logger: tt.fields.Logger,
}
got, err := c.Get(tt.args.ctx, tt.args.name)
got, err := c.Get(tt.args.ctx, chronograf.UserQuery{Name: &tt.args.name})
if (err != nil) != tt.wantErr {
t.Errorf("%q. Client.Get() error = %v, wantErr %v", tt.name, err, tt.wantErr)
continue

View File

@ -21,7 +21,7 @@ func (c *Client) Add(ctx context.Context, u *chronograf.User) (*chronograf.User,
return nil, err
}
}
return c.Get(ctx, u.Name)
return c.Get(ctx, chronograf.UserQuery{Name: &u.Name})
}
// Delete the User from InfluxDB
@ -54,14 +54,18 @@ func (c *Client) Delete(ctx context.Context, u *chronograf.User) error {
}
// Get retrieves a user if name exists.
func (c *Client) Get(ctx context.Context, name string) (*chronograf.User, error) {
func (c *Client) Get(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
if q.Name == nil {
return nil, fmt.Errorf("query must specify name")
}
users, err := c.showUsers(ctx)
if err != nil {
return nil, err
}
for _, user := range users {
if user.Name == name {
if user.Name == *q.Name {
perms, err := c.userPermissions(ctx, user.Name)
if err != nil {
return nil, err
@ -82,7 +86,7 @@ func (c *Client) Update(ctx context.Context, u *chronograf.User) error {
return c.updatePassword(ctx, u.Name, u.Passwd)
}
user, err := c.Get(ctx, u.Name)
user, err := c.Get(ctx, chronograf.UserQuery{Name: &u.Name})
if err != nil {
return err
}

View File

@ -392,7 +392,7 @@ func TestClient_Get(t *testing.T) {
Logger: log.New(log.DebugLevel),
}
defer ts.Close()
got, err := c.Get(tt.args.ctx, tt.args.name)
got, err := c.Get(tt.args.ctx, chronograf.UserQuery{Name: &tt.args.name})
if (err != nil) != tt.wantErr {
t.Errorf("%q. Client.Get() error = %v, wantErr %v", tt.name, err, tt.wantErr)
continue

View File

@ -13,7 +13,7 @@ type UsersStore struct {
AllF func(context.Context) ([]chronograf.User, error)
AddF func(context.Context, *chronograf.User) (*chronograf.User, error)
DeleteF func(context.Context, *chronograf.User) error
GetF func(ctx context.Context, name string) (*chronograf.User, error)
GetF func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error)
UpdateF func(context.Context, *chronograf.User) error
}
@ -33,8 +33,8 @@ func (s *UsersStore) Delete(ctx context.Context, u *chronograf.User) error {
}
// Get retrieves a user if name exists.
func (s *UsersStore) Get(ctx context.Context, name string) (*chronograf.User, error) {
return s.GetF(ctx, name)
func (s *UsersStore) Get(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return s.GetF(ctx, q)
}
// Update the user's permissions or roles

View File

@ -44,7 +44,7 @@ func TestService_Me(t *testing.T) {
fields: fields{
UseAuth: true,
UsersStore: &mocks.UsersStore{
GetF: func(ctx context.Context, name string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
Name: "me",
Provider: "GitHub",
@ -85,7 +85,7 @@ func TestService_Me(t *testing.T) {
fields: fields{
UseAuth: true,
UsersStore: &mocks.UsersStore{
GetF: func(ctx context.Context, name string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return nil, fmt.Errorf("Unknown User")
},
AddF: func(ctx context.Context, u *chronograf.User) (*chronograf.User, error) {
@ -125,7 +125,7 @@ func TestService_Me(t *testing.T) {
fields: fields{
UseAuth: true,
UsersStore: &mocks.UsersStore{
GetF: func(ctx context.Context, name string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return nil, fmt.Errorf("Unknown User")
},
AddF: func(ctx context.Context, u *chronograf.User) (*chronograf.User, error) {

View File

@ -451,7 +451,7 @@ func (s *Service) SourceUserID(w http.ResponseWriter, r *http.Request) {
return
}
store := ts.Users(ctx)
u, err := store.Get(ctx, uid)
u, err := store.Get(ctx, chronograf.UserQuery{Name: &uid})
if err != nil {
Error(w, http.StatusBadRequest, err.Error(), s.Logger)
return
@ -514,7 +514,7 @@ func (s *Service) UpdateSourceUser(w http.ResponseWriter, r *http.Request) {
return
}
u, err := store.Get(ctx, uid)
u, err := store.Get(ctx, chronograf.UserQuery{Name: &uid})
if err != nil {
Error(w, http.StatusBadRequest, err.Error(), s.Logger)
return

View File

@ -778,7 +778,7 @@ func TestService_SourceUserID(t *testing.T) {
},
UsersF: func(ctx context.Context) chronograf.UsersStore {
return &mocks.UsersStore{
GetF: func(ctx context.Context, uid string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
Name: "strickland",
Passwd: "discipline",
@ -833,7 +833,7 @@ func TestService_SourceUserID(t *testing.T) {
},
UsersF: func(ctx context.Context) chronograf.UsersStore {
return &mocks.UsersStore{
GetF: func(ctx context.Context, uid string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
Name: "strickland",
Passwd: "discipline",
@ -1041,7 +1041,7 @@ func TestService_UpdateSourceUser(t *testing.T) {
UpdateF: func(ctx context.Context, u *chronograf.User) error {
return nil
},
GetF: func(ctx context.Context, name string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
Name: "marty",
}, nil
@ -1093,7 +1093,7 @@ func TestService_UpdateSourceUser(t *testing.T) {
UpdateF: func(ctx context.Context, u *chronograf.User) error {
return nil
},
GetF: func(ctx context.Context, name string) (*chronograf.User, error) {
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
return &chronograf.User{
Name: "marty",
}, nil

View File

@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"sort"
"strconv"
"github.com/bouk/httprouter"
"github.com/influxdata/chronograf"
@ -132,8 +133,13 @@ var (
func (s *Service) UserID(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
id := httprouter.GetParamFromContext(ctx, "id")
user, err := s.UsersStore.Get(ctx, id)
idStr := httprouter.GetParamFromContext(ctx, "id")
id, err := strconv.ParseUint(idStr, 10, 64)
if err != nil {
Error(w, http.StatusBadRequest, fmt.Sprintf("invalid user id: %s", err.Error()), s.Logger)
return
}
user, err := s.UsersStore.Get(ctx, chronograf.UserQuery{ID: &id})
if err != nil {
Error(w, http.StatusBadRequest, err.Error(), s.Logger)
return
@ -178,9 +184,14 @@ func (s *Service) NewUser(w http.ResponseWriter, r *http.Request) {
// RemoveUser deletes a Chronograf user from store
func (s *Service) RemoveUser(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
id := httprouter.GetParamFromContext(ctx, "id")
idStr := httprouter.GetParamFromContext(ctx, "id")
id, err := strconv.ParseUint(idStr, 10, 64)
if err != nil {
Error(w, http.StatusBadRequest, fmt.Sprintf("invalid user id: %s", err.Error()), s.Logger)
return
}
u, err := s.UsersStore.Get(ctx, id)
u, err := s.UsersStore.Get(ctx, chronograf.UserQuery{ID: &id})
if err != nil {
Error(w, http.StatusNotFound, err.Error(), s.Logger)
}
@ -205,9 +216,14 @@ func (s *Service) UpdateUser(w http.ResponseWriter, r *http.Request) {
}
ctx := r.Context()
id := httprouter.GetParamFromContext(ctx, "id")
idStr := httprouter.GetParamFromContext(ctx, "id")
id, err := strconv.ParseUint(idStr, 10, 64)
if err != nil {
Error(w, http.StatusBadRequest, fmt.Sprintf("invalid user id: %s", err.Error()), s.Logger)
return
}
u, err := s.UsersStore.Get(ctx, id)
u, err := s.UsersStore.Get(ctx, chronograf.UserQuery{ID: &id})
if err != nil {
Error(w, http.StatusNotFound, err.Error(), s.Logger)
}

View File

@ -47,9 +47,9 @@ func TestService_UserID(t *testing.T) {
fields: fields{
Logger: log.New(log.DebugLevel),
UsersStore: &mocks.UsersStore{
GetF: func(ctx context.Context, ID string) (*chronograf.User, error) {
switch ID {
case "1337":
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
switch *q.ID {
case 1337:
return &chronograf.User{
ID: 1337,
Name: "billysteve",
@ -60,7 +60,7 @@ func TestService_UserID(t *testing.T) {
},
}, nil
default:
return nil, fmt.Errorf("User with ID %s not found", ID)
return nil, fmt.Errorf("User with ID %s not found", *q.ID)
}
},
},
@ -212,9 +212,9 @@ func TestService_RemoveUser(t *testing.T) {
fields: fields{
Logger: log.New(log.DebugLevel),
UsersStore: &mocks.UsersStore{
GetF: func(ctx context.Context, ID string) (*chronograf.User, error) {
switch ID {
case "1339":
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
switch *q.ID {
case 1339:
return &chronograf.User{
ID: 1339,
Name: "helena",
@ -222,7 +222,7 @@ func TestService_RemoveUser(t *testing.T) {
Scheme: "LDAP",
}, nil
default:
return nil, fmt.Errorf("User with ID %s not found", ID)
return nil, fmt.Errorf("User with ID %s not found", *q.ID)
}
},
DeleteF: func(ctx context.Context, user *chronograf.User) error {
@ -303,9 +303,9 @@ func TestService_UpdateUser(t *testing.T) {
UpdateF: func(ctx context.Context, user *chronograf.User) error {
return nil
},
GetF: func(ctx context.Context, ID string) (*chronograf.User, error) {
switch ID {
case "1336":
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
switch *q.ID {
case 1336:
return &chronograf.User{
ID: 1336,
Name: "bobbetta2",
@ -316,7 +316,7 @@ func TestService_UpdateUser(t *testing.T) {
},
}, nil
default:
return nil, fmt.Errorf("User with ID %s not found", ID)
return nil, fmt.Errorf("User with ID %s not found", *q.ID)
}
},
},
@ -351,9 +351,9 @@ func TestService_UpdateUser(t *testing.T) {
UpdateF: func(ctx context.Context, user *chronograf.User) error {
return nil
},
GetF: func(ctx context.Context, ID string) (*chronograf.User, error) {
switch ID {
case "1336":
GetF: func(ctx context.Context, q chronograf.UserQuery) (*chronograf.User, error) {
switch *q.ID {
case 1336:
return &chronograf.User{
ID: 1336,
Name: "bobbetta2",
@ -361,7 +361,7 @@ func TestService_UpdateUser(t *testing.T) {
Scheme: "OAuth2",
}, nil
default:
return nil, fmt.Errorf("User with ID %s not found", ID)
return nil, fmt.Errorf("User with ID %s not found", *q.ID)
}
},
},