Revert "MM-21209: Use the LRU cache for UserStore.Get call too (#13456)" (#13824)

This reverts commit 1e28ad4a73.
Этот коммит содержится в:
Joram Wilander
2020-02-05 09:42:36 -05:00
коммит произвёл GitHub
родитель 597a2b77cd
Коммит eb29750f19
4 изменённых файлов: 2 добавлений и 94 удалений

Просмотреть файл

@@ -223,7 +223,6 @@ func getMockStore() *mocks.Store {
mockUserStore.On("GetAllProfilesInChannel", "123", true).Return(fakeProfilesInChannelMap, nil)
mockUserStore.On("GetAllProfilesInChannel", "123", false).Return(fakeProfilesInChannelMap, nil)
mockUserStore.On("Get", "123").Return(fakeUser[0], nil)
mockStore.On("User").Return(&mockUserStore)
fakeUserTeamIds := []string{"1", "2", "3"}

Просмотреть файл

@@ -130,31 +130,8 @@ func (s LocalCacheUserStore) GetProfileByIds(userIds []string, options *store.Us
users = append(users, user.DeepCopy())
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.DeepCopy(), 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.DeepCopy(), nil
}

Просмотреть файл

@@ -19,7 +19,7 @@ func TestUserStore(t *testing.T) {
StoreTestWithSqlSupplier(t, storetest.TestUserStore)
}
func TestUserStoreGetProfileByIdsCache(t *testing.T) {
func TestUserStoreCache(t *testing.T) {
fakeUserIds := []string{"123"}
fakeUser := []*model.User{{Id: "123"}}
@@ -179,69 +179,3 @@ func TestUserStoreProfilesInChannelCache(t *testing.T) {
mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "GetAllProfilesInChannel", 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()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
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()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
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().InvalidateProfileCacheForUser("123")
_, _ = cachedStore.User().Get(fakeUserId)
mockStore.User().(*mocks.UserStore).AssertNumberOfCalls(t, "Get", 2)
})
t.Run("should always return a copy of the stored data", func(t *testing.T) {
mockStore := getMockStore()
mockCacheProvider := getMockCacheProvider()
cachedStore := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
storedUser, err := mockStore.User().Get(fakeUserId)
require.Nil(t, err)
originalProps := storedUser.NotifyProps
storedUser.NotifyProps = map[string]string{}
storedUser.NotifyProps["key"] = "somevalue"
cachedUser, err := cachedStore.User().Get(fakeUserId)
require.Nil(t, err)
assert.Equal(t, storedUser, cachedUser)
if storedUser == cachedUser {
assert.Fail(t, "should be different pointers")
}
cachedUser.NotifyProps["key"] = "othervalue"
assert.NotEqual(t, storedUser, cachedUser)
cachedUser, err = cachedStore.User().Get(fakeUserId)
require.Nil(t, err)
assert.Equal(t, storedUser, cachedUser)
if storedUser == cachedUser {
assert.Fail(t, "should be different pointers")
}
cachedUser.NotifyProps["key"] = "othervalue"
assert.NotEqual(t, storedUser, cachedUser)
storedUser.NotifyProps = originalProps
})
}

Просмотреть файл

@@ -4949,8 +4949,6 @@ func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) {
err = ss.User().ResetLastPictureUpdate(u1.Id)
require.Nil(t, err)
ss.User().InvalidateProfileCacheForUser(u1.Id)
user2, err := ss.User().Get(u1.Id)
require.Nil(t, err)