Chore: Use cache for all signed in user lookups (#88133)

* GetSignedInUser unexported (renamed to getSignedInUser)
* GetSignedInUserWithCacheCtx renamed to GetSignedInUser
* added a check for a nil cacheservice (as defensive programming / test convenience)
This commit is contained in:
Kristin Laemmert 2024-05-22 08:58:16 -04:00 committed by GitHub
parent a5414020f8
commit 16b1e285ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 23 additions and 23 deletions

View File

@ -120,7 +120,7 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Iden
return nil return nil
} }
usr, err := s.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{ usr, err := s.userService.GetSignedInUser(ctx, &user.GetSignedInUserQuery{
UserID: userID, UserID: userID,
OrgID: r.OrgID, OrgID: r.OrgID,
}) })

View File

@ -99,7 +99,7 @@ func (a *authenticator) getSignedInUser(ctx context.Context, token string) (*use
} }
querySignedInUser := user.GetSignedInUserQuery{UserID: *apikey.ServiceAccountId, OrgID: apikey.OrgID} querySignedInUser := user.GetSignedInUserQuery{UserID: *apikey.ServiceAccountId, OrgID: apikey.OrgID}
signedInUser, err := a.UserService.GetSignedInUserWithCacheCtx(ctx, &querySignedInUser) signedInUser, err := a.UserService.GetSignedInUser(ctx, &querySignedInUser)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -140,7 +140,7 @@ type fakeUserService struct {
OrgRole org.RoleType OrgRole org.RoleType
} }
func (f *fakeUserService) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { func (f *fakeUserService) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
return &user.SignedInUser{ return &user.SignedInUser{
UserID: 1, UserID: 1,
OrgID: 1, OrgID: 1,

View File

@ -17,7 +17,6 @@ type Service interface {
GetByEmail(context.Context, *GetUserByEmailQuery) (*User, error) GetByEmail(context.Context, *GetUserByEmailQuery) (*User, error)
Update(context.Context, *UpdateUserCommand) error Update(context.Context, *UpdateUserCommand) error
UpdateLastSeenAt(context.Context, *UpdateUserLastSeenAtCommand) error UpdateLastSeenAt(context.Context, *UpdateUserLastSeenAtCommand) error
GetSignedInUserWithCacheCtx(context.Context, *GetSignedInUserQuery) (*SignedInUser, error)
GetSignedInUser(context.Context, *GetSignedInUserQuery) (*SignedInUser, error) GetSignedInUser(context.Context, *GetSignedInUserQuery) (*SignedInUser, error)
Search(context.Context, *SearchUsersQuery) (*SearchUserQueryResult, error) Search(context.Context, *SearchUsersQuery) (*SearchUserQueryResult, error)
BatchDisableUsers(context.Context, *BatchDisableUsersCommand) error BatchDisableUsers(context.Context, *BatchDisableUsersCommand) error

View File

@ -292,7 +292,7 @@ func (s *Service) UpdateLastSeenAt(ctx context.Context, cmd *user.UpdateUserLast
)) ))
defer span.End() defer span.End()
u, err := s.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{ u, err := s.GetSignedInUser(ctx, &user.GetSignedInUserQuery{
UserID: cmd.UserID, UserID: cmd.UserID,
OrgID: cmd.OrgID, OrgID: cmd.OrgID,
}) })
@ -312,7 +312,7 @@ func shouldUpdateLastSeen(t time.Time) bool {
return time.Since(t) > time.Minute*5 return time.Since(t) > time.Minute*5
} }
func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { func (s *Service) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
ctx, span := s.tracer.Start(ctx, "user.GetSignedInUserWithCacheCtx", trace.WithAttributes( ctx, span := s.tracer.Start(ctx, "user.GetSignedInUserWithCacheCtx", trace.WithAttributes(
attribute.Int64("userID", query.UserID), attribute.Int64("userID", query.UserID),
attribute.Int64("orgID", query.OrgID), attribute.Int64("orgID", query.OrgID),
@ -322,22 +322,27 @@ func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.G
var signedInUser *user.SignedInUser var signedInUser *user.SignedInUser
// only check cache if we have a user ID and an org ID in query // only check cache if we have a user ID and an org ID in query
if query.OrgID > 0 && query.UserID > 0 { if s.cacheService != nil {
cacheKey := newSignedInUserCacheKey(query.OrgID, query.UserID) if query.OrgID > 0 && query.UserID > 0 {
if cached, found := s.cacheService.Get(cacheKey); found { cacheKey := newSignedInUserCacheKey(query.OrgID, query.UserID)
cachedUser := cached.(user.SignedInUser) if cached, found := s.cacheService.Get(cacheKey); found {
signedInUser = &cachedUser cachedUser := cached.(user.SignedInUser)
return signedInUser, nil signedInUser = &cachedUser
return signedInUser, nil
}
} }
} }
result, err := s.GetSignedInUser(ctx, query) result, err := s.getSignedInUser(ctx, query)
if err != nil { if err != nil {
return nil, err return nil, err
} }
cacheKey := newSignedInUserCacheKey(result.OrgID, result.UserID) if s.cacheService != nil {
s.cacheService.Set(cacheKey, *result, time.Second*5) cacheKey := newSignedInUserCacheKey(result.OrgID, result.UserID)
s.cacheService.Set(cacheKey, *result, time.Second*5)
}
return result, nil return result, nil
} }
@ -345,8 +350,8 @@ func newSignedInUserCacheKey(orgID, userID int64) string {
return fmt.Sprintf("signed-in-user-%d-%d", userID, orgID) return fmt.Sprintf("signed-in-user-%d-%d", userID, orgID)
} }
func (s *Service) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { func (s *Service) getSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
ctx, span := s.tracer.Start(ctx, "user.GetSignedInUser", trace.WithAttributes( ctx, span := s.tracer.Start(ctx, "user.getSignedInUser", trace.WithAttributes(
attribute.Int64("userID", query.UserID), attribute.Int64("userID", query.UserID),
attribute.Int64("orgID", query.OrgID), attribute.Int64("orgID", query.OrgID),
)) ))

View File

@ -117,13 +117,13 @@ func TestUserService(t *testing.T) {
query1 := &user.GetSignedInUserQuery{OrgID: 1, UserID: 1} query1 := &user.GetSignedInUserQuery{OrgID: 1, UserID: 1}
userStore.ExpectedSignedInUser = usr userStore.ExpectedSignedInUser = usr
orgService.ExpectedUserOrgDTO = []*org.UserOrgDTO{{OrgID: 0}, {OrgID: 1}} orgService.ExpectedUserOrgDTO = []*org.UserOrgDTO{{OrgID: 0}, {OrgID: 1}}
result, err := userService.GetSignedInUserWithCacheCtx(context.Background(), query1) result, err := userService.GetSignedInUser(context.Background(), query1)
require.Nil(t, err) require.Nil(t, err)
require.NotNil(t, result) require.NotNil(t, result)
assert.Equal(t, query1.OrgID, result.OrgID) assert.Equal(t, query1.OrgID, result.OrgID)
userStore.ExpectedSignedInUser = usr2 userStore.ExpectedSignedInUser = usr2
query2 := &user.GetSignedInUserQuery{OrgID: 0, UserID: 1} query2 := &user.GetSignedInUserQuery{OrgID: 0, UserID: 1}
result2, err := userService.GetSignedInUserWithCacheCtx(context.Background(), query2) result2, err := userService.GetSignedInUser(context.Background(), query2)
require.Nil(t, err) require.Nil(t, err)
require.NotNil(t, result2) require.NotNil(t, result2)
assert.Equal(t, query2.OrgID, result2.OrgID) assert.Equal(t, query2.OrgID, result2.OrgID)

View File

@ -75,10 +75,6 @@ func (f *FakeUserService) UpdateLastSeenAt(ctx context.Context, cmd *user.Update
return f.ExpectedError return f.ExpectedError
} }
func (f *FakeUserService) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
return f.GetSignedInUser(ctx, query)
}
func (f *FakeUserService) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) { func (f *FakeUserService) GetSignedInUser(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
if f.GetSignedInUserFn != nil { if f.GetSignedInUserFn != nil {
return f.GetSignedInUserFn(ctx, query) return f.GetSignedInUserFn(ctx, query)