Merge pull request #2144 from influxdata/fix/inmem-user-filtering

fix(inmem): user service now filters by id
pull/10616/head
Chris Goller 2018-12-27 08:58:37 -06:00 committed by GitHub
commit 7618286cdd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 154 additions and 40 deletions

View File

@ -57,7 +57,7 @@ func initExampleService(f platformtesting.UserFields, t *testing.T) (platform.Us
t.Fatalf("failed to populate users")
}
}
return svc, "", func() {
return svc, "kv/", func() {
defer closeFn()
for _, u := range f.Users {
if err := svc.DeleteUser(ctx, u.ID); err != nil {

View File

@ -23,7 +23,7 @@ func initExampleService(f platformtesting.UserFields, t *testing.T) (platform.Us
t.Fatalf("failed to populate users")
}
}
return svc, "", func() {
return svc, "kv/", func() {
for _, u := range f.Users {
if err := svc.DeleteUser(ctx, u.ID); err != nil {
t.Logf("failed to remove users: %v", err)

View File

@ -7,6 +7,8 @@ import (
"github.com/influxdata/platform"
)
var _ platform.UserService = (*Service)(nil)
func (s *Service) loadUser(id platform.ID) (*platform.User, *platform.Error) {
i, ok := s.userKV.Load(id.String())
if !ok {
@ -61,12 +63,23 @@ func (s *Service) findUserByName(ctx context.Context, n string) (*platform.User,
// FindUser returns the first user that matches a filter.
func (s *Service) FindUser(ctx context.Context, filter platform.UserFilter) (*platform.User, error) {
op := OpPrefix + platform.OpFindUser
if filter.Name != nil {
var o *platform.User
if filter.ID != nil {
u, err := s.FindUserByID(ctx, *filter.ID)
if err != nil {
return nil, &platform.Error{
Op: op,
Err: err,
}
}
return u, nil
}
err := s.forEachUser(ctx, func(org *platform.User) bool {
if org.Name == *filter.Name {
o = org
if filter.Name != nil {
var u *platform.User
err := s.forEachUser(ctx, func(user *platform.User) bool {
if user.Name == *filter.Name {
u = user
return false
}
return true
@ -76,7 +89,7 @@ func (s *Service) FindUser(ctx context.Context, filter platform.UserFilter) (*pl
return nil, err
}
if o == nil {
if u == nil {
return nil, &platform.Error{
Code: platform.ENotFound,
Op: op,
@ -84,7 +97,7 @@ func (s *Service) FindUser(ctx context.Context, filter platform.UserFilter) (*pl
}
}
return o, nil
return u, nil
}
return nil, &platform.Error{
@ -98,7 +111,7 @@ func (s *Service) FindUser(ctx context.Context, filter platform.UserFilter) (*pl
func (s *Service) FindUsers(ctx context.Context, filter platform.UserFilter, opt ...platform.FindOptions) ([]*platform.User, int, error) {
op := OpPrefix + platform.OpFindUsers
if filter.ID != nil {
o, err := s.FindUserByID(ctx, *filter.ID)
u, err := s.FindUserByID(ctx, *filter.ID)
if err != nil {
return nil, 0, &platform.Error{
Err: err,
@ -106,10 +119,10 @@ func (s *Service) FindUsers(ctx context.Context, filter platform.UserFilter, opt
}
}
return []*platform.User{o}, 1, nil
return []*platform.User{u}, 1, nil
}
if filter.Name != nil {
o, err := s.FindUser(ctx, filter)
u, err := s.FindUser(ctx, filter)
if err != nil {
return nil, 0, &platform.Error{
Err: err,
@ -117,13 +130,13 @@ func (s *Service) FindUsers(ctx context.Context, filter platform.UserFilter, opt
}
}
return []*platform.User{o}, 1, nil
return []*platform.User{u}, 1, nil
}
orgs := []*platform.User{}
users := []*platform.User{}
err := s.forEachUser(ctx, func(org *platform.User) bool {
orgs = append(orgs, org)
err := s.forEachUser(ctx, func(user *platform.User) bool {
users = append(users, user)
return true
})
@ -131,7 +144,7 @@ func (s *Service) FindUsers(ctx context.Context, filter platform.UserFilter, opt
return nil, 0, err
}
return orgs, len(orgs), nil
return users, len(users), nil
}
// CreateUser will create an user into storage.

View File

@ -57,7 +57,7 @@ func (c *ExampleService) FindUserByID(ctx context.Context, id platform.ID) (*pla
if err != nil {
return nil, &platform.Error{
Op: platform.OpFindUserByID,
Op: "kv/" + platform.OpFindUserByID,
Err: err,
}
}
@ -121,7 +121,7 @@ func (c *ExampleService) findUserByName(ctx context.Context, tx Tx, n string) (*
return nil, &platform.Error{
Code: platform.ENotFound,
Msg: "user not found",
Op: platform.OpFindUser,
Op: "kv/" + platform.OpFindUser,
}
}
if err != nil {
@ -140,7 +140,14 @@ func (c *ExampleService) findUserByName(ctx context.Context, tx Tx, n string) (*
// Other filters will do a linear scan across examples until it finds a match.
func (c *ExampleService) FindUser(ctx context.Context, filter platform.UserFilter) (*platform.User, error) {
if filter.ID != nil {
return c.FindUserByID(ctx, *filter.ID)
u, err := c.FindUserByID(ctx, *filter.ID)
if err != nil {
return nil, &platform.Error{
Op: "kv/" + platform.OpFindUser,
Err: err,
}
}
return u, nil
}
if filter.Name != nil {
@ -200,7 +207,7 @@ func (c *ExampleService) FindUsers(ctx context.Context, filter platform.UserFilt
if err != nil {
return nil, 0, &platform.Error{
Err: err,
Op: op,
Op: "kv/" + op,
}
}
@ -212,7 +219,7 @@ func (c *ExampleService) FindUsers(ctx context.Context, filter platform.UserFilt
if err != nil {
return nil, 0, &platform.Error{
Err: err,
Op: op,
Op: "kv/" + op,
}
}
@ -258,7 +265,7 @@ func (c *ExampleService) CreateUser(ctx context.Context, u *platform.User) error
if err != nil {
return &platform.Error{
Err: err,
Op: platform.OpCreateUser,
Op: "kv/" + platform.OpCreateUser,
}
}
@ -355,7 +362,7 @@ func (c *ExampleService) UpdateUser(ctx context.Context, id platform.ID, upd pla
if err != nil {
return nil, &platform.Error{
Err: err,
Op: platform.OpUpdateUser,
Op: "kv/" + platform.OpUpdateUser,
}
}
@ -397,7 +404,7 @@ func (c *ExampleService) DeleteUser(ctx context.Context, id platform.ID) error {
if err != nil {
return &platform.Error{
Op: platform.OpDeleteUser,
Op: "kv/" + platform.OpDeleteUser,
Err: err,
}
}

View File

@ -575,7 +575,7 @@ func FindUser(
t *testing.T,
) {
type args struct {
name string
filter platform.UserFilter
}
type wants struct {
@ -604,7 +604,9 @@ func FindUser(
},
},
args: args{
name: "abc",
filter: platform.UserFilter{
Name: func(s string) *string { return &s }("abc"),
},
},
wants: wants{
user: &platform.User{
@ -614,12 +616,109 @@ func FindUser(
},
},
{
name: "user does not exist",
name: "find existing user by its id",
fields: UserFields{
Users: []*platform.User{
{
ID: MustIDBase16(userOneID),
Name: "abc",
},
{
ID: MustIDBase16(userTwoID),
Name: "xyz",
},
},
},
args: args{
filter: platform.UserFilter{
ID: func(id platform.ID) *platform.ID { return &id }(MustIDBase16(userOneID)),
},
},
wants: wants{
user: &platform.User{
ID: MustIDBase16(userOneID),
Name: "abc",
},
},
},
{
name: "user with name does not exist",
fields: UserFields{
Users: []*platform.User{},
},
args: args{
name: "abc",
filter: platform.UserFilter{
Name: func(s string) *string { return &s }("abc"),
},
},
wants: wants{
err: &platform.Error{
Code: platform.ENotFound,
Msg: "user not found",
Op: platform.OpFindUser,
},
},
},
{
name: "user with id does not exist",
fields: UserFields{
Users: []*platform.User{},
},
args: args{
filter: platform.UserFilter{
ID: func(id platform.ID) *platform.ID { return &id }(MustIDBase16(userOneID)),
},
},
wants: wants{
err: &platform.Error{
Code: platform.ENotFound,
Msg: "user not found",
Op: platform.OpFindUser,
},
},
},
{
name: "filter with both name and ID prefers ID",
fields: UserFields{
Users: []*platform.User{
{
ID: MustIDBase16(userOneID),
Name: "abc",
},
{
ID: MustIDBase16(userTwoID),
Name: "xyz",
},
},
},
args: args{
filter: platform.UserFilter{
ID: func(id platform.ID) *platform.ID { return &id }(MustIDBase16(userOneID)),
Name: func(s string) *string { return &s }("xyz"),
},
},
wants: wants{
user: &platform.User{
ID: MustIDBase16(userOneID),
Name: "abc",
},
},
},
{
name: "filter both name and non-existent id returns no user",
fields: UserFields{
Users: []*platform.User{
{
ID: MustIDBase16(userTwoID),
Name: "xyz",
},
},
},
args: args{
filter: platform.UserFilter{
ID: func(id platform.ID) *platform.ID { return &id }(MustIDBase16(userOneID)),
Name: func(s string) *string { return &s }("xyz"),
},
},
wants: wants{
err: &platform.Error{
@ -635,13 +734,8 @@ func FindUser(
t.Run(tt.name, func(t *testing.T) {
s, opPrefix, done := init(tt.fields, t)
defer done()
ctx := context.TODO()
filter := platform.UserFilter{}
if tt.args.name != "" {
filter.Name = &tt.args.name
}
user, err := s.FindUser(ctx, filter)
ctx := context.Background()
user, err := s.FindUser(ctx, tt.args.filter)
diffPlatformErrors(tt.name, err, tt.wants.err, opPrefix, t)
if diff := cmp.Diff(user, tt.wants.user, userCmpOptions...); diff != "" {

View File

@ -16,19 +16,19 @@ func diffPlatformErrors(name string, actual, expected error, opPrefix string, t
}
if expected != nil && actual == nil {
t.Fatalf("%s failed, expected error %s but received nil", name, expected.Error())
t.Errorf("%s failed, expected error %s but received nil", name, expected.Error())
}
if platform.ErrorCode(expected) != platform.ErrorCode(actual) {
t.Fatalf("%s failed, expected error code %q but received %q", name, platform.ErrorCode(expected), platform.ErrorCode(actual))
t.Errorf("%s failed, expected error code %q but received %q", name, platform.ErrorCode(expected), platform.ErrorCode(actual))
}
if opPrefix+platform.ErrorOp(expected) != platform.ErrorOp(actual) {
t.Fatalf("%s failed, expected error op %q but received %q", name, opPrefix+platform.ErrorOp(expected), platform.ErrorOp(actual))
t.Errorf("%s failed, expected error op %q but received %q", name, opPrefix+platform.ErrorOp(expected), platform.ErrorOp(actual))
}
if platform.ErrorMessage(expected) != platform.ErrorMessage(actual) {
t.Fatalf("%s failed, expected error message %q but received %q", name, platform.ErrorMessage(expected), platform.ErrorMessage(actual))
t.Errorf("%s failed, expected error message %q but received %q", name, platform.ErrorMessage(expected), platform.ErrorMessage(actual))
}
}