MM-53669: Use the cache layer for EmojiStore.GetMultipleByName (#24030)

There was already a cache present for emoji names. But we weren't using
it for the GetMultipleByName method. Now we implement that method
to look up the cache for every emoji name passed.

Secondly, a bigger problem was that we were making the DB call for system
emojis as well. Since system emojis aren't stored in the DB, it would
fall through the cache layer and always make a redundant DB call. In the
profiles, this should up as taking 16% of the total time to serve
a getPostsForChannel API endpoint.

We fix this by filtering the emojis to only custom emojis
before making the call.

https://mattermost.atlassian.net/browse/MM-53669

```release-note
NONE
```

server/channels/app/emoji.go

@@ -191,7 +191,7 @@ func (a *App) GetEmoji(c request.CTX, emojiId string) (*model.Emoji, *model.AppE
 		return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusForbidden)
 	}
 
-	emoji, err := a.Srv().Store().Emoji().Get(context.Background(), emojiId, true)
+	emoji, err := a.Srv().Store().Emoji().Get(c.Context(), emojiId, true)
 	if err != nil {
 		var nfErr *store.ErrNotFound
 		switch {
@@ -214,7 +214,7 @@ func (a *App) GetEmojiByName(c request.CTX, emojiName string) (*model.Emoji, *mo
 		return nil, model.NewAppError("GetEmojiByName", "api.emoji.storage.app_error", nil, "", http.StatusForbidden)
 	}
 
-	emoji, err := a.Srv().Store().Emoji().GetByName(context.Background(), emojiName, true)
+	emoji, err := a.Srv().Store().Emoji().GetByName(c.Context(), emojiName, true)
 	if err != nil {
 		var nfErr *store.ErrNotFound
 		switch {
@@ -233,7 +233,21 @@ func (a *App) GetMultipleEmojiByName(c request.CTX, names []string) ([]*model.Em
 		return nil, model.NewAppError("GetMultipleEmojiByName", "api.emoji.disabled.app_error", nil, "", http.StatusForbidden)
 	}
 
-	emoji, err := a.Srv().Store().Emoji().GetMultipleByName(names)
+	// Filtering out system emojis
+	i := 0
+	for _, n := range names {
+		if _, ok := model.GetSystemEmojiId(n); !ok {
+			names[i] = n
+			i++
+		}
+	}
+	names = names[:i]
+
+	if len(names) == 0 {
+		return []*model.Emoji{}, nil
+	}
+
+	emoji, err := a.Srv().Store().Emoji().GetMultipleByName(c.Context(), names)
 	if err != nil {
 		return nil, model.NewAppError("GetMultipleEmojiByName", "app.emoji.get_by_name.app_error", nil, fmt.Sprintf("names=%v, %v", names, err.Error()), http.StatusInternalServerError)
 	}
@@ -242,7 +256,7 @@ func (a *App) GetMultipleEmojiByName(c request.CTX, names []string) ([]*model.Em
 }
 
 func (a *App) GetEmojiImage(c request.CTX, emojiId string) ([]byte, string, *model.AppError) {
-	_, storeErr := a.Srv().Store().Emoji().Get(context.Background(), emojiId, true)
+	_, storeErr := a.Srv().Store().Emoji().Get(c.Context(), emojiId, true)
 	if storeErr != nil {
 		var nfErr *store.ErrNotFound
 		switch {

server/channels/app/emoji_test.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+	"testing"
+
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetMultipleEmojiByName(t *testing.T) {
+	// The fact that we use mock store ensures that
+	// the call to the DB does not happen. If it did, we would have needed
+	// to provide the mock explicitly.
+	th := SetupWithStoreMock(t)
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.EnableCustomEmoji = true
+	})
+
+	// Ensure it returns empty for system emojis
+	emojis, appErr := th.App.GetMultipleEmojiByName(th.Context, []string{"+1"})
+	require.Nil(t, appErr)
+	assert.Empty(t, emojis)
+}

server/channels/app/post_metadata.go

@@ -438,7 +438,6 @@ func (a *App) getCustomEmojisForPost(c request.CTX, post *model.Post, reactions
 	}
 
 	names := getEmojiNamesForPost(post, reactions)
-
 	if len(names) == 0 {
 		return []*model.Emoji{}, nil
 	}

server/channels/store/localcachelayer/emoji_layer.go

@@ -90,12 +90,50 @@ func (es *LocalCacheEmojiStore) GetByName(ctx context.Context, name string, allo
 	es.emojiByNameMut.Unlock()
 
 	emoji, err := es.EmojiStore.GetByName(ctx, name, allowFromCache)
+	if err != nil {
+		return nil, err
+	}
 
-	if allowFromCache && err == nil {
+	if allowFromCache {
 		es.addToCache(emoji)
 	}
 
-	return emoji, err
+	return emoji, nil
+}
+
+func (es *LocalCacheEmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
+	emojis := []*model.Emoji{}
+	remainingEmojiNames := make([]string, 0)
+
+	for _, name := range names {
+		if emoji, ok := es.getFromCacheByName(name); ok {
+			emojis = append(emojis, emoji)
+		} else {
+			// If it was invalidated, then we need to query master.
+			es.emojiByNameMut.Lock()
+			if es.emojiByNameInvalidations[name] {
+				ctx = sqlstore.WithMaster(ctx)
+				// And then remove the key from the map.
+				delete(es.emojiByNameInvalidations, name)
+			}
+			es.emojiByNameMut.Unlock()
+
+			remainingEmojiNames = append(remainingEmojiNames, name)
+		}
+	}
+
+	if len(remainingEmojiNames) > 0 {
+		remainingEmojis, err := es.EmojiStore.GetMultipleByName(ctx, remainingEmojiNames)
+		if err != nil {
+			return nil, err
+		}
+		for _, emoji := range remainingEmojis {
+			es.addToCache(emoji)
+			emojis = append(emojis, emoji)
+		}
+	}
+
+	return emojis, nil
 }
 
 func (es *LocalCacheEmojiStore) Delete(emoji *model.Emoji, time int64) error {

server/channels/store/localcachelayer/emoji_layer_test.go

@@ -21,6 +21,7 @@ func TestEmojiStore(t *testing.T) {
 
 func TestEmojiStoreCache(t *testing.T) {
 	fakeEmoji := model.Emoji{Id: "123", Name: "name123"}
+	fakeEmoji2 := model.Emoji{Id: "321", Name: "name321"}
 	ctxEmoji := model.Emoji{Id: "master", Name: "name123"}
 
 	t.Run("first call by id not cached, second cached and returning same data", func(t *testing.T) {
@@ -39,7 +40,7 @@ func TestEmojiStoreCache(t *testing.T) {
 		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1)
 	})
 
-	t.Run("first call by name not cached, second cached and returning same data", func(t *testing.T) {
+	t.Run("GetByName: first call by name not cached, second cached and returning same data", func(t *testing.T) {
 		mockStore := getMockStore()
 		mockCacheProvider := getMockCacheProvider()
 		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
@@ -55,6 +56,43 @@ func TestEmojiStoreCache(t *testing.T) {
 		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1)
 	})
 
+	t.Run("GetMultipleByName: first call by name not cached, second cached and returning same data", func(t *testing.T) {
+		mockStore := getMockStore()
+		mockCacheProvider := getMockCacheProvider()
+		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
+		require.NoError(t, err)
+
+		emojis, err := cachedStore.Emoji().GetMultipleByName(context.Background(), []string{"name123"})
+		require.NoError(t, err)
+		require.Len(t, emojis, 1)
+		assert.Equal(t, emojis[0], &fakeEmoji)
+		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetMultipleByName", 1)
+		emojis, err = cachedStore.Emoji().GetMultipleByName(context.Background(), []string{"name123"})
+		require.NoError(t, err)
+		require.Len(t, emojis, 1)
+		assert.Equal(t, emojis[0], &fakeEmoji)
+		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetMultipleByName", 1)
+	})
+
+	t.Run("GetMultipleByName: multiple elements", func(t *testing.T) {
+		mockStore := getMockStore()
+		mockCacheProvider := getMockCacheProvider()
+		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
+		require.NoError(t, err)
+
+		emojis, err := cachedStore.Emoji().GetMultipleByName(context.Background(), []string{"name123", "name321"})
+		require.NoError(t, err)
+		require.Len(t, emojis, 2)
+		assert.Equal(t, emojis[0], &fakeEmoji)
+		assert.Equal(t, emojis[1], &fakeEmoji2)
+		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetMultipleByName", 1)
+		emojis, err = cachedStore.Emoji().GetMultipleByName(context.Background(), []string{"name123"})
+		require.NoError(t, err)
+		require.Len(t, emojis, 1)
+		assert.Equal(t, emojis[0], &fakeEmoji)
+		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetMultipleByName", 1)
+	})
+
 	t.Run("first call by id not cached, second force not cached", func(t *testing.T) {
 		mockStore := getMockStore()
 		mockCacheProvider := getMockCacheProvider()
@@ -107,7 +145,7 @@ func TestEmojiStoreCache(t *testing.T) {
 		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2)
 	})
 
-	t.Run("first call by id, second call by name cached", func(t *testing.T) {
+	t.Run("first call by id, second call by name and GetMultipleByName cached", func(t *testing.T) {
 		mockStore := getMockStore()
 		mockCacheProvider := getMockCacheProvider()
 		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider)
@@ -117,6 +155,8 @@ func TestEmojiStoreCache(t *testing.T) {
 		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1)
 		cachedStore.Emoji().GetByName(context.Background(), "name123", true)
 		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 0)
+		cachedStore.Emoji().GetMultipleByName(context.Background(), []string{"name123"})
+		mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetMultipleByName", 0)
 	})
 
 	t.Run("first call by name, second call by id cached", func(t *testing.T) {

server/channels/store/localcachelayer/main_test.go

@@ -69,6 +69,7 @@ func getMockStore() *mocks.Store {
 	mockStore.On("Webhook").Return(&mockWebhookStore)
 
 	fakeEmoji := model.Emoji{Id: "123", Name: "name123"}
+	fakeEmoji2 := model.Emoji{Id: "321", Name: "name321"}
 	ctxEmoji := model.Emoji{Id: "master", Name: "name123"}
 	mockEmojiStore := mocks.EmojiStore{}
 	mockEmojiStore.On("Get", mock.Anything, "123", true).Return(&fakeEmoji, nil)
@@ -77,6 +78,8 @@ func getMockStore() *mocks.Store {
 	mockEmojiStore.On("Get", sqlstore.WithMaster(context.Background()), "master", true).Return(&ctxEmoji, nil)
 	mockEmojiStore.On("GetByName", mock.Anything, "name123", true).Return(&fakeEmoji, nil)
 	mockEmojiStore.On("GetByName", mock.Anything, "name123", false).Return(&fakeEmoji, nil)
+	mockEmojiStore.On("GetMultipleByName", context.Background(), []string{"name123"}).Return([]*model.Emoji{&fakeEmoji}, nil)
+	mockEmojiStore.On("GetMultipleByName", context.Background(), []string{"name123", "name321"}).Return([]*model.Emoji{&fakeEmoji, &fakeEmoji2}, nil)
 	mockEmojiStore.On("GetByName", context.Background(), "master", true).Return(&ctxEmoji, nil)
 	mockEmojiStore.On("GetByName", sqlstore.WithMaster(context.Background()), "master", false).Return(&ctxEmoji, nil)
 	mockEmojiStore.On("Delete", &fakeEmoji, int64(0)).Return(nil)

server/channels/store/opentracinglayer/opentracinglayer.go

@@ -3415,7 +3415,7 @@ func (s *OpenTracingLayerEmojiStore) GetList(offset int, limit int, sort string)
 	return result, err
 }
 
-func (s *OpenTracingLayerEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) {
+func (s *OpenTracingLayerEmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
 	origCtx := s.Root.Store.Context()
 	span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "EmojiStore.GetMultipleByName")
 	s.Root.Store.SetContext(newCtx)
@@ -3424,7 +3424,7 @@ func (s *OpenTracingLayerEmojiStore) GetMultipleByName(names []string) ([]*model
 	}()
 
 	defer span.Finish()
-	result, err := s.EmojiStore.GetMultipleByName(names)
+	result, err := s.EmojiStore.GetMultipleByName(ctx, names)
 	if err != nil {
 		span.LogFields(spanlog.Error(err))
 		ext.Error.Set(span, true)

server/channels/store/retrylayer/retrylayer.go

@@ -3818,11 +3818,11 @@ func (s *RetryLayerEmojiStore) GetList(offset int, limit int, sort string) ([]*m
 
 }
 
-func (s *RetryLayerEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) {
+func (s *RetryLayerEmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
 
 	tries := 0
 	for {
-		result, err := s.EmojiStore.GetMultipleByName(names)
+		result, err := s.EmojiStore.GetMultipleByName(ctx, names)
 		if err == nil {
 			return result, nil
 		}

server/channels/store/sqlstore/emoji_store.go

@@ -52,13 +52,13 @@ func (es SqlEmojiStore) GetByName(ctx context.Context, name string, allowFromCac
 	return es.getBy(ctx, "Name", name)
 }
 
-func (es SqlEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) {
+func (es SqlEmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
 	// Creating (?, ?, ?) len(names) number of times.
 	keys := strings.Join(strings.Fields(strings.Repeat("? ", len(names))), ",")
 	args := makeStringArgs(names)
 
 	emojis := []*model.Emoji{}
-	if err := es.GetReplicaX().Select(&emojis,
+	if err := es.DBXFromContext(ctx).Select(&emojis,
 		`SELECT
 			*
 		FROM

server/channels/store/store.go

@@ -669,7 +669,7 @@ type EmojiStore interface {
 	Save(emoji *model.Emoji) (*model.Emoji, error)
 	Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error)
 	GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error)
-	GetMultipleByName(names []string) ([]*model.Emoji, error)
+	GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error)
 	GetList(offset, limit int, sort string) ([]*model.Emoji, error)
 	Delete(emoji *model.Emoji, timestamp int64) error
 	Search(name string, prefixOnly bool, limit int) ([]*model.Emoji, error)

server/channels/store/storetest/emoji_store.go

@@ -154,26 +154,26 @@ func testEmojiGetMultipleByName(t *testing.T, ss store.Store) {
 	}()
 
 	t.Run("one emoji", func(t *testing.T) {
-		received, err := ss.Emoji().GetMultipleByName([]string{emojis[0].Name})
+		received, err := ss.Emoji().GetMultipleByName(context.Background(), []string{emojis[0].Name})
 		require.NoError(t, err, "could not get emoji")
 		require.Len(t, received, 1, "got incorrect emoji")
 		require.Equal(t, *received[0], emojis[0], "got incorrect emoji")
 	})
 
 	t.Run("multiple emojis", func(t *testing.T) {
-		received, err := ss.Emoji().GetMultipleByName([]string{emojis[0].Name, emojis[1].Name, emojis[2].Name})
+		received, err := ss.Emoji().GetMultipleByName(context.Background(), []string{emojis[0].Name, emojis[1].Name, emojis[2].Name})
 		require.NoError(t, err, "could not get emojis")
 		require.Len(t, received, 3, "got incorrect emojis")
 	})
 
 	t.Run("one nonexistent emoji", func(t *testing.T) {
-		received, err := ss.Emoji().GetMultipleByName([]string{"ab"})
+		received, err := ss.Emoji().GetMultipleByName(context.Background(), []string{"ab"})
 		require.NoError(t, err, "could not get emoji", err)
 		require.Empty(t, received, "got incorrect emoji")
 	})
 
 	t.Run("multiple emojis with nonexistent names", func(t *testing.T) {
-		received, err := ss.Emoji().GetMultipleByName([]string{emojis[0].Name, emojis[1].Name, emojis[2].Name, "abcd", "1234"})
+		received, err := ss.Emoji().GetMultipleByName(context.Background(), []string{emojis[0].Name, emojis[1].Name, emojis[2].Name, "abcd", "1234"})
 		require.NoError(t, err, "could not get emojis")
 		require.Len(t, received, 3, "got incorrect emojis")
 	})

server/channels/store/storetest/mocks/EmojiStore.go

@@ -108,25 +108,25 @@ func (_m *EmojiStore) GetList(offset int, limit int, sort string) ([]*model.Emoj
 	return r0, r1
 }
 
-// GetMultipleByName provides a mock function with given fields: names
+// GetMultipleByName provides a mock function with given fields: ctx, names
-func (_m *EmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) {
+func (_m *EmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
-	ret := _m.Called(names)
+	ret := _m.Called(ctx, names)
 
 	var r0 []*model.Emoji
 	var r1 error
-	if rf, ok := ret.Get(0).(func([]string) ([]*model.Emoji, error)); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, []string) ([]*model.Emoji, error)); ok {
-		return rf(names)
+		return rf(ctx, names)
 	}
-	if rf, ok := ret.Get(0).(func([]string) []*model.Emoji); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, []string) []*model.Emoji); ok {
-		r0 = rf(names)
+		r0 = rf(ctx, names)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).([]*model.Emoji)
 		}
 	}
 
-	if rf, ok := ret.Get(1).(func([]string) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, []string) error); ok {
-		r1 = rf(names)
+		r1 = rf(ctx, names)
 	} else {
 		r1 = ret.Error(1)
 	}

server/channels/store/timerlayer/timerlayer.go

@@ -3128,10 +3128,10 @@ func (s *TimerLayerEmojiStore) GetList(offset int, limit int, sort string) ([]*m
 	return result, err
 }
 
-func (s *TimerLayerEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) {
+func (s *TimerLayerEmojiStore) GetMultipleByName(ctx context.Context, names []string) ([]*model.Emoji, error) {
 	start := time.Now()
 
-	result, err := s.EmojiStore.GetMultipleByName(names)
+	result, err := s.EmojiStore.GetMultipleByName(ctx, names)
 
 	elapsed := float64(time.Since(start)) / float64(time.Second)
 	if s.Root.Metrics != nil {