Skip to content

Commit

Permalink
Merge pull request #2144 from influxdata/fix/inmem-user-filtering
Browse files Browse the repository at this point in the history 
fix(inmem): user service now filters by id
  • Loading branch information
@goller
goller authored Dec 27, 2018
2 parents f5b0bde + c4f6f72 commit 7618286
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 39 deletions.
2 changes: 1 addition & 1 deletion bolt/kv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion inmem/kv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
41 changes: 27 additions & 14 deletions inmem/user_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.ID != nil {
u, err := s.FindUserByID(ctx, *filter.ID)
if err != nil {
return nil, &platform.Error{
Op: op,
Err: err,
}
}
return u, nil
}

if filter.Name != nil {
var o *platform.User
var u *platform.User

err := s.forEachUser(ctx, func(org *platform.User) bool {
if org.Name == *filter.Name {
o = org
err := s.forEachUser(ctx, func(user *platform.User) bool {
if user.Name == *filter.Name {
u = user
return false
}
return true
Expand All @@ -76,15 +89,15 @@ 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,
Msg: "user not found",
}
}

return o, nil
return u, nil
}

return nil, &platform.Error{
Expand All @@ -98,40 +111,40 @@ 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,
Op: op,
}
}

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,
Op: op,
}
}

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

if err != nil {
return nil, 0, err
}

return orgs, len(orgs), nil
return users, len(users), nil
}

// CreateUser will create an user into storage.
Expand Down
23 changes: 15 additions & 8 deletions kv/example.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}
Expand Down
116 changes: 105 additions & 11 deletions testing/user_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func FindUser(
t *testing.T,
) {
type args struct {
name string
filter platform.UserFilter
}

type wants struct {
Expand Down Expand Up @@ -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{
Expand All @@ -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{
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{
name: "abc",
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{
Expand All @@ -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 != "" {
Expand Down
8 changes: 4 additions & 4 deletions testing/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down

0 comments on commit 7618286

Please sign in to comment.