From 6dcbad049c7f39f324afd1895c50c150c31f357d Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 26 Feb 2021 22:17:51 +0530 Subject: [PATCH] MM-32693: Use master when emoji_layer is invalidated (#16951) We add a map which tracks invalidations to the emoji caches and use master whenever a fresh item is fetched from DB. https://mattermost.atlassian.net/browse/MM-32693 ```release-note NONE ``` Co-authored-by: Mattermod --- app/emoji.go | 11 ++-- app/import_functions.go | 2 +- app/import_functions_test.go | 4 +- store/localcachelayer/emoji_layer.go | 61 ++++++++++++++---- store/localcachelayer/emoji_layer_test.go | 72 +++++++++++++++------- store/localcachelayer/layer.go | 9 ++- store/localcachelayer/main_test.go | 16 +++-- store/opentracinglayer/opentracinglayer.go | 8 +-- store/retrylayer/retrylayer.go | 8 +-- store/sqlstore/emoji_store.go | 13 ++-- store/store.go | 4 +- store/storetest/emoji_store.go | 7 ++- store/storetest/mocks/EmojiStore.go | 30 ++++----- store/timerlayer/timerlayer.go | 8 +-- 14 files changed, 169 insertions(+), 84 deletions(-) diff --git a/app/emoji.go b/app/emoji.go index 458c23f916..5d1492f971 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -5,6 +5,7 @@ package app import ( "bytes" + "context" "errors" "fmt" "image" @@ -57,7 +58,7 @@ func (a *App) CreateEmoji(sessionUserId string, emoji *model.Emoji, multiPartIma return nil, model.NewAppError("createEmoji", "api.emoji.create.other_user.app_error", nil, "", http.StatusForbidden) } - if existingEmoji, err := a.Srv().Store.Emoji().GetByName(emoji.Name, true); err == nil && existingEmoji != nil { + if existingEmoji, err := a.Srv().Store.Emoji().GetByName(context.Background(), emoji.Name, true); err == nil && existingEmoji != nil { return nil, model.NewAppError("createEmoji", "api.emoji.create.duplicate.app_error", nil, "", http.StatusBadRequest) } @@ -185,7 +186,7 @@ func (a *App) GetEmoji(emojiId string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmoji", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - emoji, err := a.Srv().Store.Emoji().Get(emojiId, false) + emoji, err := a.Srv().Store.Emoji().Get(context.Background(), emojiId, false) if err != nil { var nfErr *store.ErrNotFound switch { @@ -208,7 +209,7 @@ func (a *App) GetEmojiByName(emojiName string) (*model.Emoji, *model.AppError) { return nil, model.NewAppError("GetEmojiByName", "api.emoji.storage.app_error", nil, "", http.StatusNotImplemented) } - emoji, err := a.Srv().Store.Emoji().GetByName(emojiName, true) + emoji, err := a.Srv().Store.Emoji().GetByName(context.Background(), emojiName, true) if err != nil { var nfErr *store.ErrNotFound switch { @@ -236,7 +237,7 @@ func (a *App) GetMultipleEmojiByName(names []string) ([]*model.Emoji, *model.App } func (a *App) GetEmojiImage(emojiId string) ([]byte, string, *model.AppError) { - _, storeErr := a.Srv().Store.Emoji().Get(emojiId, true) + _, storeErr := a.Srv().Store.Emoji().Get(context.Background(), emojiId, true) if storeErr != nil { var nfErr *store.ErrNotFound switch { @@ -282,7 +283,7 @@ func (a *App) GetEmojiStaticUrl(emojiName string) (string, *model.AppError) { return path.Join(subPath, "/static/emoji", id+".png"), nil } - emoji, err := a.Srv().Store.Emoji().GetByName(emojiName, true) + emoji, err := a.Srv().Store.Emoji().GetByName(context.Background(), emojiName, true) if err == nil { return path.Join(subPath, "/api/v4/emoji", emoji.Id, "image"), nil } diff --git a/app/import_functions.go b/app/import_functions.go index cd9d60b68a..81765ac831 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1735,7 +1735,7 @@ func (a *App) importEmoji(data *EmojiImportData, dryRun bool) *model.AppError { var emoji *model.Emoji - emoji, err := a.Srv().Store.Emoji().GetByName(*data.Name, true) + emoji, err := a.Srv().Store.Emoji().GetByName(context.Background(), *data.Name, true) if err != nil { var nfErr *store.ErrNotFound if !errors.As(err, &nfErr) { diff --git a/app/import_functions_test.go b/app/import_functions_test.go index d4f6beed2e..4b52fd0e63 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -3853,7 +3853,7 @@ func TestImportImportEmoji(t *testing.T) { err := th.App.importEmoji(&data, true) assert.NotNil(t, err, "Invalid emoji should have failed dry run") - emoji, nErr := th.App.Srv().Store.Emoji().GetByName(*data.Name, true) + emoji, nErr := th.App.Srv().Store.Emoji().GetByName(context.Background(), *data.Name, true) assert.Nil(t, emoji, "Emoji should not have been imported") assert.Error(t, nErr) @@ -3873,7 +3873,7 @@ func TestImportImportEmoji(t *testing.T) { err = th.App.importEmoji(&data, false) assert.Nil(t, err, "Valid emoji should have succeeded apply mode") - emoji, nErr = th.App.Srv().Store.Emoji().GetByName(*data.Name, true) + emoji, nErr = th.App.Srv().Store.Emoji().GetByName(context.Background(), *data.Name, true) assert.NotNil(t, emoji, "Emoji should have been imported") assert.NoError(t, nErr, "Emoji should have been imported without any error") diff --git a/store/localcachelayer/emoji_layer.go b/store/localcachelayer/emoji_layer.go index 80eec50fe0..cbc563b701 100644 --- a/store/localcachelayer/emoji_layer.go +++ b/store/localcachelayer/emoji_layer.go @@ -4,19 +4,30 @@ package localcachelayer import ( + "context" + "sync" + "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/sqlstore" ) type LocalCacheEmojiStore struct { store.EmojiStore - rootStore *LocalCacheStore + rootStore *LocalCacheStore + emojiByIdMut sync.Mutex + emojiByIdInvalidations map[string]bool + emojiByNameMut sync.Mutex + emojiByNameInvalidations map[string]bool } func (es *LocalCacheEmojiStore) handleClusterInvalidateEmojiById(msg *model.ClusterMessage) { if msg.Data == ClearCacheMessageData { es.rootStore.emojiCacheById.Purge() } else { + es.emojiByIdMut.Lock() + es.emojiByIdInvalidations[msg.Data] = true + es.emojiByIdMut.Unlock() es.rootStore.emojiCacheById.Remove(msg.Data) } } @@ -25,18 +36,30 @@ func (es *LocalCacheEmojiStore) handleClusterInvalidateEmojiIdByName(msg *model. if msg.Data == ClearCacheMessageData { es.rootStore.emojiIdCacheByName.Purge() } else { + es.emojiByNameMut.Lock() + es.emojiByNameInvalidations[msg.Data] = true + es.emojiByNameMut.Unlock() es.rootStore.emojiIdCacheByName.Remove(msg.Data) } } -func (es LocalCacheEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { +func (es *LocalCacheEmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { if allowFromCache { if emoji, ok := es.getFromCacheById(id); ok { return emoji, nil } } - emoji, err := es.EmojiStore.Get(id, allowFromCache) + // If it was invalidated, then we need to query master. + es.emojiByIdMut.Lock() + if es.emojiByIdInvalidations[id] { + // And then remove the key from the map. + ctx = sqlstore.WithMaster(ctx) + delete(es.emojiByIdInvalidations, id) + } + es.emojiByIdMut.Unlock() + + emoji, err := es.EmojiStore.Get(ctx, id, allowFromCache) if allowFromCache && err == nil { es.addToCache(emoji) @@ -45,9 +68,9 @@ func (es LocalCacheEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji return emoji, err } -func (es LocalCacheEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { +func (es *LocalCacheEmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { if id, ok := model.GetSystemEmojiId(name); ok { - return es.Get(id, allowFromCache) + return es.Get(ctx, id, allowFromCache) } if allowFromCache { @@ -56,7 +79,16 @@ func (es LocalCacheEmojiStore) GetByName(name string, allowFromCache bool) (*mod } } - emoji, err := es.EmojiStore.GetByName(name, allowFromCache) + // 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() + + emoji, err := es.EmojiStore.GetByName(ctx, name, allowFromCache) if allowFromCache && err == nil { es.addToCache(emoji) @@ -65,7 +97,7 @@ func (es LocalCacheEmojiStore) GetByName(name string, allowFromCache bool) (*mod return emoji, err } -func (es LocalCacheEmojiStore) Delete(emoji *model.Emoji, time int64) error { +func (es *LocalCacheEmojiStore) Delete(emoji *model.Emoji, time int64) error { err := es.EmojiStore.Delete(emoji, time) if err == nil { @@ -75,12 +107,12 @@ func (es LocalCacheEmojiStore) Delete(emoji *model.Emoji, time int64) error { return err } -func (es LocalCacheEmojiStore) addToCache(emoji *model.Emoji) { +func (es *LocalCacheEmojiStore) addToCache(emoji *model.Emoji) { es.rootStore.doStandardAddToCache(es.rootStore.emojiCacheById, emoji.Id, emoji) es.rootStore.doStandardAddToCache(es.rootStore.emojiIdCacheByName, emoji.Name, emoji.Id) } -func (es LocalCacheEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) { +func (es *LocalCacheEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) { var emoji *model.Emoji if err := es.rootStore.doStandardReadCache(es.rootStore.emojiCacheById, id, &emoji); err == nil { return emoji, true @@ -88,7 +120,7 @@ func (es LocalCacheEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) return nil, false } -func (es LocalCacheEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) { +func (es *LocalCacheEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) { var emojiId string if err := es.rootStore.doStandardReadCache(es.rootStore.emojiIdCacheByName, name, &emojiId); err == nil { return es.getFromCacheById(emojiId) @@ -96,7 +128,14 @@ func (es LocalCacheEmojiStore) getFromCacheByName(name string) (*model.Emoji, bo return nil, false } -func (es LocalCacheEmojiStore) removeFromCache(emoji *model.Emoji) { +func (es *LocalCacheEmojiStore) removeFromCache(emoji *model.Emoji) { + es.emojiByIdMut.Lock() + es.emojiByIdInvalidations[emoji.Id] = true + es.emojiByIdMut.Unlock() es.rootStore.doInvalidateCacheCluster(es.rootStore.emojiCacheById, emoji.Id) + + es.emojiByNameMut.Lock() + es.emojiByNameInvalidations[emoji.Name] = true + es.emojiByNameMut.Unlock() es.rootStore.doInvalidateCacheCluster(es.rootStore.emojiIdCacheByName, emoji.Name) } diff --git a/store/localcachelayer/emoji_layer_test.go b/store/localcachelayer/emoji_layer_test.go index b028d2f93b..012c787962 100644 --- a/store/localcachelayer/emoji_layer_test.go +++ b/store/localcachelayer/emoji_layer_test.go @@ -4,6 +4,7 @@ package localcachelayer import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -20,6 +21,7 @@ func TestEmojiStore(t *testing.T) { func TestEmojiStoreCache(t *testing.T) { fakeEmoji := model.Emoji{Id: "123", Name: "name123"} + 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) { mockStore := getMockStore() @@ -27,11 +29,11 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - emoji, err := cachedStore.Emoji().Get("123", true) + emoji, err := cachedStore.Emoji().Get(context.Background(), "123", true) require.NoError(t, err) assert.Equal(t, emoji, &fakeEmoji) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) - emoji, err = cachedStore.Emoji().Get("123", true) + emoji, err = cachedStore.Emoji().Get(context.Background(), "123", true) require.NoError(t, err) assert.Equal(t, emoji, &fakeEmoji) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) @@ -43,11 +45,11 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - emoji, err := cachedStore.Emoji().GetByName("name123", true) + emoji, err := cachedStore.Emoji().GetByName(context.Background(), "name123", true) require.NoError(t, err) assert.Equal(t, emoji, &fakeEmoji) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) - emoji, err = cachedStore.Emoji().GetByName("name123", true) + emoji, err = cachedStore.Emoji().GetByName(context.Background(), "name123", true) require.NoError(t, err) assert.Equal(t, emoji, &fakeEmoji) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) @@ -59,9 +61,9 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) - cachedStore.Emoji().Get("123", false) + cachedStore.Emoji().Get(context.Background(), "123", false) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) }) @@ -71,9 +73,9 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) - cachedStore.Emoji().GetByName("name123", false) + cachedStore.Emoji().GetByName(context.Background(), "name123", false) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) }) @@ -83,11 +85,11 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().Get("123", false) + cachedStore.Emoji().Get(context.Background(), "123", false) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) }) @@ -97,11 +99,11 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().GetByName("name123", false) + cachedStore.Emoji().GetByName(context.Background(), "name123", false) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) }) @@ -111,9 +113,9 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 0) }) @@ -123,9 +125,9 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 0) }) @@ -135,10 +137,23 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) cachedStore.Emoji().Delete(&fakeEmoji, 0) - cachedStore.Emoji().Get("123", true) + cachedStore.Emoji().Get(context.Background(), "123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) + }) + + t.Run("call by id, use master", func(t *testing.T) { + mockStore := getMockStore() + mockCacheProvider := getMockCacheProvider() + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) + require.NoError(t, err) + + cachedStore.Emoji().Get(context.Background(), "master", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + cachedStore.Emoji().Delete(&ctxEmoji, 0) + cachedStore.Emoji().Get(context.Background(), "master", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) }) @@ -148,10 +163,23 @@ func TestEmojiStoreCache(t *testing.T) { cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) require.NoError(t, err) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) cachedStore.Emoji().Delete(&fakeEmoji, 0) - cachedStore.Emoji().GetByName("name123", true) + cachedStore.Emoji().GetByName(context.Background(), "name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) + }) + + t.Run("call by name, use master", func(t *testing.T) { + mockStore := getMockStore() + mockCacheProvider := getMockCacheProvider() + cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider) + require.NoError(t, err) + + cachedStore.Emoji().GetByName(context.Background(), "master", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + cachedStore.Emoji().Delete(&ctxEmoji, 0) + cachedStore.Emoji().GetByName(context.Background(), "master", true) mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) }) } diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go index 196d6d3e46..513837778b 100644 --- a/store/localcachelayer/layer.go +++ b/store/localcachelayer/layer.go @@ -81,7 +81,7 @@ type LocalCacheStore struct { scheme LocalCacheSchemeStore schemeCache cache.Cache - emoji LocalCacheEmojiStore + emoji *LocalCacheEmojiStore emojiCacheById cache.Cache emojiIdCacheByName cache.Cache @@ -197,7 +197,12 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf }); err != nil { return } - localCacheStore.emoji = LocalCacheEmojiStore{EmojiStore: baseStore.Emoji(), rootStore: &localCacheStore} + localCacheStore.emoji = &LocalCacheEmojiStore{ + EmojiStore: baseStore.Emoji(), + rootStore: &localCacheStore, + emojiByIdInvalidations: make(map[string]bool), + emojiByNameInvalidations: make(map[string]bool), + } // Channels if localCacheStore.channelPinnedPostCountsCache, err = cacheProvider.NewCache(&cache.CacheOptions{ diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index d047e97539..fc842c430b 100644 --- a/store/localcachelayer/main_test.go +++ b/store/localcachelayer/main_test.go @@ -4,6 +4,7 @@ package localcachelayer import ( + "context" "fmt" "testing" @@ -14,6 +15,7 @@ import ( "github.com/mattermost/mattermost-server/v5/services/cache" cachemocks "github.com/mattermost/mattermost-server/v5/services/cache/mocks" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/sqlstore" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/testlib" ) @@ -68,12 +70,18 @@ func getMockStore() *mocks.Store { mockStore.On("Webhook").Return(&mockWebhookStore) fakeEmoji := model.Emoji{Id: "123", Name: "name123"} + ctxEmoji := model.Emoji{Id: "master", Name: "name123"} mockEmojiStore := mocks.EmojiStore{} - mockEmojiStore.On("Get", "123", true).Return(&fakeEmoji, nil) - mockEmojiStore.On("Get", "123", false).Return(&fakeEmoji, nil) - mockEmojiStore.On("GetByName", "name123", true).Return(&fakeEmoji, nil) - mockEmojiStore.On("GetByName", "name123", false).Return(&fakeEmoji, nil) + mockEmojiStore.On("Get", mock.Anything, "123", true).Return(&fakeEmoji, nil) + mockEmojiStore.On("Get", mock.Anything, "123", false).Return(&fakeEmoji, nil) + mockEmojiStore.On("Get", context.Background(), "master", true).Return(&ctxEmoji, nil) + 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("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) + mockEmojiStore.On("Delete", &ctxEmoji, int64(0)).Return(nil) mockStore.On("Emoji").Return(&mockEmojiStore) mockCount := int64(10) diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 91a46e6f77..652db3211f 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -2807,7 +2807,7 @@ func (s *OpenTracingLayerEmojiStore) Delete(emoji *model.Emoji, time int64) erro return err } -func (s *OpenTracingLayerEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { +func (s *OpenTracingLayerEmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "EmojiStore.Get") s.Root.Store.SetContext(newCtx) @@ -2816,7 +2816,7 @@ func (s *OpenTracingLayerEmojiStore) Get(id string, allowFromCache bool) (*model }() defer span.Finish() - result, err := s.EmojiStore.Get(id, allowFromCache) + result, err := s.EmojiStore.Get(ctx, id, allowFromCache) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) @@ -2825,7 +2825,7 @@ func (s *OpenTracingLayerEmojiStore) Get(id string, allowFromCache bool) (*model return result, err } -func (s *OpenTracingLayerEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { +func (s *OpenTracingLayerEmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "EmojiStore.GetByName") s.Root.Store.SetContext(newCtx) @@ -2834,7 +2834,7 @@ func (s *OpenTracingLayerEmojiStore) GetByName(name string, allowFromCache bool) }() defer span.Finish() - result, err := s.EmojiStore.GetByName(name, allowFromCache) + result, err := s.EmojiStore.GetByName(ctx, name, allowFromCache) if err != nil { span.LogFields(spanlog.Error(err)) ext.Error.Set(span, true) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index a6f48d8704..9b58f96b43 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -3004,11 +3004,11 @@ func (s *RetryLayerEmojiStore) Delete(emoji *model.Emoji, time int64) error { } -func (s *RetryLayerEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { +func (s *RetryLayerEmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { tries := 0 for { - result, err := s.EmojiStore.Get(id, allowFromCache) + result, err := s.EmojiStore.Get(ctx, id, allowFromCache) if err == nil { return result, nil } @@ -3024,11 +3024,11 @@ func (s *RetryLayerEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji } -func (s *RetryLayerEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { +func (s *RetryLayerEmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { tries := 0 for { - result, err := s.EmojiStore.GetByName(name, allowFromCache) + result, err := s.EmojiStore.GetByName(ctx, name, allowFromCache) if err == nil { return result, nil } diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 9f1dff38c6..3fac915413 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -4,6 +4,7 @@ package sqlstore import ( + "context" "database/sql" "fmt" @@ -57,12 +58,12 @@ func (es SqlEmojiStore) Save(emoji *model.Emoji) (*model.Emoji, error) { return emoji, nil } -func (es SqlEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { - return es.getBy("Id", id) +func (es SqlEmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { + return es.getBy(ctx, "Id", id) } -func (es SqlEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { - return es.getBy("Name", name) +func (es SqlEmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { + return es.getBy(ctx, "Name", name) } func (es SqlEmojiStore) GetMultipleByName(names []string) ([]*model.Emoji, error) { @@ -146,10 +147,10 @@ func (es SqlEmojiStore) Search(name string, prefixOnly bool, limit int) ([]*mode } // getBy returns one active (not deleted) emoji, found by any one column (what/key). -func (es SqlEmojiStore) getBy(what, key string) (*model.Emoji, error) { +func (es SqlEmojiStore) getBy(ctx context.Context, what, key string) (*model.Emoji, error) { var emoji *model.Emoji - err := es.GetReplica().SelectOne(&emoji, + err := es.DBFromContext(ctx).SelectOne(&emoji, `SELECT * FROM diff --git a/store/store.go b/store/store.go index 901eb28220..e0af55041d 100644 --- a/store/store.go +++ b/store/store.go @@ -546,8 +546,8 @@ type TokenStore interface { type EmojiStore interface { Save(emoji *model.Emoji) (*model.Emoji, error) - Get(id string, allowFromCache bool) (*model.Emoji, error) - GetByName(name string, allowFromCache bool) (*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) GetList(offset, limit int, sort string) ([]*model.Emoji, error) Delete(emoji *model.Emoji, time int64) error diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 6501f8ca66..dc35f704bb 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -4,6 +4,7 @@ package storetest import ( + "context" "testing" "time" @@ -80,12 +81,12 @@ func testEmojiGet(t *testing.T, ss store.Store) { }() for _, emoji := range emojis { - _, err := ss.Emoji().Get(emoji.Id, false) + _, err := ss.Emoji().Get(context.Background(), emoji.Id, false) require.NoErrorf(t, err, "failed to get emoji with id %v", emoji.Id) } for _, emoji := range emojis { - _, err := ss.Emoji().Get(emoji.Id, true) + _, err := ss.Emoji().Get(context.Background(), emoji.Id, true) require.NoErrorf(t, err, "failed to get emoji with id %v", emoji.Id) } } @@ -119,7 +120,7 @@ func testEmojiGetByName(t *testing.T, ss store.Store) { }() for _, emoji := range emojis { - _, err := ss.Emoji().GetByName(emoji.Name, true) + _, err := ss.Emoji().GetByName(context.Background(), emoji.Name, true) require.NoErrorf(t, err, "failed to get emoji with name %v", emoji.Name) } } diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index d0d45c6d08..75f0f02c26 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -5,6 +5,8 @@ package mocks import ( + context "context" + model "github.com/mattermost/mattermost-server/v5/model" mock "github.com/stretchr/testify/mock" ) @@ -28,13 +30,13 @@ func (_m *EmojiStore) Delete(emoji *model.Emoji, time int64) error { return r0 } -// Get provides a mock function with given fields: id, allowFromCache -func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { - ret := _m.Called(id, allowFromCache) +// Get provides a mock function with given fields: ctx, id, allowFromCache +func (_m *EmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { + ret := _m.Called(ctx, id, allowFromCache) var r0 *model.Emoji - if rf, ok := ret.Get(0).(func(string, bool) *model.Emoji); ok { - r0 = rf(id, allowFromCache) + if rf, ok := ret.Get(0).(func(context.Context, string, bool) *model.Emoji); ok { + r0 = rf(ctx, id, allowFromCache) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Emoji) @@ -42,8 +44,8 @@ func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) } var r1 error - if rf, ok := ret.Get(1).(func(string, bool) error); ok { - r1 = rf(id, allowFromCache) + if rf, ok := ret.Get(1).(func(context.Context, string, bool) error); ok { + r1 = rf(ctx, id, allowFromCache) } else { r1 = ret.Error(1) } @@ -51,13 +53,13 @@ func (_m *EmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) return r0, r1 } -// GetByName provides a mock function with given fields: name, allowFromCache -func (_m *EmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { - ret := _m.Called(name, allowFromCache) +// GetByName provides a mock function with given fields: ctx, name, allowFromCache +func (_m *EmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { + ret := _m.Called(ctx, name, allowFromCache) var r0 *model.Emoji - if rf, ok := ret.Get(0).(func(string, bool) *model.Emoji); ok { - r0 = rf(name, allowFromCache) + if rf, ok := ret.Get(0).(func(context.Context, string, bool) *model.Emoji); ok { + r0 = rf(ctx, name, allowFromCache) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.Emoji) @@ -65,8 +67,8 @@ func (_m *EmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, } var r1 error - if rf, ok := ret.Get(1).(func(string, bool) error); ok { - r1 = rf(name, allowFromCache) + if rf, ok := ret.Get(1).(func(context.Context, string, bool) error); ok { + r1 = rf(ctx, name, allowFromCache) } else { r1 = ret.Error(1) } diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index ff0bf6eaf9..9a8ecc666e 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -2575,10 +2575,10 @@ func (s *TimerLayerEmojiStore) Delete(emoji *model.Emoji, time int64) error { return err } -func (s *TimerLayerEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, error) { +func (s *TimerLayerEmojiStore) Get(ctx context.Context, id string, allowFromCache bool) (*model.Emoji, error) { start := timemodule.Now() - result, err := s.EmojiStore.Get(id, allowFromCache) + result, err := s.EmojiStore.Get(ctx, id, allowFromCache) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil { @@ -2591,10 +2591,10 @@ func (s *TimerLayerEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji return result, err } -func (s *TimerLayerEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, error) { +func (s *TimerLayerEmojiStore) GetByName(ctx context.Context, name string, allowFromCache bool) (*model.Emoji, error) { start := timemodule.Now() - result, err := s.EmojiStore.GetByName(name, allowFromCache) + result, err := s.EmojiStore.GetByName(ctx, name, allowFromCache) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil {