From f18323980fe4b2e1707a625a09c04b7e6af3fe62 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 17 Sep 2024 09:37:54 +0530 Subject: [PATCH] MM-60413: Fix issues due to incorrect cache provider (#28181) We were using a mock cache provider which mixed up LRU caches with each other. This led to incorrect unmarshalling method calls. We fix this by using the real cache provider. https://mattermost.atlassian.net/browse/MM-60413 ```release-note NONE ``` --- server/channels/store/localcachelayer/layer_test.go | 3 ++- server/channels/store/localcachelayer/user_layer.go | 7 +++---- server/platform/services/cache/lru.go | 5 ----- server/platform/services/cache/lru_test.go | 6 +++--- server/platform/services/cache/redis.go | 11 ----------- 5 files changed, 8 insertions(+), 24 deletions(-) diff --git a/server/channels/store/localcachelayer/layer_test.go b/server/channels/store/localcachelayer/layer_test.go index 0012194bcb..453337b5d6 100644 --- a/server/channels/store/localcachelayer/layer_test.go +++ b/server/channels/store/localcachelayer/layer_test.go @@ -14,6 +14,7 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/channels/store/sqlstore" "github.com/mattermost/mattermost/server/v8/channels/store/storetest" + "github.com/mattermost/mattermost/server/v8/platform/services/cache" "golang.org/x/sync/errgroup" ) @@ -108,7 +109,7 @@ func initStores(logger mlog.LoggerIFace) { if err != nil { return err } - st.Store, err = NewLocalCacheLayer(st.SqlStore, nil, nil, getMockCacheProvider(), logger) + st.Store, err = NewLocalCacheLayer(st.SqlStore, nil, nil, cache.NewProvider(), logger) if err != nil { return err } diff --git a/server/channels/store/localcachelayer/user_layer.go b/server/channels/store/localcachelayer/user_layer.go index 5dd5808414..24de677b6a 100644 --- a/server/channels/store/localcachelayer/user_layer.go +++ b/server/channels/store/localcachelayer/user_layer.go @@ -84,8 +84,7 @@ func (s *LocalCacheUserStore) InvalidateProfilesInChannelCacheByUser(userId stri toPass := make([]any, 0, len(keys)) for i := 0; i < len(keys); i++ { - // Note: keep https://github.com/mattermost/mattermost/pull/27830 in mind. - var userMap map[string]*model.User + var userMap model.UserMap toPass = append(toPass, &userMap) } errs := s.rootStore.doMultiReadCache(s.rootStore.profilesInChannelCache, keys, toPass) @@ -96,7 +95,7 @@ func (s *LocalCacheUserStore) InvalidateProfilesInChannelCacheByUser(userId stri } continue } - gotMap := *(toPass[i].(*map[string]*model.User)) + gotMap := *(toPass[i].(*model.UserMap)) if gotMap == nil { s.rootStore.logger.Warn("Found nil userMap in InvalidateProfilesInChannelCacheByUser. This is not expected") continue @@ -147,7 +146,7 @@ func (s *LocalCacheUserStore) GetAllProfiles(options *model.UserGetOptions) ([]* func (s *LocalCacheUserStore) GetAllProfilesInChannel(ctx context.Context, channelId string, allowFromCache bool) (map[string]*model.User, error) { if allowFromCache { - var cachedMap map[string]*model.User + var cachedMap model.UserMap if err := s.rootStore.doStandardReadCache(s.rootStore.profilesInChannelCache, channelId, &cachedMap); err == nil { return cachedMap, nil } diff --git a/server/platform/services/cache/lru.go b/server/platform/services/cache/lru.go index ee7c07bfda..a23faef5af 100644 --- a/server/platform/services/cache/lru.go +++ b/server/platform/services/cache/lru.go @@ -220,11 +220,6 @@ func (l *LRU) get(key string, value any) error { _, err := u.UnmarshalMsg(val) *v = &u return err - case *map[string]*model.User: - var u model.UserMap - _, err := u.UnmarshalMsg(val) - *v = u - return err } // Slow path for other structs. diff --git a/server/platform/services/cache/lru_test.go b/server/platform/services/cache/lru_test.go index a109511bb6..620082b6a1 100644 --- a/server/platform/services/cache/lru_test.go +++ b/server/platform/services/cache/lru_test.go @@ -294,12 +294,12 @@ func TestLRUMarshalUnMarshal(t *testing.T) { u.Timezone = nil require.Equal(t, user, u) - tt := make(map[string]*model.User) + tt := make(model.UserMap) tt["1"] = u - err = l.SetWithDefaultExpiry("mm", model.UserMap(tt)) + err = l.SetWithDefaultExpiry("mm", tt) require.NoError(t, err) - var out map[string]*model.User + var out model.UserMap err = l.Get("mm", &out) require.NoError(t, err) out["1"].Timezone = nil diff --git a/server/platform/services/cache/redis.go b/server/platform/services/cache/redis.go index 38b72d2c5c..9bf5bbf68a 100644 --- a/server/platform/services/cache/redis.go +++ b/server/platform/services/cache/redis.go @@ -123,11 +123,6 @@ func (r *Redis) Get(key string, value any) error { _, err := u.UnmarshalMsg(val) *v = &u return err - case *map[string]*model.User: - var u model.UserMap - _, err := u.UnmarshalMsg(val) - *v = u - return err } // Slow path for other structs. @@ -193,12 +188,6 @@ func (r *Redis) GetMulti(keys []string, values []any) []error { *v = &u errs[i] = err continue - case *map[string]*model.User: - var u model.UserMap - _, err := u.UnmarshalMsg(buf) - *v = u - errs[i] = err - continue } // Slow path for other structs.