From c4f6f729fd67dc8ba7cca794fbc4083b3410dd62 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Wed, 26 Dec 2018 21:38:18 -0600 Subject: [PATCH] fix(inmem): user service now filters by id Related: #2072 --- bolt/kv_test.go | 2 +- inmem/kv_test.go | 2 +- inmem/user_service.go | 41 +++++++++----- kv/example.go | 23 +++++--- testing/user_service.go | 116 ++++++++++++++++++++++++++++++++++++---- testing/util.go | 8 +-- 6 files changed, 153 insertions(+), 39 deletions(-) diff --git a/bolt/kv_test.go b/bolt/kv_test.go index 8b083518b7f..28512445821 100644 --- a/bolt/kv_test.go +++ b/bolt/kv_test.go @@ -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 { diff --git a/inmem/kv_test.go b/inmem/kv_test.go index 69eccf84450..5c5e5a1d4c9 100644 --- a/inmem/kv_test.go +++ b/inmem/kv_test.go @@ -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) diff --git a/inmem/user_service.go b/inmem/user_service.go index d1d63b5ff6a..4a5b3cd1088 100644 --- a/inmem/user_service.go +++ b/inmem/user_service.go @@ -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.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 @@ -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. diff --git a/kv/example.go b/kv/example.go index 866b2d704e2..0768f6e1c4a 100644 --- a/kv/example.go +++ b/kv/example.go @@ -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, } } diff --git a/testing/user_service.go b/testing/user_service.go index 307d468a89c..cb7b06df7dc 100644 --- a/testing/user_service.go +++ b/testing/user_service.go @@ -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{ + 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{ @@ -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 != "" { diff --git a/testing/util.go b/testing/util.go index 63fe1d61c9f..52b7d48275f 100644 --- a/testing/util.go +++ b/testing/util.go @@ -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)) } }