MM-20105: Migrate channelByNameCache cache from store/sqlstore/channel_store.go to the new store/localcachelayer (#13189)

* migrate * add unit test * change syntax * delete comments * remove includedeleted search method * remove include search method in correct file * change constant * add Mock.On * change to &channel * fix type for Mock.on().return() * remove inc counter * suggestions takened * get rid of unneeded concat * go fmt * import v5
2019-12-03 21:55:02 -08:00
--- a/store/localcachelayer/channel_layer.go
+++ b/store/localcachelayer/channel_layer.go
@@ -21,6 +21,14 @@ func (s *LocalCacheChannelStore) handleClusterInvalidateChannelMemberCounts(msg
 	}
 }

+func (s *LocalCacheChannelStore) handleClusterInvalidateChannelByName(msg *model.ClusterMessage) {
+	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
+		s.rootStore.channelByNameCache.Purge()
+	} else {
+		s.rootStore.channelByNameCache.Remove(msg.Data)
+	}
+}
+
 func (s *LocalCacheChannelStore) handleClusterInvalidateChannelPinnedPostCount(msg *model.ClusterMessage) {
 	if msg.Data == CLEAR_CACHE_MESSAGE_DATA {
 		s.rootStore.channelPinnedPostCountsCache.Purge()
@@ -39,12 +47,14 @@ func (s *LocalCacheChannelStore) handleClusterInvalidateChannelGuestCounts(msg *

 func (s LocalCacheChannelStore) ClearCaches() {
 	s.rootStore.doClearCacheCluster(s.rootStore.channelMemberCountsCache)
+	s.rootStore.doClearCacheCluster(s.rootStore.channelByNameCache)
 	s.rootStore.doClearCacheCluster(s.rootStore.channelPinnedPostCountsCache)
 	s.rootStore.doClearCacheCluster(s.rootStore.channelGuestCountCache)
 	s.ChannelStore.ClearCaches()
 	if s.rootStore.metrics != nil {
 		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Pinned Post Counts - Purge")
 		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge")
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel By Name - Purge")
 		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Guest Count - Purge")
 	}
 }
@@ -63,6 +73,13 @@ func (s LocalCacheChannelStore) InvalidateMemberCount(channelId string) {
 	}
 }

+func (s LocalCacheChannelStore) InvalidateChannelByName(teamId, name string) {
+	s.rootStore.doInvalidateCacheCluster(s.rootStore.channelByNameCache, teamId+name)
+	if s.rootStore.metrics != nil {
+		s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel by Name - Remove by TeamId and Name")
+	}
+}
+
 func (s LocalCacheChannelStore) InvalidateGuestCount(channelId string) {
 	s.rootStore.doInvalidateCacheCluster(s.rootStore.channelGuestCountCache, channelId)
 	if s.rootStore.metrics != nil {
@@ -113,6 +130,63 @@ func (s LocalCacheChannelStore) GetMemberCountFromCache(channelId string) int64
 	return count
 }

+// ChannelCacheByName methods
+func (s LocalCacheChannelStore) GetByName(teamId string, name string, allowFromCache bool) (*model.Channel, *model.AppError) {
+	return s.getByName(teamId, name, false, allowFromCache)
+}
+
+func (s LocalCacheChannelStore) GetByNames(teamId string, names []string, allowFromCache bool) ([]*model.Channel, *model.AppError) {
+	var channels []*model.Channel
+
+	if allowFromCache {
+		var misses []string
+		visited := make(map[string]struct{})
+		for _, name := range names {
+			if _, ok := visited[name]; ok {
+				continue
+			}
+			visited[name] = struct{}{}
+			if cacheItem := s.rootStore.doStandardReadCache(s.rootStore.channelByNameCache, teamId+name); cacheItem != nil {
+				channels = append(channels, cacheItem.(*model.Channel))
+			} else {
+				misses = append(misses, name)
+			}
+		}
+		names = misses
+	}
+
+	if len(names) > 0 {
+		dbChannels, err := s.ChannelStore.GetByNames(teamId, names, allowFromCache)
+
+		if err != nil {
+			return nil, err
+		}
+
+		for _, channel := range dbChannels {
+			s.rootStore.doStandardAddToCache(s.rootStore.channelByNameCache, teamId+channel.Name, channel)
+			channels = append(channels, channel) // add missing channels to the ones just found
+		}
+	}
+
+	return channels, nil
+}
+
+func (s LocalCacheChannelStore) getByName(teamId string, name string, includeDeleted bool, allowFromCache bool) (*model.Channel, *model.AppError) {
+	if allowFromCache {
+		if cacheItem := s.rootStore.doStandardReadCache(s.rootStore.channelByNameCache, teamId+name); cacheItem != nil {
+			return cacheItem.(*model.Channel), nil
+		}
+	}
+
+	channel, err := s.ChannelStore.GetByName(teamId, name, allowFromCache)
+
+	if allowFromCache && err == nil {
+		s.rootStore.doStandardAddToCache(s.rootStore.channelByNameCache, teamId+name, channel)
+	}
+
+	return channel, err
+}
+
 func (s LocalCacheChannelStore) GetPinnedPostCount(channelId string, allowFromCache bool) (int64, *model.AppError) {
 	if allowFromCache {
 		if count := s.rootStore.doStandardReadCache(s.rootStore.channelPinnedPostCountsCache, channelId); count != nil {
--- a/store/localcachelayer/channel_layer_test.go
+++ b/store/localcachelayer/channel_layer_test.go
@@ -4,6 +4,7 @@
 package localcachelayer

 import (
+	"github.com/mattermost/mattermost-server/v5/model"
 	"testing"

 	"github.com/stretchr/testify/assert"
@@ -91,6 +92,70 @@ func TestChannelStoreChannelMemberCountsCache(t *testing.T) {
 	})
 }

+func TestChannelStoreChannelByNameCache(t *testing.T) {
+	teamIdString := "teamID123"
+	nameString := "nameId987"
+	fakeChannel := model.Channel{Name: nameString, TeamId: teamIdString}
+
+	t.Run("first call by name not cached, second cached and returning same data", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		channel, err := cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		require.Nil(t, err)
+		assert.Equal(t, channel, &fakeChannel)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+		channel, err = cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		require.Nil(t, err)
+		assert.Equal(t, channel, &fakeChannel)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+	})
+
+	t.Run("first call by name not cached, second force no cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Channel().GetByName(teamIdString, nameString, false)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call force no cached, second not cached, third cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Channel().GetByName(teamIdString, nameString, false)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 2)
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, clear cache, second call not cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Channel().ClearCaches()
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, invalidate cache, second call not cached", func(t *testing.T) {
+		mockStore := getMockStore()
+		cachedStore := NewLocalCacheLayer(mockStore, nil, nil)
+
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 1)
+		cachedStore.Channel().InvalidateChannelByName(teamIdString, nameString)
+		cachedStore.Channel().GetByName(teamIdString, nameString, true)
+		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+}
+
 func TestChannelStoreChannelPinnedPostsCountsCache(t *testing.T) {
 	countResult := int64(10)

--- a/store/localcachelayer/layer.go
+++ b/store/localcachelayer/layer.go
@@ -35,6 +35,8 @@ const (
 	CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE
 	CHANNEL_MEMBERS_COUNTS_CACHE_SEC  = 30 * 60

+	CHANNEL_CACHE_SEC = 900 // 15 mins
+
 	LAST_POSTS_CACHE_SIZE = 20000
 	LAST_POSTS_CACHE_SEC  = 30 * 60

@@ -62,6 +64,7 @@ type LocalCacheStore struct {
 	emojiIdCacheByName           *utils.Cache
 	channel                      LocalCacheChannelStore
 	channelMemberCountsCache     *utils.Cache
+	channelByNameCache           *utils.Cache
 	channelGuestCountCache       *utils.Cache
 	channelPinnedPostCountsCache *utils.Cache
 	webhook                      LocalCacheWebhookStore
@@ -93,6 +96,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 	localCacheStore.emoji = LocalCacheEmojiStore{EmojiStore: baseStore.Emoji(), rootStore: &localCacheStore}
 	localCacheStore.channelPinnedPostCountsCache = utils.NewLruWithParams(CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SIZE, "ChannelPinnedPostsCounts", CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_PINNEDPOSTS_COUNTS)
 	localCacheStore.channelMemberCountsCache = utils.NewLruWithParams(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE, "ChannelMemberCounts", CHANNEL_MEMBERS_COUNTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS)
+	localCacheStore.channelByNameCache = utils.NewLruWithParams(CHANNEL_CACHE_SEC, "ChannelByName", CHANNEL_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_BY_NAME)
 	localCacheStore.channelGuestCountCache = utils.NewLruWithParams(CHANNEL_GUEST_COUNT_CACHE_SIZE, "ChannelGuestsCount", CHANNEL_GUEST_COUNT_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT)
 	localCacheStore.channel = LocalCacheChannelStore{ChannelStore: baseStore.Channel(), rootStore: &localCacheStore}
 	localCacheStore.postLastPostsCache = utils.NewLruWithParams(LAST_POSTS_CACHE_SIZE, "LastPost", LAST_POSTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS)
@@ -111,6 +115,7 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME, localCacheStore.emoji.handleClusterInvalidateEmojiIdByName)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_PINNEDPOSTS_COUNTS, localCacheStore.channel.handleClusterInvalidateChannelPinnedPostCount)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS, localCacheStore.channel.handleClusterInvalidateChannelMemberCounts)
+		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_BY_NAME, localCacheStore.channel.handleClusterInvalidateChannelByName)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_GUEST_COUNT, localCacheStore.channel.handleClusterInvalidateChannelGuestCounts)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_LAST_POSTS, localCacheStore.post.handleClusterInvalidateLastPosts)
 		cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_PROFILE_BY_IDS, localCacheStore.user.handleClusterInvalidateScheme)
