diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index 200987a802..f9c761647f 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -13,6 +13,7 @@ import ( "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/mailservice" "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/localcachelayer" "github.com/mattermost/mattermost-server/store/sqlstore" "github.com/mattermost/mattermost-server/utils" "github.com/pkg/errors" @@ -61,7 +62,7 @@ func (s *Server) RunOldAppInitalization() error { if s.FakeApp().Srv.newStore == nil { s.FakeApp().Srv.newStore = func() store.Store { - return store.NewTimerLayer(store.NewLayeredStore(sqlstore.NewSqlSupplier(s.FakeApp().Config().SqlSettings, s.Metrics), s.Metrics, s.Cluster), s.Metrics) + return store.NewTimerLayer(localcachelayer.NewLocalCacheLayer(store.NewLayeredStore(sqlstore.NewSqlSupplier(s.FakeApp().Config().SqlSettings, s.Metrics), s.Metrics, s.Cluster), s.Metrics, s.Cluster), s.Metrics) } } diff --git a/store/layered_store.go b/store/layered_store.go index 3837de8415..26c8f0c9f2 100644 --- a/store/layered_store.go +++ b/store/layered_store.go @@ -22,7 +22,6 @@ type LayeredStoreDatabaseLayer interface { type LayeredStore struct { TmpContext context.Context - ReactionStore ReactionStore RoleStore RoleStore SchemeStore SchemeStore DatabaseLayer LayeredStoreDatabaseLayer @@ -38,7 +37,6 @@ func NewLayeredStore(db LayeredStoreDatabaseLayer, metrics einterfaces.MetricsIn LocalCacheLayer: NewLocalCacheSupplier(metrics, cluster), } - store.ReactionStore = &LayeredReactionStore{store} store.RoleStore = &LayeredRoleStore{store} store.SchemeStore = &LayeredSchemeStore{store} @@ -143,7 +141,7 @@ func (s *LayeredStore) FileInfo() FileInfoStore { } func (s *LayeredStore) Reaction() ReactionStore { - return s.ReactionStore + return s.DatabaseLayer.Reaction() } func (s *LayeredStore) Job() JobStore { @@ -219,34 +217,6 @@ func (s *LayeredStore) TotalSearchDbConnections() int { return s.DatabaseLayer.TotalSearchDbConnections() } -type LayeredReactionStore struct { - *LayeredStore -} - -func (s *LayeredReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) { - return s.LayerChainHead.ReactionSave(s.TmpContext, reaction) -} - -func (s *LayeredReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) { - return s.LayerChainHead.ReactionDelete(s.TmpContext, reaction) -} - -func (s *LayeredReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) { - return s.LayerChainHead.ReactionGetForPost(s.TmpContext, postId) -} - -func (s *LayeredReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) { - return s.LayerChainHead.ReactionsBulkGetForPosts(s.TmpContext, postIds) -} - -func (s *LayeredReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError { - return s.LayerChainHead.ReactionDeleteAllWithEmojiName(s.TmpContext, emojiName) -} - -func (s *LayeredReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { - return s.LayerChainHead.ReactionPermanentDeleteBatch(s.TmpContext, endTime, limit) -} - type LayeredRoleStore struct { *LayeredStore } diff --git a/store/layered_store_supplier.go b/store/layered_store_supplier.go index 9d86d9c566..1bba88ca0e 100644 --- a/store/layered_store_supplier.go +++ b/store/layered_store_supplier.go @@ -21,16 +21,6 @@ type LayeredStoreSupplier interface { SetChainNext(LayeredStoreSupplier) Next() LayeredStoreSupplier - // - // Reactions - //), hints ...LayeredStoreHint) - ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) - ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) - ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) - ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError - ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) - ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) - // Roles RoleSave(ctx context.Context, role *model.Role, hints ...LayeredStoreHint) (*model.Role, *model.AppError) RoleGet(ctx context.Context, roleId string, hints ...LayeredStoreHint) (*model.Role, *model.AppError) diff --git a/store/local_cache_supplier.go b/store/local_cache_supplier.go index d85f4a229a..4f5667e5a8 100644 --- a/store/local_cache_supplier.go +++ b/store/local_cache_supplier.go @@ -12,9 +12,6 @@ import ( ) const ( - REACTION_CACHE_SIZE = 20000 - REACTION_CACHE_SEC = 30 * 60 - ROLE_CACHE_SIZE = 20000 ROLE_CACHE_SEC = 30 * 60 @@ -28,12 +25,11 @@ const ( ) type LocalCacheSupplier struct { - next LayeredStoreSupplier - reactionCache *utils.Cache - roleCache *utils.Cache - schemeCache *utils.Cache - metrics einterfaces.MetricsInterface - cluster einterfaces.ClusterInterface + next LayeredStoreSupplier + roleCache *utils.Cache + schemeCache *utils.Cache + metrics einterfaces.MetricsInterface + cluster einterfaces.ClusterInterface } // Caching Interface @@ -50,15 +46,13 @@ type ObjectCache interface { func NewLocalCacheSupplier(metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) *LocalCacheSupplier { supplier := &LocalCacheSupplier{ - reactionCache: utils.NewLruWithParams(REACTION_CACHE_SIZE, "Reaction", REACTION_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS), - roleCache: utils.NewLruWithParams(ROLE_CACHE_SIZE, "Role", ROLE_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES), - schemeCache: utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES), - metrics: metrics, - cluster: cluster, + roleCache: utils.NewLruWithParams(ROLE_CACHE_SIZE, "Role", ROLE_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES), + schemeCache: utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES), + metrics: metrics, + cluster: cluster, } if cluster != nil { - cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, supplier.handleClusterInvalidateReaction) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, supplier.handleClusterInvalidateRole) } @@ -128,7 +122,6 @@ func (s *LocalCacheSupplier) doClearCacheCluster(cache ObjectCache) { } func (s *LocalCacheSupplier) Invalidate() { - s.doClearCacheCluster(s.reactionCache) s.doClearCacheCluster(s.roleCache) s.doClearCacheCluster(s.schemeCache) } diff --git a/store/local_cache_supplier_reactions.go b/store/local_cache_supplier_reactions.go deleted file mode 100644 index d9af607d95..0000000000 --- a/store/local_cache_supplier_reactions.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package store - -import ( - "context" - - "github.com/mattermost/mattermost-server/model" -) - -func (s *LocalCacheSupplier) handleClusterInvalidateReaction(msg *model.ClusterMessage) { - if msg.Data == CLEAR_CACHE_MESSAGE_DATA { - s.reactionCache.Purge() - } else { - s.reactionCache.Remove(msg.Data) - } -} - -func (s *LocalCacheSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { - defer s.doInvalidateCacheCluster(s.reactionCache, reaction.PostId) - return s.Next().ReactionSave(ctx, reaction, hints...) -} - -func (s *LocalCacheSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { - defer s.doInvalidateCacheCluster(s.reactionCache, reaction.PostId) - return s.Next().ReactionDelete(ctx, reaction, hints...) -} - -func (s *LocalCacheSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - if result := s.doStandardReadCache(ctx, s.reactionCache, postId, hints...); result != nil { - return result.Data.([]*model.Reaction), nil - } - - reaction, err := s.Next().ReactionGetForPost(ctx, postId, hints...) - if err != nil { - return nil, err - } - - result := NewSupplierResult() - result.Data = reaction - s.doStandardAddToCache(ctx, s.reactionCache, postId, result, hints...) - - return reaction, nil -} - -func (s *LocalCacheSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError { - // This could be improved. Right now we just clear the whole - // cache because we don't have a way find what post Ids have this emoji name. - defer s.doClearCacheCluster(s.reactionCache) - return s.Next().ReactionDeleteAllWithEmojiName(ctx, emojiName, hints...) -} - -func (s *LocalCacheSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) { - // Don't bother to clear the cache as the posts will be gone anyway and the reactions being deleted will - // expire from the cache in due course. - return s.Next().ReactionPermanentDeleteBatch(ctx, endTime, limit) -} - -func (s *LocalCacheSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - // Ignoring this. - return s.Next().ReactionsBulkGetForPosts(ctx, postIds, hints...) -} diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go new file mode 100644 index 0000000000..9f369e1cd6 --- /dev/null +++ b/store/localcachelayer/layer.go @@ -0,0 +1,97 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "github.com/mattermost/mattermost-server/einterfaces" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/utils" +) + +const ( + REACTION_CACHE_SIZE = 20000 + REACTION_CACHE_SEC = 30 * 60 + + CLEAR_CACHE_MESSAGE_DATA = "" +) + +type LocalCacheStore struct { + store.Store + metrics einterfaces.MetricsInterface + cluster einterfaces.ClusterInterface + reaction LocalCacheReactionStore + reactionCache *utils.Cache +} + +func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) LocalCacheStore { + localCacheStore := LocalCacheStore{ + Store: baseStore, + cluster: cluster, + metrics: metrics, + } + localCacheStore.reactionCache = utils.NewLruWithParams(REACTION_CACHE_SIZE, "Reaction", REACTION_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS) + localCacheStore.reaction = LocalCacheReactionStore{ReactionStore: baseStore.Reaction(), rootStore: &localCacheStore} + + if cluster != nil { + cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, localCacheStore.reaction.handleClusterInvalidateReaction) + } + return localCacheStore +} + +func (s LocalCacheStore) Reaction() store.ReactionStore { + return s.reaction +} + +func (s LocalCacheStore) DropAllTables() { + s.Invalidate() + s.Store.DropAllTables() +} + +func (s *LocalCacheStore) doInvalidateCacheCluster(cache *utils.Cache, key string) { + cache.Remove(key) + if s.cluster != nil { + msg := &model.ClusterMessage{ + Event: cache.GetInvalidateClusterEvent(), + SendType: model.CLUSTER_SEND_BEST_EFFORT, + Data: key, + } + s.cluster.SendClusterMessage(msg) + } +} + +func (s *LocalCacheStore) doStandardAddToCache(cache *utils.Cache, key string, value interface{}) { + cache.AddWithDefaultExpires(key, value) +} + +func (s *LocalCacheStore) doStandardReadCache(cache *utils.Cache, key string) interface{} { + if cacheItem, ok := cache.Get(key); ok { + if s.metrics != nil { + s.metrics.IncrementMemCacheHitCounter(cache.Name()) + } + return cacheItem + } + + if s.metrics != nil { + s.metrics.IncrementMemCacheMissCounter(cache.Name()) + } + + return nil +} + +func (s *LocalCacheStore) doClearCacheCluster(cache *utils.Cache) { + cache.Purge() + if s.cluster != nil { + msg := &model.ClusterMessage{ + Event: cache.GetInvalidateClusterEvent(), + SendType: model.CLUSTER_SEND_BEST_EFFORT, + Data: CLEAR_CACHE_MESSAGE_DATA, + } + s.cluster.SendClusterMessage(msg) + } +} + +func (s *LocalCacheStore) Invalidate() { + s.doClearCacheCluster(s.reactionCache) +} diff --git a/store/localcachelayer/layer_test.go b/store/localcachelayer/layer_test.go new file mode 100644 index 0000000000..71f3fefa82 --- /dev/null +++ b/store/localcachelayer/layer_test.go @@ -0,0 +1,99 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "sync" + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/mattermost/mattermost-server/store/sqlstore" + "github.com/mattermost/mattermost-server/store/storetest" +) + +type storeType struct { + Name string + SqlSettings *model.SqlSettings + SqlSupplier *sqlstore.SqlSupplier + Store store.Store +} + +var storeTypes []*storeType + +func StoreTest(t *testing.T, f func(*testing.T, store.Store)) { + defer func() { + if err := recover(); err != nil { + tearDownStores() + panic(err) + } + }() + for _, st := range storeTypes { + st := st + t.Run(st.Name, func(t *testing.T) { f(t, st.Store) }) + } +} + +func StoreTestWithSqlSupplier(t *testing.T, f func(*testing.T, store.Store, storetest.SqlSupplier)) { + defer func() { + if err := recover(); err != nil { + tearDownStores() + panic(err) + } + }() + for _, st := range storeTypes { + st := st + t.Run(st.Name, func(t *testing.T) { f(t, st.Store, st.SqlSupplier) }) + } +} + +func initStores() { + storeTypes = append(storeTypes, &storeType{ + Name: "LocalCache+MySQL", + SqlSettings: storetest.MakeSqlSettings(model.DATABASE_DRIVER_MYSQL), + }) + storeTypes = append(storeTypes, &storeType{ + Name: "LocalCache+PostgreSQL", + SqlSettings: storetest.MakeSqlSettings(model.DATABASE_DRIVER_POSTGRES), + }) + + defer func() { + if err := recover(); err != nil { + tearDownStores() + panic(err) + } + }() + var wg sync.WaitGroup + for _, st := range storeTypes { + st := st + wg.Add(1) + go func() { + defer wg.Done() + st.SqlSupplier = sqlstore.NewSqlSupplier(*st.SqlSettings, nil) + st.Store = NewLocalCacheLayer(store.NewLayeredStore(st.SqlSupplier, nil, nil), nil, nil) + st.Store.DropAllTables() + st.Store.MarkSystemRanUnitTests() + }() + } + wg.Wait() +} + +var tearDownStoresOnce sync.Once + +func tearDownStores() { + tearDownStoresOnce.Do(func() { + var wg sync.WaitGroup + wg.Add(len(storeTypes)) + for _, st := range storeTypes { + st := st + go func() { + if st.Store != nil { + st.Store.Close() + } + wg.Done() + }() + } + wg.Wait() + }) +} diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go new file mode 100644 index 0000000000..cb0466c656 --- /dev/null +++ b/store/localcachelayer/main_test.go @@ -0,0 +1,21 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "testing" + + "github.com/mattermost/mattermost-server/testlib" +) + +var mainHelper *testlib.MainHelper + +func TestMain(m *testing.M) { + mainHelper = testlib.NewMainHelperWithOptions(nil) + defer mainHelper.Close() + + initStores() + mainHelper.Main(m) + tearDownStores() +} diff --git a/store/localcachelayer/reaction_layer.go b/store/localcachelayer/reaction_layer.go new file mode 100644 index 0000000000..19920ec02a --- /dev/null +++ b/store/localcachelayer/reaction_layer.go @@ -0,0 +1,58 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" +) + +type LocalCacheReactionStore struct { + store.ReactionStore + rootStore *LocalCacheStore +} + +func (s *LocalCacheReactionStore) handleClusterInvalidateReaction(msg *model.ClusterMessage) { + if msg.Data == CLEAR_CACHE_MESSAGE_DATA { + s.rootStore.reactionCache.Purge() + } else { + s.rootStore.reactionCache.Remove(msg.Data) + } +} + +func (s LocalCacheReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) { + defer s.rootStore.doInvalidateCacheCluster(s.rootStore.reactionCache, reaction.PostId) + return s.ReactionStore.Save(reaction) +} + +func (s LocalCacheReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) { + defer s.rootStore.doInvalidateCacheCluster(s.rootStore.reactionCache, reaction.PostId) + return s.ReactionStore.Delete(reaction) +} + +func (s LocalCacheReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) { + if !allowFromCache { + return s.ReactionStore.GetForPost(postId, false) + } + + if reaction := s.rootStore.doStandardReadCache(s.rootStore.reactionCache, postId); reaction != nil { + return reaction.([]*model.Reaction), nil + } + + reaction, err := s.ReactionStore.GetForPost(postId, false) + if err != nil { + return nil, err + } + + s.rootStore.doStandardAddToCache(s.rootStore.reactionCache, postId, reaction) + + return reaction, nil +} + +func (s LocalCacheReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError { + // This could be improved. Right now we just clear the whole + // cache because we don't have a way find what post Ids have this emoji name. + defer s.rootStore.doClearCacheCluster(s.rootStore.reactionCache) + return s.ReactionStore.DeleteAllWithEmojiName(emojiName) +} diff --git a/store/localcachelayer/reaction_layer_test.go b/store/localcachelayer/reaction_layer_test.go new file mode 100644 index 0000000000..b449b705dd --- /dev/null +++ b/store/localcachelayer/reaction_layer_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store/storetest" + "github.com/mattermost/mattermost-server/store/storetest/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReactionStore(t *testing.T) { + StoreTest(t, storetest.TestReactionStore) +} + +func TestReactionStoreCache(t *testing.T) { + fakeReaction := model.Reaction{PostId: "123"} + + t.Run("first call not cached, second cached and returning same data", func(t *testing.T) { + mockStore := mocks.Store{} + mockReactionsStore := mocks.ReactionStore{} + mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil) + mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil) + mockStore.On("Reaction").Return(&mockReactionsStore) + cachedStore := NewLocalCacheLayer(&mockStore, nil, nil) + + reaction, err := cachedStore.Reaction().GetForPost("123", true) + require.Nil(t, err) + assert.Equal(t, reaction, []*model.Reaction{&fakeReaction}) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1) + require.Nil(t, err) + assert.Equal(t, reaction, []*model.Reaction{&fakeReaction}) + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1) + }) + + t.Run("first call not cached, second force no cached", func(t *testing.T) { + mockStore := mocks.Store{} + mockReactionsStore := mocks.ReactionStore{} + mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil) + mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil) + mockStore.On("Reaction").Return(&mockReactionsStore) + cachedStore := NewLocalCacheLayer(&mockStore, nil, nil) + + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1) + cachedStore.Reaction().GetForPost("123", false) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2) + }) + + t.Run("first call not cached, save, and then not cached again", func(t *testing.T) { + mockStore := mocks.Store{} + mockReactionsStore := mocks.ReactionStore{} + mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil) + mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil) + mockStore.On("Reaction").Return(&mockReactionsStore) + cachedStore := NewLocalCacheLayer(&mockStore, nil, nil) + + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1) + cachedStore.Reaction().Save(&fakeReaction) + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2) + }) + + t.Run("first call not cached, delete, and then not cached again", func(t *testing.T) { + mockStore := mocks.Store{} + mockReactionsStore := mocks.ReactionStore{} + mockReactionsStore.On("Save", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("Delete", &fakeReaction).Return(&model.Reaction{}, nil) + mockReactionsStore.On("GetForPost", "123", false).Return([]*model.Reaction{&fakeReaction}, nil) + mockReactionsStore.On("GetForPost", "123", true).Return([]*model.Reaction{&fakeReaction}, nil) + mockStore.On("Reaction").Return(&mockReactionsStore) + cachedStore := NewLocalCacheLayer(&mockStore, nil, nil) + + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 1) + cachedStore.Reaction().Delete(&fakeReaction) + cachedStore.Reaction().GetForPost("123", true) + mockReactionsStore.AssertNumberOfCalls(t, "GetForPost", 2) + }) +} diff --git a/store/redis_supplier_reactions.go b/store/redis_supplier_reactions.go deleted file mode 100644 index b4b4b42645..0000000000 --- a/store/redis_supplier_reactions.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package store - -import ( - "context" - - "github.com/mattermost/mattermost-server/mlog" - "github.com/mattermost/mattermost-server/model" -) - -func (s *RedisSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { - if err := s.client.Del("reactions:" + reaction.PostId).Err(); err != nil { - mlog.Error("Redis failed to remove key reactions:" + reaction.PostId + " Error: " + err.Error()) - } - return s.Next().ReactionSave(ctx, reaction, hints...) -} - -func (s *RedisSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...LayeredStoreHint) (*model.Reaction, *model.AppError) { - if err := s.client.Del("reactions:" + reaction.PostId).Err(); err != nil { - mlog.Error("Redis failed to remove key reactions:" + reaction.PostId + " Error: " + err.Error()) - } - return s.Next().ReactionDelete(ctx, reaction, hints...) -} - -func (s *RedisSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - var reaction []*model.Reaction - found, err := s.load("reactions:"+postId, &reaction) - if found { - return reaction, nil - } - - if err != nil { - mlog.Error("Redis encountered an error on read: " + err.Error()) - } - - reaction, appErr := s.Next().ReactionGetForPost(ctx, postId, hints...) - if appErr != nil { - return nil, appErr - } - - if err := s.save("reactions:"+postId, reaction, REDIS_EXPIRY_TIME); err != nil { - mlog.Error("Redis encountered and error on write: " + err.Error()) - } - - return reaction, nil -} - -func (s *RedisSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...LayeredStoreHint) *model.AppError { - // Ignoring this. It's probably OK to have the emoji slowly expire from Redis. - return s.Next().ReactionDeleteAllWithEmojiName(ctx, emojiName, hints...) -} - -func (s *RedisSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...LayeredStoreHint) (int64, *model.AppError) { - // Ignoring this. It's probably OK to have the emoji slowly expire from Redis. - return s.Next().ReactionPermanentDeleteBatch(ctx, endTime, limit, hints...) -} - -func (s *RedisSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - // Ignoring this. - return s.Next().ReactionsBulkGetForPosts(ctx, postIds, hints...) -} diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 1eb8a17bf0..675b0029c9 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -151,9 +151,9 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.TermsOfService = NewSqlTermsOfServiceStore(supplier, metrics) supplier.oldStores.UserTermsOfService = NewSqlUserTermsOfServiceStore(supplier) supplier.oldStores.linkMetadata = NewSqlLinkMetadataStore(supplier) + supplier.oldStores.reaction = NewSqlReactionStore(supplier) supplier.oldStores.group = NewSqlGroupStore(supplier) - initSqlSupplierReactions(supplier) initSqlSupplierRoles(supplier) initSqlSupplierSchemes(supplier) diff --git a/store/sqlstore/supplier_reactions.go b/store/sqlstore/supplier_reactions.go index 77f076feb1..e607ed94c9 100644 --- a/store/sqlstore/supplier_reactions.go +++ b/store/sqlstore/supplier_reactions.go @@ -4,7 +4,6 @@ package sqlstore import ( - "context" "fmt" "net/http" @@ -14,16 +13,27 @@ import ( "github.com/mattermost/mattermost-server/store" ) -func initSqlSupplierReactions(sqlStore SqlStore) { +type SqlReactionStore struct { + SqlStore +} + +func NewSqlReactionStore(sqlStore SqlStore) store.ReactionStore { + s := &SqlReactionStore{sqlStore} + for _, db := range sqlStore.GetAllConns() { table := db.AddTableWithName(model.Reaction{}, "Reactions").SetKeys(false, "UserId", "PostId", "EmojiName") table.ColMap("UserId").SetMaxSize(26) table.ColMap("PostId").SetMaxSize(26) table.ColMap("EmojiName").SetMaxSize(64) } + + return s } -func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { +func (s SqlReactionStore) CreateIndexesIfNotExists() { +} + +func (s *SqlReactionStore) Save(reaction *model.Reaction) (*model.Reaction, *model.AppError) { reaction.PreSave() if err := reaction.IsValid(); err != nil { return nil, err @@ -49,7 +59,7 @@ func (s *SqlSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction return reaction, nil } -func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { +func (s *SqlReactionStore) Delete(reaction *model.Reaction) (*model.Reaction, *model.AppError) { transaction, err := s.GetMaster().Begin() if err != nil { return nil, model.NewAppError("SqlReactionStore.Delete", "store.sql_reaction.delete.begin.app_error", nil, err.Error(), http.StatusInternalServerError) @@ -68,7 +78,7 @@ func (s *SqlSupplier) ReactionDelete(ctx context.Context, reaction *model.Reacti return reaction, nil } -func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { +func (s *SqlReactionStore) GetForPost(postId string, allowFromCache bool) ([]*model.Reaction, *model.AppError) { var reactions []*model.Reaction if _, err := s.GetReplica().Select(&reactions, @@ -86,7 +96,7 @@ func (s *SqlSupplier) ReactionGetForPost(ctx context.Context, postId string, hin return reactions, nil } -func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { +func (s *SqlReactionStore) BulkGetForPosts(postIds []string) ([]*model.Reaction, *model.AppError) { keys, params := MapStringsToQueryParams(postIds, "postId") var reactions []*model.Reaction @@ -103,7 +113,7 @@ func (s *SqlSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []st return reactions, nil } -func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { +func (s *SqlReactionStore) DeleteAllWithEmojiName(emojiName string) *model.AppError { var reactions []*model.Reaction if _, err := s.GetReplica().Select(&reactions, @@ -138,7 +148,7 @@ func (s *SqlSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiN return nil } -func (s *SqlSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { +func (s *SqlReactionStore) PermanentDeleteBatch(endTime int64, limit int64) (int64, *model.AppError) { var query string if s.DriverName() == "postgres" { query = "DELETE from Reactions WHERE CreateAt = any (array (SELECT CreateAt FROM Reactions WHERE CreateAt < :EndTime LIMIT :Limit))" diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index 96f715c9dc..360a5cf0c8 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -368,187 +368,6 @@ func (_m *LayeredStoreDatabaseLayer) Reaction() store.ReactionStore { return r0 } -// ReactionDelete provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreDatabaseLayer) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, reaction) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { - r0 = rf(ctx, reaction, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, reaction, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionDeleteAllWithEmojiName provides a mock function with given fields: ctx, emojiName, hints -func (_m *LayeredStoreDatabaseLayer) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, emojiName) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { - r0 = rf(ctx, emojiName, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } - } - - return r0 -} - -// ReactionGetForPost provides a mock function with given fields: ctx, postId, hints -func (_m *LayeredStoreDatabaseLayer) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, postId) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 []*model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) []*model.Reaction); ok { - r0 = rf(ctx, postId, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, postId, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionPermanentDeleteBatch provides a mock function with given fields: ctx, endTime, limit, hints -func (_m *LayeredStoreDatabaseLayer) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, endTime, limit) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 int64 - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) int64); ok { - r0 = rf(ctx, endTime, limit, hints...) - } else { - r0 = ret.Get(0).(int64) - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, endTime, limit, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionSave provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreDatabaseLayer) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, reaction) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { - r0 = rf(ctx, reaction, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, reaction, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionsBulkGetForPosts provides a mock function with given fields: ctx, postIds, hints -func (_m *LayeredStoreDatabaseLayer) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, postIds) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 []*model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) []*model.Reaction); ok { - r0 = rf(ctx, postIds, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, []string, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, postIds, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - // Role provides a mock function with given fields: func (_m *LayeredStoreDatabaseLayer) Role() store.RoleStore { ret := _m.Called() diff --git a/store/storetest/mocks/LayeredStoreSupplier.go b/store/storetest/mocks/LayeredStoreSupplier.go index be946185ca..45682da8f9 100644 --- a/store/storetest/mocks/LayeredStoreSupplier.go +++ b/store/storetest/mocks/LayeredStoreSupplier.go @@ -30,187 +30,6 @@ func (_m *LayeredStoreSupplier) Next() store.LayeredStoreSupplier { return r0 } -// ReactionDelete provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreSupplier) ReactionDelete(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, reaction) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { - r0 = rf(ctx, reaction, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, reaction, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionDeleteAllWithEmojiName provides a mock function with given fields: ctx, emojiName, hints -func (_m *LayeredStoreSupplier) ReactionDeleteAllWithEmojiName(ctx context.Context, emojiName string, hints ...store.LayeredStoreHint) *model.AppError { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, emojiName) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { - r0 = rf(ctx, emojiName, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } - } - - return r0 -} - -// ReactionGetForPost provides a mock function with given fields: ctx, postId, hints -func (_m *LayeredStoreSupplier) ReactionGetForPost(ctx context.Context, postId string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, postId) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 []*model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, string, ...store.LayeredStoreHint) []*model.Reaction); ok { - r0 = rf(ctx, postId, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, string, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, postId, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionPermanentDeleteBatch provides a mock function with given fields: ctx, endTime, limit, hints -func (_m *LayeredStoreSupplier) ReactionPermanentDeleteBatch(ctx context.Context, endTime int64, limit int64, hints ...store.LayeredStoreHint) (int64, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, endTime, limit) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 int64 - if rf, ok := ret.Get(0).(func(context.Context, int64, int64, ...store.LayeredStoreHint) int64); ok { - r0 = rf(ctx, endTime, limit, hints...) - } else { - r0 = ret.Get(0).(int64) - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, int64, int64, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, endTime, limit, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionSave provides a mock function with given fields: ctx, reaction, hints -func (_m *LayeredStoreSupplier) ReactionSave(ctx context.Context, reaction *model.Reaction, hints ...store.LayeredStoreHint) (*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, reaction) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 *model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.Reaction); ok { - r0 = rf(ctx, reaction, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, *model.Reaction, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, reaction, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - -// ReactionsBulkGetForPosts provides a mock function with given fields: ctx, postIds, hints -func (_m *LayeredStoreSupplier) ReactionsBulkGetForPosts(ctx context.Context, postIds []string, hints ...store.LayeredStoreHint) ([]*model.Reaction, *model.AppError) { - _va := make([]interface{}, len(hints)) - for _i := range hints { - _va[_i] = hints[_i] - } - var _ca []interface{} - _ca = append(_ca, ctx, postIds) - _ca = append(_ca, _va...) - ret := _m.Called(_ca...) - - var r0 []*model.Reaction - if rf, ok := ret.Get(0).(func(context.Context, []string, ...store.LayeredStoreHint) []*model.Reaction); ok { - r0 = rf(ctx, postIds, hints...) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Reaction) - } - } - - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(context.Context, []string, ...store.LayeredStoreHint) *model.AppError); ok { - r1 = rf(ctx, postIds, hints...) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } - } - - return r0, r1 -} - // RoleDelete provides a mock function with given fields: ctx, roldId, hints func (_m *LayeredStoreSupplier) RoleDelete(ctx context.Context, roldId string, hints ...store.LayeredStoreHint) (*model.Role, *model.AppError) { _va := make([]interface{}, len(hints))