From 221f70a6cb4645d660df5339ab1921fd497bd7df Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 17 Nov 2020 21:13:35 +0530 Subject: [PATCH] MM-29487: Fix racy TestMuteCommandSpecificChannel (#16290) The code was doing `go fakeApp.UpdateProductNotices()` due to which the Store object was accessed in a racy manner when it was reset if the localcachelayer flag was set. We fix this by using the StoreOverride functionality to pre-define what store to use while initializing the server itself so that there isn't a need to reset the store _after_ the server is set. We also apply the same fix in the app and api4 layers too to prevent similar things from happening there. https://mattermost.atlassian.net/browse/MM-29487 ```release-note NONE ``` Co-authored-by: Mattermod --- api4/apitestlib.go | 13 ++++++++----- app/helper_test.go | 14 ++++++++------ app/slashcommands/helper_test.go | 13 +++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index e9d9e9efb8..aa12985465 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -99,16 +99,19 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent var options []app.Option options = append(options, app.ConfigStore(configStore)) - options = append(options, app.StoreOverride(dbStore)) + if includeCache { + // Adds the cache layer to the test store + options = append(options, app.StoreOverride(func(s *app.Server) store.Store { + return localcachelayer.NewLocalCacheLayer(dbStore, s.Metrics, s.Cluster, s.CacheProvider) + })) + } else { + options = append(options, app.StoreOverride(dbStore)) + } s, err := app.NewServer(options...) if err != nil { panic(err) } - if includeCache { - // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) - } th := &TestHelper{ App: app.New(app.ServerConnector(s)), diff --git a/app/helper_test.go b/app/helper_test.go index 93027727e0..af451bd9c5 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -65,7 +65,14 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo var options []Option options = append(options, ConfigStore(configStore)) - options = append(options, StoreOverride(dbStore)) + if includeCacheLayer { + // Adds the cache layer to the test store + options = append(options, StoreOverride(func(s *Server) store.Store { + return localcachelayer.NewLocalCacheLayer(dbStore, s.Metrics, s.Cluster, s.CacheProvider) + })) + } else { + options = append(options, StoreOverride(dbStore)) + } options = append(options, SetLogger(mlog.NewTestingLogger(tb, buffer))) s, err := NewServer(options...) @@ -73,11 +80,6 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo panic(err) } - if includeCacheLayer { - // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) - } - th := &TestHelper{ App: New(ServerConnector(s)), Server: s, diff --git a/app/slashcommands/helper_test.go b/app/slashcommands/helper_test.go index 9e86283b80..c7a9f226f7 100644 --- a/app/slashcommands/helper_test.go +++ b/app/slashcommands/helper_test.go @@ -60,7 +60,13 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo var options []app.Option options = append(options, app.ConfigStore(memoryStore)) - options = append(options, app.StoreOverride(dbStore)) + if includeCacheLayer { + options = append(options, app.StoreOverride(func(s *app.Server) store.Store { + return localcachelayer.NewLocalCacheLayer(dbStore, s.Metrics, s.Cluster, s.CacheProvider) + })) + } else { + options = append(options, app.StoreOverride(dbStore)) + } options = append(options, app.SetLogger(mlog.NewTestingLogger(tb, buffer))) s, err := app.NewServer(options...) @@ -68,11 +74,6 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo panic(err) } - if includeCacheLayer { - // Adds the cache layer to the test store - s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) - } - th := &TestHelper{ App: app.New(app.ServerConnector(s)), Server: s,