@@ -209,6 +214,7 @@ func (s *LocalCacheStore) Invalidate() {
 	s.doClearCacheCluster(s.emojiCacheById)
 	s.doClearCacheCluster(s.emojiIdCacheByName)
 	s.doClearCacheCluster(s.channelMemberCountsCache)
+	s.doClearCacheCluster(s.channelByNameCache)
 	s.doClearCacheCluster(s.channelPinnedPostCountsCache)
 	s.doClearCacheCluster(s.channelGuestCountCache)
 	s.doClearCacheCluster(s.postLastPostsCache)
--- a/store/localcachelayer/main_test.go
+++ b/store/localcachelayer/main_test.go
@@ -58,11 +58,16 @@ func getMockStore() *mocks.Store {
 	mockStore.On("Emoji").Return(&mockEmojiStore)

 	mockCount := int64(10)
+	teamIdString := "teamID123"
+	nameString := "nameId987"
+	fakeChannel := model.Channel{Name: nameString, TeamId: teamIdString}
 	mockGuestCount := int64(12)
 	mockChannelStore := mocks.ChannelStore{}
 	mockChannelStore.On("ClearCaches").Return()
 	mockChannelStore.On("GetMemberCount", "id", true).Return(mockCount, nil)
 	mockChannelStore.On("GetMemberCount", "id", false).Return(mockCount, nil)
+	mockChannelStore.On("GetByName", teamIdString, nameString, true).Return(&fakeChannel, nil)
+	mockChannelStore.On("GetByName", teamIdString, nameString, false).Return(&fakeChannel, nil)
 	mockChannelStore.On("GetGuestCount", "id", true).Return(mockGuestCount, nil)
 	mockChannelStore.On("GetGuestCount", "id", false).Return(mockGuestCount, nil)
 	mockStore.On("Channel").Return(&mockChannelStore)
--- a/store/sqlstore/channel_store.go
+++ b/store/sqlstore/channel_store.go
@@ -277,19 +277,16 @@ type publicChannel struct {
 var allChannelMembersForUserCache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE)
 var allChannelMembersNotifyPropsForChannelCache = utils.NewLru(ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE)
 var channelCache = utils.NewLru(model.CHANNEL_CACHE_SIZE)
-var channelByNameCache = utils.NewLru(model.CHANNEL_CACHE_SIZE)

 func (s SqlChannelStore) ClearCaches() {
 	allChannelMembersForUserCache.Purge()
 	allChannelMembersNotifyPropsForChannelCache.Purge()
 	channelCache.Purge()
-	channelByNameCache.Purge()

 	if s.metrics != nil {
 		s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members for User - Purge")
 		s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members Notify Props for Channel - Purge")
 		s.metrics.IncrementMemCacheInvalidationCounter("Channel - Purge")
-		s.metrics.IncrementMemCacheInvalidationCounter("Channel By Name - Purge")
 	}
 }

@@ -671,10 +668,6 @@ func (s SqlChannelStore) InvalidateChannel(id string) {
 }

 func (s SqlChannelStore) InvalidateChannelByName(teamId, name string) {
-	channelByNameCache.Remove(teamId + name)
-	if s.metrics != nil {
-		s.metrics.IncrementMemCacheInvalidationCounter("Channel by Name - Remove by TeamId and Name")
-	}
 }

 func (s SqlChannelStore) Get(id string, allowFromCache bool) (*model.Channel, *model.AppError) {
@@ -1123,29 +1116,6 @@ func (s SqlChannelStore) GetByName(teamId string, name string, allowFromCache bo
 func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCache bool) ([]*model.Channel, *model.AppError) {
 	var channels []*model.Channel

-	if allowFromCache {
-		var misses []string
-		visited := make(map[string]struct{})
-		for _, name := range names {
-			if _, ok := visited[name]; ok {
-				continue
-			}
-			visited[name] = struct{}{}
-			if cacheItem, ok := channelByNameCache.Get(teamId + name); ok {
-				if s.metrics != nil {
-					s.metrics.IncrementMemCacheHitCounter("Channel By Name")
-				}
-				channels = append(channels, cacheItem.(*model.Channel))
-			} else {
-				if s.metrics != nil {
-					s.metrics.IncrementMemCacheMissCounter("Channel By Name")
-				}
-				misses = append(misses, name)
-			}
-		}
-		names = misses
-	}
-
 	if len(names) > 0 {
 		props := map[string]interface{}{}
 		var namePlaceholders []string
@@ -1163,14 +1133,9 @@ func (s SqlChannelStore) GetByNames(teamId string, names []string, allowFromCach
 			query = `SELECT * FROM Channels WHERE Name IN (` + strings.Join(namePlaceholders, ", ") + `) AND TeamId = :TeamId AND DeleteAt = 0`
 		}

-		var dbChannels []*model.Channel
-		if _, err := s.GetReplica().Select(&dbChannels, query, props); err != nil && err != sql.ErrNoRows {
+		if _, err := s.GetReplica().Select(&channels, query, props); err != nil && err != sql.ErrNoRows {
 			return nil, model.NewAppError("SqlChannelStore.GetByName", "store.sql_channel.get_by_name.existing.app_error", nil, "teamId="+teamId+", "+err.Error(), http.StatusInternalServerError)
 		}
-		for _, channel := range dbChannels {
-			channelByNameCache.AddWithExpiresInSecs(teamId+channel.Name, channel, CHANNEL_CACHE_SEC)
-			channels = append(channels, channel)
-		}
 	}

 	return channels, nil
@@ -1189,18 +1154,6 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo
 	}
 	channel := model.Channel{}

-	if allowFromCache {
-		if cacheItem, ok := channelByNameCache.Get(teamId + name); ok {
-			if s.metrics != nil {
-				s.metrics.IncrementMemCacheHitCounter("Channel By Name")
-			}
-			return cacheItem.(*model.Channel), nil
-		}
-		if s.metrics != nil {
-			s.metrics.IncrementMemCacheMissCounter("Channel By Name")
-		}
-	}
-
 	if err := s.GetReplica().SelectOne(&channel, query, map[string]interface{}{"TeamId": teamId, "Name": name}); err != nil {
 		if err == sql.ErrNoRows {
 			return nil, model.NewAppError("SqlChannelStore.GetByName", store.MISSING_CHANNEL_ERROR, nil, "teamId="+teamId+", "+"name="+name+", "+err.Error(), http.StatusNotFound)
@@ -1208,7 +1161,6 @@ func (s SqlChannelStore) getByName(teamId string, name string, includeDeleted bo
 		return nil, model.NewAppError("SqlChannelStore.GetByName", "store.sql_channel.get_by_name.existing.app_error", nil, "teamId="+teamId+", "+"name="+name+", "+err.Error(), http.StatusInternalServerError)
 	}

-	channelByNameCache.AddWithExpiresInSecs(teamId+name, &channel, CHANNEL_CACHE_SEC)
 	return &channel, nil
 }