From b162cf92cd637f041284c63baa3bfc4ebabe3e3f Mon Sep 17 00:00:00 2001 From: Mario de Frutos Dieguez Date: Wed, 4 Mar 2020 14:18:03 +0100 Subject: [PATCH] MM-21976: Include cache layer to be tested (#13749) --- api4/apitestlib.go | 32 ++++++++++++++++++++++---------- api4/user_test.go | 7 ------- app/authentication.go | 13 +++++++++++++ app/config_test.go | 3 ++- app/helper_test.go | 29 +++++++++++++++++++++-------- app/team_test.go | 1 + app/user.go | 7 +++++-- migrations/helper_test.go | 5 +++++ web/web_test.go | 25 +++++++++++++++++++------ 9 files changed, 88 insertions(+), 34 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index d1647d5d26..3b768aaec4 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -20,6 +20,7 @@ import ( "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/testlib" "github.com/mattermost/mattermost-server/v5/utils" @@ -52,11 +53,13 @@ type TestHelper struct { SystemAdminClient *model.Client4 SystemAdminUser *model.User tempWorkspace string + + IncludeCacheLayer bool } var mainHelper *testlib.MainHelper -func setupTestHelper(dbStore store.Store, enterprise bool, updateConfig func(*model.Config)) *TestHelper { +func setupTestHelper(dbStore store.Store, enterprise bool, includeCache bool, updateConfig func(*model.Config)) *TestHelper { tempWorkspace, err := ioutil.TempDir("", "apptest") if err != nil { panic(err) @@ -83,11 +86,16 @@ func setupTestHelper(dbStore store.Store, enterprise bool, updateConfig func(*mo 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: s.FakeApp(), - Server: s, - ConfigStore: memoryStore, + App: s.FakeApp(), + Server: s, + ConfigStore: memoryStore, + IncludeCacheLayer: includeCache, } th.App.UpdateConfig(func(cfg *model.Config) { @@ -151,7 +159,7 @@ func SetupEnterprise(tb testing.TB) *TestHelper { dbStore := mainHelper.GetStore() dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, true, nil) + return setupTestHelper(dbStore, true, true, nil) } func Setup(tb testing.TB) *TestHelper { @@ -166,7 +174,7 @@ func Setup(tb testing.TB) *TestHelper { dbStore := mainHelper.GetStore() dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, false, nil) + return setupTestHelper(dbStore, false, true, nil) } func SetupConfig(tb testing.TB, updateConfig func(cfg *model.Config)) *TestHelper { @@ -181,11 +189,11 @@ func SetupConfig(tb testing.TB, updateConfig func(cfg *model.Config)) *TestHelpe dbStore := mainHelper.GetStore() dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, false, updateConfig) + return setupTestHelper(dbStore, false, true, updateConfig) } func SetupConfigWithStoreMock(tb testing.TB, updateConfig func(cfg *model.Config)) *TestHelper { - th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), false, updateConfig) + th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), false, false, updateConfig) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -193,7 +201,7 @@ func SetupConfigWithStoreMock(tb testing.TB, updateConfig func(cfg *model.Config } func SetupWithStoreMock(tb testing.TB) *TestHelper { - th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), false, nil) + th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), false, false, nil) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -201,7 +209,7 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { } func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper { - th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), true, nil) + th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), true, false, nil) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -226,6 +234,10 @@ func (me *TestHelper) ShutdownApp() { func (me *TestHelper) TearDown() { utils.DisableDebugLogForTest() + if me.IncludeCacheLayer { + // Clean all the caches + me.App.InvalidateAllCaches() + } me.ShutdownApp() diff --git a/api4/user_test.go b/api4/user_test.go index 302ecad1a4..415dda8647 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -15,7 +15,6 @@ import ( "github.com/mattermost/mattermost-server/v5/app" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/services/mailservice" - "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/utils/testutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -4365,8 +4364,6 @@ func TestLoginLockout(t *testing.T) { func TestDemoteUserToGuest(t *testing.T) { t.Run("websocket update user event", func(t *testing.T) { th := Setup(t).InitBasic() - th.Server.Store = localcachelayer.NewLocalCacheLayer(th.Server.Store, - th.Server.Metrics, th.Server.Cluster, th.Server.CacheProvider) defer th.TearDown() user := th.BasicUser @@ -4413,15 +4410,12 @@ func TestDemoteUserToGuest(t *testing.T) { require.True(t, ok, "expected user") assert.Equal(t, "system_guest", eventUser.Roles) }) - th.App.InvalidateAllCaches() }) } func TestPromoteGuestToUser(t *testing.T) { t.Run("websocket update user event", func(t *testing.T) { th := Setup(t).InitBasic() - th.Server.Store = localcachelayer.NewLocalCacheLayer(th.Server.Store, - th.Server.Metrics, th.Server.Cluster, th.Server.CacheProvider) defer th.TearDown() user := th.BasicUser @@ -4469,6 +4463,5 @@ func TestPromoteGuestToUser(t *testing.T) { require.True(t, ok, "expected user") assert.Equal(t, "system_user", eventUser.Roles) }) - th.App.InvalidateAllCaches() }) } diff --git a/app/authentication.go b/app/authentication.go index 45dab8905e..1814df9ab2 100644 --- a/app/authentication.go +++ b/app/authentication.go @@ -54,6 +54,9 @@ func (a *App) CheckPasswordAndAllCriteria(user *model.User, password string, mfa if passErr := a.Srv().Store.User().UpdateFailedPasswordAttempts(user.Id, user.FailedAttempts+1); passErr != nil { return passErr } + + a.InvalidateCacheForUser(user.Id) + return err } @@ -65,6 +68,9 @@ func (a *App) CheckPasswordAndAllCriteria(user *model.User, password string, mfa return passErr } } + + a.InvalidateCacheForUser(user.Id) + return err } @@ -72,6 +78,8 @@ func (a *App) CheckPasswordAndAllCriteria(user *model.User, password string, mfa return passErr } + a.InvalidateCacheForUser(user.Id) + if err := a.CheckUserPostflightAuthenticationCriteria(user); err != nil { return err } @@ -89,6 +97,9 @@ func (a *App) DoubleCheckPassword(user *model.User, password string) *model.AppE if passErr := a.Srv().Store.User().UpdateFailedPasswordAttempts(user.Id, user.FailedAttempts+1); passErr != nil { return passErr } + + a.InvalidateCacheForUser(user.Id) + return err } @@ -96,6 +107,8 @@ func (a *App) DoubleCheckPassword(user *model.User, password string) *model.AppE return passErr } + a.InvalidateCacheForUser(user.Id) + return nil } diff --git a/app/config_test.go b/app/config_test.go index 637885a973..38fd0d6ca9 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/store/sqlstore" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/utils" @@ -125,7 +126,7 @@ func TestEnsureInstallationDate(t *testing.T) { for _, tc := range tt { t.Run(tc.Name, func(t *testing.T) { - sqlStore := th.App.Srv().Store.User().(*sqlstore.SqlUserStore) + sqlStore := th.App.Srv().Store.User().(localcachelayer.LocalCacheUserStore).UserStore.(*sqlstore.SqlUserStore) sqlStore.GetMaster().Exec("DELETE FROM Users") for _, createAt := range tc.UsersCreationDates { diff --git a/app/helper_test.go b/app/helper_test.go index dfa626319c..b8292c89ff 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -17,6 +17,7 @@ import ( "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/testlib" "github.com/mattermost/mattermost-server/v5/utils" @@ -35,9 +36,11 @@ type TestHelper struct { SystemAdminUser *model.User tempWorkspace string + + IncludeCacheLayer bool } -func setupTestHelper(dbStore store.Store, enterprise bool, tb testing.TB, configSet func(*model.Config)) *TestHelper { +func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, tb testing.TB, configSet func(*model.Config)) *TestHelper { tempWorkspace, err := ioutil.TempDir("", "apptest") if err != nil { panic(err) @@ -66,9 +69,15 @@ func setupTestHelper(dbStore store.Store, enterprise bool, tb testing.TB, config 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: s.FakeApp(), - Server: s, + App: s.FakeApp(), + Server: s, + IncludeCacheLayer: includeCacheLayer, } th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxUsersPerTeam = 50 }) @@ -113,7 +122,7 @@ func SetupEnterprise(tb testing.TB) *TestHelper { dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, true, tb, nil) + return setupTestHelper(dbStore, true, true, tb, nil) } func Setup(tb testing.TB) *TestHelper { @@ -124,12 +133,12 @@ func Setup(tb testing.TB) *TestHelper { dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, false, tb, nil) + return setupTestHelper(dbStore, false, true, tb, nil) } func SetupWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, false, tb, nil) + th := setupTestHelper(mockStore, false, false, tb, nil) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -138,7 +147,7 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, true, tb, nil) + th := setupTestHelper(mockStore, true, false, tb, nil) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -153,7 +162,7 @@ func SetupWithCustomConfig(tb testing.TB, configSet func(*model.Config)) *TestHe dbStore.DropAllTables() dbStore.MarkSystemRanUnitTests() - return setupTestHelper(dbStore, false, tb, configSet) + return setupTestHelper(dbStore, false, true, tb, configSet) } var initBasicOnce sync.Once @@ -543,6 +552,10 @@ func (me *TestHelper) ShutdownApp() { } func (me *TestHelper) TearDown() { + if me.IncludeCacheLayer { + // Clean all the caches + me.App.InvalidateAllCaches() + } me.ShutdownApp() if err := recover(); err != nil { panic(err) diff --git a/app/team_test.go b/app/team_test.go index 1ee25c0f78..07d29638fe 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -298,6 +298,7 @@ func TestAddUserToTeamByToken(t *testing.T) { guestEmail := rguest.Email rguest.Email = "test@restricted.com" _, err := th.App.Srv().Store.User().Update(rguest, false) + th.App.InvalidateCacheForUser(rguest.Id) require.Nil(t, err) require.Nil(t, th.App.Srv().Store.Token().Save(token)) _, err = th.App.AddUserToTeamByToken(rguest.Id, token.Token) diff --git a/app/user.go b/app/user.go index 0d19084a9c..bcc24b4baf 100644 --- a/app/user.go +++ b/app/user.go @@ -1279,6 +1279,8 @@ func (a *App) UpdatePassword(user *model.User, newPassword string) *model.AppErr return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, err.Error(), http.StatusInternalServerError) } + a.InvalidateCacheForUser(user.Id) + return nil } @@ -1681,11 +1683,12 @@ func (a *App) GetTotalUsersStats(viewRestrictions *model.ViewUsersRestrictions) } func (a *App) VerifyUserEmail(userId, email string) *model.AppError { - _, err := a.Srv().Store.User().VerifyEmail(userId, email) - if err != nil { + if _, err := a.Srv().Store.User().VerifyEmail(userId, email); err != nil { return err } + a.InvalidateCacheForUser(userId) + user, err := a.GetUser(userId) if err != nil { diff --git a/migrations/helper_test.go b/migrations/helper_test.go index 5eb3e812f1..931aca55f4 100644 --- a/migrations/helper_test.go +++ b/migrations/helper_test.go @@ -11,6 +11,7 @@ import ( "github.com/mattermost/mattermost-server/v5/config" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/utils" ) @@ -45,6 +46,8 @@ func setupTestHelper(enterprise bool) *TestHelper { if err != nil { panic(err) } + // Adds the cache layer to the test store + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) th := &TestHelper{ App: s.FakeApp(), @@ -244,6 +247,8 @@ func (me *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel) } func (me *TestHelper) TearDown() { + // Clean all the caches + me.App.InvalidateAllCaches() me.Server.Shutdown() if err := recover(); err != nil { panic(err) diff --git a/web/web_test.go b/web/web_test.go index 6fddeb9d13..6e04453eef 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -18,6 +18,7 @@ import ( "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin" "github.com/mattermost/mattermost-server/v5/store" + "github.com/mattermost/mattermost-server/v5/store/localcachelayer" "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" "github.com/mattermost/mattermost-server/v5/testlib" "github.com/mattermost/mattermost-server/v5/utils" @@ -40,6 +41,8 @@ type TestHelper struct { SystemAdminUser *model.User tempWorkspace string + + IncludeCacheLayer bool } func SetupWithStoreMock(tb testing.TB) *TestHelper { @@ -47,7 +50,7 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { tb.SkipNow() } store := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(tb, store) + th := setupTestHelper(tb, store, false) emptyMockStore := mocks.Store{} emptyMockStore.On("Close").Return(nil) th.App.Srv().Store = &emptyMockStore @@ -60,10 +63,10 @@ func Setup(tb testing.TB) *TestHelper { } store := mainHelper.GetStore() store.DropAllTables() - return setupTestHelper(tb, store) + return setupTestHelper(tb, store, true) } -func setupTestHelper(t testing.TB, store store.Store) *TestHelper { +func setupTestHelper(t testing.TB, store store.Store, includeCacheLayer bool) *TestHelper { memoryStore, err := config.NewMemoryStoreWithOptions(&config.MemoryStoreOptions{IgnoreEnvironmentOverrides: true}) if err != nil { panic("failed to initialize memory store: " + err.Error()) @@ -77,6 +80,11 @@ func setupTestHelper(t testing.TB, store store.Store) *TestHelper { if err != nil { panic(err) } + if includeCacheLayer { + // Adds the cache layer to the test store + s.Store = localcachelayer.NewLocalCacheLayer(s.Store, s.Metrics, s.Cluster, s.CacheProvider) + } + a := s.FakeApp() prevListenAddress := *a.Config().ServiceSettings.ListenAddress a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = ":0" }) @@ -108,9 +116,10 @@ func setupTestHelper(t testing.TB, store store.Store) *TestHelper { }) th := &TestHelper{ - App: a, - Server: s, - Web: web, + App: a, + Server: s, + Web: web, + IncludeCacheLayer: includeCacheLayer, } return th @@ -144,6 +153,10 @@ func (th *TestHelper) InitBasic() *TestHelper { } func (th *TestHelper) TearDown() { + if th.IncludeCacheLayer { + // Clean all the caches + th.App.InvalidateAllCaches() + } th.Server.Shutdown() if err := recover(); err != nil { panic(err)