diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index bfa2dcd449..32db15e7f1 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -61,7 +61,11 @@ func (s *Server) RunOldAppInitialization() error { if s.FakeApp().Srv.newStore == nil { s.FakeApp().Srv.newStore = func() store.Store { - return store.NewTimerLayer(localcachelayer.NewLocalCacheLayer(sqlstore.NewSqlSupplier(s.FakeApp().Config().SqlSettings, s.Metrics), s.Metrics, s.Cluster), s.Metrics) + return store.NewTimerLayer( + localcachelayer.NewLocalCacheLayer( + sqlstore.NewSqlSupplier(s.FakeApp().Config().SqlSettings, s.Metrics), + s.Metrics, s.Cluster), + s.Metrics) } } diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index 15514facec..e306fe50a1 100644 --- a/store/localcachelayer/main_test.go +++ b/store/localcachelayer/main_test.go @@ -112,6 +112,7 @@ func getMockStore() *mocks.Store { mockUserStore := mocks.UserStore{} mockUserStore.On("GetProfileByIds", []string{"123"}, &store.UserGetByIdsOpts{}, true).Return(fakeUser, nil) mockUserStore.On("GetProfileByIds", []string{"123"}, &store.UserGetByIdsOpts{}, false).Return(fakeUser, nil) + mockUserStore.On("Get", "123").Return(fakeUser[0], nil) mockStore.On("User").Return(&mockUserStore) fakeUserTeamIds := []string{"1", "2", "3"} diff --git a/store/localcachelayer/user_layer.go b/store/localcachelayer/user_layer.go index 7104602a9e..102df7e342 100644 --- a/store/localcachelayer/user_layer.go +++ b/store/localcachelayer/user_layer.go @@ -80,8 +80,31 @@ func (s LocalCacheUserStore) GetProfileByIds(userIds []string, options *store.Us for _, user := range remainingUsers { s.rootStore.doStandardAddToCache(s.rootStore.userProfileByIdsCache, user.Id, user) } - } return users, nil } + +// Get is a cache wrapper around the SqlStore method to get a user profile by id. +// It checks if the user entry is present in the cache, returning the entry from cache +// if it is present. Otherwise, it fetches the entry from the store and stores it in the +// cache. +func (s LocalCacheUserStore) Get(id string) (*model.User, *model.AppError) { + cacheItem := s.rootStore.doStandardReadCache(s.rootStore.userProfileByIdsCache, id) + if cacheItem != nil { + if s.rootStore.metrics != nil { + s.rootStore.metrics.AddMemCacheHitCounter("Profile By Id", float64(1)) + } + u := *cacheItem.(*model.User) + return &u, nil + } + if s.rootStore.metrics != nil { + s.rootStore.metrics.AddMemCacheMissCounter("Profile By Id", float64(1)) + } + user, err := s.UserStore.Get(id) + if err != nil { + return nil, model.NewAppError("SqlUserStore.Get", "store.sql_user.get.app_error", nil, err.Error(), http.StatusInternalServerError) + } + s.rootStore.doStandardAddToCache(s.rootStore.userProfileByIdsCache, id, user) + return user, nil +} diff --git a/store/localcachelayer/user_layer_test.go b/store/localcachelayer/user_layer_test.go index 257968791c..9b43be64f2 100644 --- a/store/localcachelayer/user_layer_test.go +++ b/store/localcachelayer/user_layer_test.go @@ -19,7 +19,7 @@ func TestUserStore(t *testing.T) { StoreTestWithSqlSupplier(t, storetest.TestUserStore) } -func TestUserStoreCache(t *testing.T) { +func TestUserStoreGetProfileByIdsCache(t *testing.T) { fakeUserIds := []string{"123"} fakeUser := []*model.User{{Id: "123"}} @@ -64,3 +64,36 @@ func TestUserStoreCache(t *testing.T) { mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "GetProfileByIds", 2) }) } + +func TestUserStoreGetCache(t *testing.T) { + fakeUserId := "123" + fakeUser := &model.User{Id: "123"} + + t.Run("first call not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + gotUser, err := cachedStore.User().Get(fakeUserId) + require.Nil(t, err) + assert.Equal(t, fakeUser, gotUser) + mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "Get", 1) + + _, _ = cachedStore.User().Get(fakeUserId) + mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "Get", 1) + }) + + t.Run("first call not cached, invalidate, and then not cached again", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + gotUser, err := cachedStore.User().Get(fakeUserId) + require.Nil(t, err) + assert.Equal(t, fakeUser, gotUser) + mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "Get", 1) + + cachedStore.User().InvalidatProfileCacheForUser("123") + + _, _ = cachedStore.User().Get(fakeUserId) + mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "Get", 2) + }) +} diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 58a32883f5..dbb5fea27e 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -4949,6 +4949,8 @@ func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) { err = ss.User().ResetLastPictureUpdate(u1.Id) require.Nil(t, err) + ss.User().InvalidatProfileCacheForUser(u1.Id) + user2, err := ss.User().Get(u1.Id) require.Nil(t, err)