From 9447cb90748539b8f5cb8e5f09a3a9627e6080f6 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 25 Sep 2024 10:00:14 +0530 Subject: [PATCH] MM-60171: Using a generic function to allocate values (#28245) We sprinkle a bit of generic magic to refactor a lot of duplicate code. To avoid exposing unnecessary code, I duplicated the function twice. But let me know if you have strong opinions about this. https://mattermost.atlassian.net/browse/MM-60171 ```release-note NONE ``` --------- Co-authored-by: Mattermost Build --- server/channels/app/platform/session.go | 11 +++------- server/channels/app/platform/status.go | 18 +++------------- server/channels/app/platform/utils.go | 10 +++++++++ .../store/localcachelayer/channel_layer.go | 21 +++---------------- .../channels/store/localcachelayer/layer.go | 10 +++++++++ .../store/localcachelayer/role_layer.go | 6 +----- .../store/localcachelayer/user_layer.go | 19 +++-------------- 7 files changed, 33 insertions(+), 62 deletions(-) diff --git a/server/channels/app/platform/session.go b/server/channels/app/platform/session.go index 0b0bf495e8..27d5f0d315 100644 --- a/server/channels/app/platform/session.go +++ b/server/channels/app/platform/session.go @@ -63,14 +63,9 @@ func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) { return nil } - toPass := make([]any, 0, len(keys)) - for i := 0; i < len(keys); i++ { - // This always needs to be a pointer to a value. - // Otherwise the msp unmarshaler will fail to work. - var session model.Session - toPass = append(toPass, &session) - } - + // This always needs to be model.Session, not *model.Session. + // Otherwise the msp unmarshaler will fail to work. + toPass := allocateCacheTargets[model.Session](len(keys)) errs := ps.sessionCache.GetMulti(keys, toPass) for i, err := range errs { if err != nil { diff --git a/server/channels/app/platform/status.go b/server/channels/app/platform/status.go index 57ce185e91..b15e3ace8a 100644 --- a/server/channels/app/platform/status.go +++ b/server/channels/app/platform/status.go @@ -46,11 +46,7 @@ func (ps *PlatformService) GetAllStatuses() map[string]*model.Status { return nil } - toPass := make([]any, 0, len(keys)) - for i := 0; i < len(keys); i++ { - var status *model.Status - toPass = append(toPass, &status) - } + toPass := allocateCacheTargets[*model.Status](len(keys)) errs := ps.statusCache.GetMulti(keys, toPass) for i, err := range errs { if err != nil { @@ -84,11 +80,7 @@ func (ps *PlatformService) GetStatusesByIds(userIDs []string) (map[string]any, * metrics := ps.Metrics() missingUserIds := []string{} - toPass := make([]any, 0, len(userIDs)) - for i := 0; i < len(userIDs); i++ { - var status *model.Status - toPass = append(toPass, &status) - } + toPass := allocateCacheTargets[*model.Status](len(userIDs)) // First, we do a GetMulti to get all the status objects. errs := ps.statusCache.GetMulti(userIDs, toPass) for i, err := range errs { @@ -147,11 +139,7 @@ func (ps *PlatformService) GetUserStatusesByIds(userIDs []string) ([]*model.Stat metrics := ps.Metrics() missingUserIds := []string{} - toPass := make([]any, 0, len(userIDs)) - for i := 0; i < len(userIDs); i++ { - var status *model.Status - toPass = append(toPass, &status) - } + toPass := allocateCacheTargets[*model.Status](len(userIDs)) // First, we do a GetMulti to get all the status objects. errs := ps.statusCache.GetMulti(userIDs, toPass) for i, err := range errs { diff --git a/server/channels/app/platform/utils.go b/server/channels/app/platform/utils.go index 240e35655d..c8f48c0495 100644 --- a/server/channels/app/platform/utils.go +++ b/server/channels/app/platform/utils.go @@ -20,3 +20,13 @@ func maxInt(a, b int) int { } return b } + +// allocateCacheTargets is used to fill target value types +// for getting items from cache. +func allocateCacheTargets[T any](l int) []any { + toPass := make([]any, 0, l) + for i := 0; i < l; i++ { + toPass = append(toPass, new(T)) + } + return toPass +} diff --git a/server/channels/store/localcachelayer/channel_layer.go b/server/channels/store/localcachelayer/channel_layer.go index 4e3bd1f173..2a58f51405 100644 --- a/server/channels/store/localcachelayer/channel_layer.go +++ b/server/channels/store/localcachelayer/channel_layer.go @@ -245,12 +245,7 @@ func (s LocalCacheChannelStore) GetMany(ids []string, allowFromCache bool) (mode var channelsToQuery []string if allowFromCache { - var toPass []any - for i := 0; i < len(ids); i++ { - var channel *model.Channel - toPass = append(toPass, &channel) - } - + toPass := allocateCacheTargets[*model.Channel](len(ids)) errs := s.rootStore.doMultiReadCache(s.rootStore.roleCache, ids, toPass) for i, err := range errs { if err != nil { @@ -352,12 +347,7 @@ func (s LocalCacheChannelStore) getByNames(teamId string, names []string, allowF newKeys = append(newKeys, teamId+name) } - toPass := make([]any, 0, len(newKeys)) - for i := 0; i < len(newKeys); i++ { - var channel *model.Channel - toPass = append(toPass, &channel) - } - + toPass := allocateCacheTargets[*model.Channel](len(newKeys)) errs := s.rootStore.doMultiReadCache(s.rootStore.roleCache, newKeys, toPass) for i, err := range errs { if err != nil { @@ -474,12 +464,7 @@ func (s LocalCacheChannelStore) GetChannelsMemberCount(channelIDs []string) (_ m counts := make(map[string]int64) remainingChannels := make([]string, 0) - toPass := make([]any, 0, len(channelIDs)) - for i := 0; i < len(channelIDs); i++ { - var cacheItem int64 - toPass = append(toPass, &cacheItem) - } - + toPass := allocateCacheTargets[int64](len(channelIDs)) errs := s.rootStore.doMultiReadCache(s.rootStore.reaction.rootStore.channelMemberCountsCache, channelIDs, toPass) for i, err := range errs { if err != nil { diff --git a/server/channels/store/localcachelayer/layer.go b/server/channels/store/localcachelayer/layer.go index 76858bdf8c..20e55c0fdd 100644 --- a/server/channels/store/localcachelayer/layer.go +++ b/server/channels/store/localcachelayer/layer.go @@ -579,3 +579,13 @@ func (s *LocalCacheStore) Invalidate() { s.doClearCacheCluster(s.teamAllTeamIdsForUserCache) s.doClearCacheCluster(s.rolePermissionsCache) } + +// allocateCacheTargets is used to fill target value types +// for getting items from cache. +func allocateCacheTargets[T any](l int) []any { + toPass := make([]any, 0, l) + for i := 0; i < l; i++ { + toPass = append(toPass, new(T)) + } + return toPass +} diff --git a/server/channels/store/localcachelayer/role_layer.go b/server/channels/store/localcachelayer/role_layer.go index fdd65cd88d..23047dd881 100644 --- a/server/channels/store/localcachelayer/role_layer.go +++ b/server/channels/store/localcachelayer/role_layer.go @@ -62,11 +62,7 @@ func (s LocalCacheRoleStore) GetByNames(names []string) ([]*model.Role, error) { var foundRoles []*model.Role var rolesToQuery []string - toPass := make([]any, 0, len(names)) - for i := 0; i < len(names); i++ { - var role *model.Role - toPass = append(toPass, &role) - } + toPass := allocateCacheTargets[*model.Role](len(names)) errs := s.rootStore.doMultiReadCache(s.rootStore.roleCache, names, toPass) for i, err := range errs { if err != nil { diff --git a/server/channels/store/localcachelayer/user_layer.go b/server/channels/store/localcachelayer/user_layer.go index 24de677b6a..85744995c3 100644 --- a/server/channels/store/localcachelayer/user_layer.go +++ b/server/channels/store/localcachelayer/user_layer.go @@ -82,11 +82,7 @@ func (s *LocalCacheUserStore) InvalidateProfilesInChannelCacheByUser(userId stri return nil } - toPass := make([]any, 0, len(keys)) - for i := 0; i < len(keys); i++ { - var userMap model.UserMap - toPass = append(toPass, &userMap) - } + toPass := allocateCacheTargets[model.UserMap](len(keys)) errs := s.rootStore.doMultiReadCache(s.rootStore.profilesInChannelCache, keys, toPass) for i, err := range errs { if err != nil { @@ -177,11 +173,7 @@ func (s *LocalCacheUserStore) GetProfileByIds(ctx context.Context, userIds []str remainingUserIds := make([]string, 0) fromMaster := false - toPass := make([]any, 0, len(userIds)) - for i := 0; i < len(userIds); i++ { - var user *model.User - toPass = append(toPass, &user) - } + toPass := allocateCacheTargets[*model.User](len(userIds)) errs := s.rootStore.doMultiReadCache(s.rootStore.userProfileByIdsCache, userIds, toPass) for i, err := range errs { if err != nil { @@ -263,12 +255,7 @@ func (s *LocalCacheUserStore) GetMany(ctx context.Context, ids []string) ([]*mod uniqIDs := dedup(ids) fromMaster := false - toPass := make([]any, 0, len(uniqIDs)) - for i := 0; i < len(uniqIDs); i++ { - var user *model.User - toPass = append(toPass, &user) - } - + toPass := allocateCacheTargets[*model.User](len(uniqIDs)) errs := s.rootStore.doMultiReadCache(s.rootStore.userProfileByIdsCache, uniqIDs, toPass) for i, err := range errs { if err != nil {