From a2a78577e9fa042d629be2bc67abcc5d0061d365 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 8 Mar 2022 16:28:28 -0400 Subject: [PATCH] Revert "[MM-41576] Revamp database schema version (#19586)" (#19746) * Revert "[MM-41576] Revamp database schema version (#19586)" This reverts commit 645fee3fe3b3cba74948511fd55a581b927ebd35. * Revert "MM-42049 - license endpoint not working (#19686)" This reverts commit 4fe89e5847d460d22d05c845215d828092c468ac. --- api4/config_test.go | 2 +- api4/system.go | 26 ----- api4/system_local.go | 1 - api4/system_test.go | 17 --- app/app_iface.go | 1 - app/app_test.go | 1 - app/channel_test.go | 3 - app/config.go | 5 - app/config_test.go | 1 - app/enterprise_test.go | 1 - app/helper_test.go | 2 + app/license.go | 47 ++++----- app/notification_push_test.go | 5 - app/opentracing/opentracing_layer.go | 22 ---- app/plugin_signature_test.go | 1 - app/post_test.go | 1 - app/product_notices_test.go | 1 - app/server.go | 15 +-- app/server_test.go | 4 +- app/slashcommands/helper_test.go | 8 +- app/team_test.go | 2 - app/web_hub_test.go | 1 - cmd/mattermost/commands/db.go | 41 -------- config/client.go | 1 + i18n/en.json | 4 - jobs/jobs_test.go | 6 +- jobs/jobs_watcher.go | 5 +- jobs/schedulers.go | 26 ++++- jobs/schedulers_test.go | 12 +-- jobs/server.go | 25 +---- jobs/server_test.go | 115 ++++++++++++++++----- jobs/workers.go | 17 +++ model/client4.go | 13 --- model/system.go | 5 - store/opentracinglayer/opentracinglayer.go | 4 + store/retrylayer/retrylayer.go | 4 + store/sqlstore/store.go | 28 +---- store/sqlstore/store_test.go | 73 ------------- store/sqlstore/upgrade.go | 12 +-- store/sqlstore/upgrade_test.go | 66 ++---------- store/store.go | 3 +- store/storetest/mocks/Store.go | 42 ++------ store/storetest/store.go | 11 +- store/timerlayer/timerlayer.go | 4 + testlib/store.go | 2 - web/context_test.go | 1 - web/handlers_test.go | 3 - 47 files changed, 215 insertions(+), 475 deletions(-) diff --git a/api4/config_test.go b/api4/config_test.go index ba715f4fbac..e0630598fa9 100644 --- a/api4/config_test.go +++ b/api4/config_test.go @@ -577,7 +577,7 @@ func TestGetOldClientConfig(t *testing.T) { config, _, err := client.GetOldClientConfig("") require.NoError(t, err) - require.Empty(t, config["Version"], "config not returned correctly") + require.NotEmpty(t, config["Version"], "config not returned correctly") require.Empty(t, config["GoogleDeveloperKey"], "config should be missing developer key") }) diff --git a/api4/system.go b/api4/system.go index 3675004218f..398913c27d9 100644 --- a/api4/system.go +++ b/api4/system.go @@ -70,7 +70,6 @@ func (api *API) InitSystem() { api.BaseRoutes.System.Handle("/support_packet", api.APISessionRequired(generateSupportPacket)).Methods("GET") api.BaseRoutes.System.Handle("/onboarding/complete", api.APISessionRequired(getOnboarding)).Methods("GET") api.BaseRoutes.System.Handle("/onboarding/complete", api.APISessionRequired(completeOnboarding)).Methods("POST") - api.BaseRoutes.System.Handle("/schema/version", api.APISessionRequired(getAppliedSchemaMigrations)).Methods("GET") } func generateSupportPacket(c *Context, w http.ResponseWriter, r *http.Request) { @@ -936,28 +935,3 @@ func completeOnboarding(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.Success() ReturnStatusOK(w) } - -func getAppliedSchemaMigrations(c *Context, w http.ResponseWriter, r *http.Request) { - if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { - c.Err = model.NewAppError("getAppliedMigrations", "app.system.applied_migrations.not_authorized", nil, "", http.StatusForbidden) - return - } - - auditRec := c.MakeAuditRecord("getAppliedSchemaMigrations", audit.Fail) - defer c.LogAuditRec(auditRec) - - migrations, appErr := c.App.GetAppliedSchemaMigrations() - if appErr != nil { - c.Err = appErr - return - } - - js, jsonErr := json.Marshal(migrations) - if jsonErr != nil { - c.Err = model.NewAppError("getAppliedMigrations", "api.marshal_error", nil, jsonErr.Error(), http.StatusInternalServerError) - return - } - - w.Write(js) - auditRec.Success() -} diff --git a/api4/system_local.go b/api4/system_local.go index 3b316f0f53d..872ac386092 100644 --- a/api4/system_local.go +++ b/api4/system_local.go @@ -18,7 +18,6 @@ func (api *API) InitSystemLocal() { api.BaseRoutes.APIRoot.Handle("/server_busy", api.APILocal(getServerBusyExpires)).Methods("GET") api.BaseRoutes.APIRoot.Handle("/server_busy", api.APILocal(clearServerBusy)).Methods("DELETE") api.BaseRoutes.APIRoot.Handle("/integrity", api.APILocal(localCheckIntegrity)).Methods("POST") - api.BaseRoutes.System.Handle("/schema/version", api.APILocal(getAppliedSchemaMigrations)).Methods("GET") } func localCheckIntegrity(c *Context, w http.ResponseWriter, r *http.Request) { diff --git a/api4/system_test.go b/api4/system_test.go index 30805a519f2..aeddc477c6c 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -909,20 +909,3 @@ func TestCompleteOnboarding(t *testing.T) { }) } - -func TestGetAppliedSchemaMigrations(t *testing.T) { - th := Setup(t) - defer th.TearDown() - - t.Run("as a regular user", func(t *testing.T) { - _, resp, err := th.Client.GetAppliedSchemaMigrations() - require.Error(t, err) - CheckForbiddenStatus(t, resp) - }) - - th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { - _, resp, err := c.GetAppliedSchemaMigrations() - require.NoError(t, err) - CheckOKStatus(t, resp) - }) -} diff --git a/app/app_iface.go b/app/app_iface.go index ead90216889..80bc7158aca 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -550,7 +550,6 @@ type AppIface interface { GetAllTeamsPage(offset int, limit int, opts *model.TeamSearch) ([]*model.Team, *model.AppError) GetAllTeamsPageWithCount(offset int, limit int, opts *model.TeamSearch) (*model.TeamsWithCount, *model.AppError) GetAnalytics(name string, teamID string) (model.AnalyticsRows, *model.AppError) - GetAppliedSchemaMigrations() ([]model.AppliedMigration, *model.AppError) GetAudits(userID string, limit int) (model.Audits, *model.AppError) GetAuditsPage(userID string, page int, perPage int) (model.Audits, *model.AppError) GetAuthorizationCode(w http.ResponseWriter, r *http.Request, service string, props map[string]string, loginHint string) (string, *model.AppError) diff --git a/app/app_test.go b/app/app_test.go index 2a47e03b6e7..525bb106671 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -56,7 +56,6 @@ func TestUnitUpdateConfig(t *testing.T) { mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) mockStore.On("License").Return(&mockLicenseStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) prev := *th.App.Config().ServiceSettings.SiteURL diff --git a/app/channel_test.go b/app/channel_test.go index e48837ae6e8..45fbbee47c5 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -2103,7 +2103,6 @@ func TestMarkChannelAsUnreadFromPostPanic(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("System").Return(&mockSystemStore) mockStore.On("License").Return(&mockLicenseStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true @@ -2133,7 +2132,6 @@ func TestClearChannelMembersCache(t *testing.T) { ChannelId: "1", }}, nil) mockStore.On("Channel").Return(&mockChannelStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.ClearChannelMembersCache("channelID") } @@ -2154,7 +2152,6 @@ func TestGetMemberCountsByGroup(t *testing.T) { } mockChannelStore.On("GetMemberCountsByGroup", context.Background(), "channelID", true).Return(cmc, nil) mockStore.On("Channel").Return(&mockChannelStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) resp, err := th.App.GetMemberCountsByGroup(context.Background(), "channelID", true) require.Nil(t, err) require.ElementsMatch(t, cmc, resp) diff --git a/app/config.go b/app/config.go index 3a22a86e740..b7033d01a35 100644 --- a/app/config.go +++ b/app/config.go @@ -420,11 +420,6 @@ func (a *App) ClientConfigWithComputed() map[string]string { if installationDate, err := a.ch.srv.getSystemInstallDate(); err == nil { respCfg["InstallationDate"] = strconv.FormatInt(installationDate, 10) } - if ver, err := a.ch.srv.Store.GetDBSchemaVersion(); err != nil { - mlog.Error("Could not get the schema version", mlog.Err(err)) - } else { - respCfg["Version"] = strconv.Itoa(ver) - } return respCfg } diff --git a/app/config_test.go b/app/config_test.go index d276d34f5b2..f24eda326a2 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -79,7 +79,6 @@ func TestClientConfigWithComputed(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) config := th.App.ClientConfigWithComputed() _, ok := config["NoAccounts"] diff --git a/app/enterprise_test.go b/app/enterprise_test.go index be2e6d87de1..d01b6079dda 100644 --- a/app/enterprise_test.go +++ b/app/enterprise_test.go @@ -77,7 +77,6 @@ func TestSAMLSettings(t *testing.T) { mockSystemStore.On("GetByName", "UpgradedFromTE").Return(&model.System{Name: "UpgradedFromTE", Value: "false"}, nil) mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil) mockSystemStore.On("GetByName", "FirstServerRunTimestamp").Return(&model.System{Name: "FirstServerRunTimestamp", Value: "10"}, nil) - mockStore.On("GetDBSchemaVersion").Return(1, nil) mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) diff --git a/app/helper_test.go b/app/helper_test.go index 58063b9a10d..90d6893cd47 100644 --- a/app/helper_test.go +++ b/app/helper_test.go @@ -134,6 +134,8 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo if enterprise { th.App.Srv().SetLicense(model.NewTestLicense()) + th.App.Srv().Jobs.InitWorkers() + th.App.Srv().Jobs.InitSchedulers() } else { th.App.Srv().SetLicense(nil) } diff --git a/app/license.go b/app/license.go index 779728f9143..f6e9bafc109 100644 --- a/app/license.go +++ b/app/license.go @@ -177,34 +177,6 @@ func (s *Server) SaveLicense(licenseBytes []byte) (*model.License, *model.AppErr return nil, model.NewAppError("addLicense", model.ExpiredLicenseError, nil, "", http.StatusBadRequest) } - if *s.Config().JobSettings.RunJobs && s.Jobs != nil { - if err := s.Jobs.StopWorkers(); err != nil && !errors.Is(err, jobs.ErrWorkersNotRunning) { - mlog.Warn("Stopping job server workers failed", mlog.Err(err)) - } - } - - if *s.Config().JobSettings.RunScheduler && s.Jobs != nil { - if err := s.Jobs.StopSchedulers(); err != nil && !errors.Is(err, jobs.ErrSchedulersNotRunning) { - mlog.Error("Stopping job server schedulers failed", mlog.Err(err)) - } - } - - defer func() { - // restart job server workers - this handles the edge case where a license file is uploaded, but the job server - // doesn't start until the server is restarted, which prevents the 'run job now' buttons in system console from - // functioning as expected - if *s.Config().JobSettings.RunJobs && s.Jobs != nil { - if err := s.Jobs.StartWorkers(); err != nil { - mlog.Error("Starting job server workers failed", mlog.Err(err)) - } - } - if *s.Config().JobSettings.RunScheduler && s.Jobs != nil { - if err := s.Jobs.StartSchedulers(); err != nil && !errors.Is(err, jobs.ErrSchedulersRunning) { - mlog.Error("Starting job server schedulers failed", mlog.Err(err)) - } - } - }() - if ok := s.SetLicense(&license); !ok { return nil, model.NewAppError("addLicense", model.ExpiredLicenseError, nil, "", http.StatusBadRequest) } @@ -236,6 +208,25 @@ func (s *Server) SaveLicense(licenseBytes []byte) (*model.License, *model.AppErr s.ReloadConfig() s.InvalidateAllCaches() + // restart job server workers - this handles the edge case where a license file is uploaded, but the job server + // doesn't start until the server is restarted, which prevents the 'run job now' buttons in system console from + // functioning as expected + if *s.Config().JobSettings.RunJobs && s.Jobs != nil { + if err := s.Jobs.StopWorkers(); err != nil && !errors.Is(err, jobs.ErrWorkersNotRunning) { + mlog.Warn("Stopping job server workers failed", mlog.Err(err)) + } + if err := s.Jobs.InitWorkers(); err != nil { + mlog.Error("Initializing job server workers failed", mlog.Err(err)) + } else if err := s.Jobs.StartWorkers(); err != nil { + mlog.Error("Starting job server workers failed", mlog.Err(err)) + } + } + if *s.Config().JobSettings.RunScheduler && s.Jobs != nil { + if err := s.Jobs.StartSchedulers(); err != nil && !errors.Is(err, jobs.ErrSchedulersRunning) { + mlog.Error("Starting job server schedulers failed", mlog.Err(err)) + } + } + return &license, nil } diff --git a/app/notification_push_test.go b/app/notification_push_test.go index 769ace98b77..867e2ef1367 100644 --- a/app/notification_push_test.go +++ b/app/notification_push_test.go @@ -566,7 +566,6 @@ func TestGetPushNotificationMessage(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) for name, tc := range map[string]struct { Message string @@ -1149,7 +1148,6 @@ func TestClearPushNotificationSync(t *testing.T) { mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) mockStore.On("Session").Return(&mockSessionStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.EmailSettings.PushNotificationServer = pushServer.URL @@ -1223,7 +1221,6 @@ func TestUpdateMobileAppBadgeSync(t *testing.T) { mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) mockStore.On("Session").Return(&mockSessionStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.EmailSettings.PushNotificationServer = pushServer.URL @@ -1290,7 +1287,6 @@ func TestSendAckToPushProxy(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.EmailSettings.PushNotificationServer = pushServer.URL @@ -1532,7 +1528,6 @@ func BenchmarkPushNotificationThroughput(b *testing.B) { mockStore.On("System").Return(&mockSystemStore) mockStore.On("Session").Return(&mockSessionStore) mockStore.On("Preference").Return(&mockPreferenceStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) // create 50 users, each having 2 sessions. type userSession struct { diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index ee59baa2df1..4e3d0f4c3e4 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -4604,28 +4604,6 @@ func (a *OpenTracingAppLayer) GetAnalytics(name string, teamID string) (model.An return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetAppliedSchemaMigrations() ([]model.AppliedMigration, *model.AppError) { - origCtx := a.ctx - span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAppliedSchemaMigrations") - - a.ctx = newCtx - a.app.Srv().Store.SetContext(newCtx) - defer func() { - a.app.Srv().Store.SetContext(origCtx) - a.ctx = origCtx - }() - - defer span.Finish() - resultVar0, resultVar1 := a.app.GetAppliedSchemaMigrations() - - if resultVar1 != nil { - span.LogFields(spanlog.Error(resultVar1)) - ext.Error.Set(span, true) - } - - return resultVar0, resultVar1 -} - func (a *OpenTracingAppLayer) GetAudits(userID string, limit int) (model.Audits, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetAudits") diff --git a/app/plugin_signature_test.go b/app/plugin_signature_test.go index f1b0d31d1d7..8b96de69861 100644 --- a/app/plugin_signature_test.go +++ b/app/plugin_signature_test.go @@ -34,7 +34,6 @@ func TestPluginPublicKeys(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) path, _ := fileutils.FindDir("tests") publicKeyFilename := "test-public-key.plugin.gpg" diff --git a/app/post_test.go b/app/post_test.go index b49e6e32f54..ebad76739f5 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -464,7 +464,6 @@ func TestImageProxy(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" diff --git a/app/product_notices_test.go b/app/product_notices_test.go index 8403634dc5e..6091409cf37 100644 --- a/app/product_notices_test.go +++ b/app/product_notices_test.go @@ -32,7 +32,6 @@ func TestNoticeValidation(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("Preference").Return(&mockPreferenceStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) mockSystemStore.On("SaveOrUpdate", &model.System{Name: "ActiveLicenseId", Value: ""}).Return(nil) mockSystemStore.On("GetByName", "UpgradedFromTE").Return(&model.System{Name: "UpgradedFromTE", Value: "false"}, nil) mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil) diff --git a/app/server.go b/app/server.go index 5b243cd8671..90f58f6fa93 100644 --- a/app/server.go +++ b/app/server.go @@ -20,7 +20,6 @@ import ( "os/exec" "path" "runtime" - "strconv" "strings" "sync" "sync/atomic" @@ -753,8 +752,8 @@ func (s *Server) Channels() *Channels { // Return Database type (postgres or mysql) and current version of Mattermost func (s *Server) DatabaseTypeAndMattermostVersion() (string, string) { - schemaVersion, _ := s.Store.GetDBSchemaVersion() - return *s.Config().SqlSettings.DriverName, strconv.Itoa(schemaVersion) + mattermostVersion, _ := s.Store.System().GetByName("Version") + return *s.Config().SqlSettings.DriverName, mattermostVersion.Value } // initLogging initializes and configures the logger(s). This may be called more than once. @@ -1880,6 +1879,8 @@ func (ch *Channels) ClientConfigHash() string { func (s *Server) initJobs() { s.Jobs = jobs.NewJobServer(s, s.Store, s.Metrics) + s.Jobs.InitWorkers() + s.Jobs.InitSchedulers() if jobsDataRetentionJobInterface != nil { builder := jobsDataRetentionJobInterface(s) @@ -2303,11 +2304,3 @@ func runDNDStatusExpireJob(a *App) { } }) } - -func (a *App) GetAppliedSchemaMigrations() ([]model.AppliedMigration, *model.AppError) { - table, err := a.Srv().Store.GetAppliedMigrations() - if err != nil { - return nil, model.NewAppError("GetDBSchemaTable", "api.file.read_file.app_error", nil, err.Error(), http.StatusInternalServerError) - } - return table, nil -} diff --git a/app/server_test.go b/app/server_test.go index ab2142a70c4..515f76b4ee4 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -235,7 +235,7 @@ func TestDatabaseTypeAndMattermostVersion(t *testing.T) { databaseType, mattermostVersion := th.Server.DatabaseTypeAndMattermostVersion() assert.Equal(t, "postgres", databaseType) - assert.GreaterOrEqual(t, mattermostVersion, strconv.Itoa(1)) + assert.Equal(t, "5.31.0", mattermostVersion) os.Setenv("MM_SQLSETTINGS_DRIVERNAME", "mysql") @@ -244,7 +244,7 @@ func TestDatabaseTypeAndMattermostVersion(t *testing.T) { databaseType, mattermostVersion = th2.Server.DatabaseTypeAndMattermostVersion() assert.Equal(t, "mysql", databaseType) - assert.GreaterOrEqual(t, mattermostVersion, strconv.Itoa(1)) + assert.Equal(t, "5.31.0", mattermostVersion) } func TestGenerateSupportPacket(t *testing.T) { diff --git a/app/slashcommands/helper_test.go b/app/slashcommands/helper_test.go index 56ba662f7f6..66d1777d2c8 100644 --- a/app/slashcommands/helper_test.go +++ b/app/slashcommands/helper_test.go @@ -125,13 +125,9 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo }) if enterprise { - th.App.Srv().Jobs.StopWorkers() - th.App.Srv().Jobs.StopSchedulers() - th.App.Srv().SetLicense(model.NewTestLicense()) - - th.App.Srv().Jobs.StartWorkers() - th.App.Srv().Jobs.StartSchedulers() + th.App.Srv().Jobs.InitWorkers() + th.App.Srv().Jobs.InitSchedulers() } else { th.App.Srv().SetLicense(nil) } diff --git a/app/team_test.go b/app/team_test.go index 964cb10c4e2..eab7512ed19 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -882,7 +882,6 @@ func TestLeaveTeamPanic(t *testing.T) { mockStore.On("System").Return(&mockSystemStore) mockStore.On("License").Return(&mockLicenseStore) mockStore.On("Team").Return(&mockTeamStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) team := &model.Team{Id: "myteam"} user := &model.User{Id: "userID"} @@ -1240,7 +1239,6 @@ func TestClearTeamMembersCache(t *testing.T) { TeamId: "1", }}, nil) mockStore.On("Team").Return(&mockTeamStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.ClearTeamMembersCache("teamID") } diff --git a/app/web_hub_test.go b/app/web_hub_test.go index 7031434c858..423a800053a 100644 --- a/app/web_hub_test.go +++ b/app/web_hub_test.go @@ -161,7 +161,6 @@ func TestHubSessionRevokeRace(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) userService, err := users.New(users.ServiceConfig{ UserStore: &mockUserStore, diff --git a/cmd/mattermost/commands/db.go b/cmd/mattermost/commands/db.go index dfe8c81b7b0..8501b35d489 100644 --- a/cmd/mattermost/commands/db.go +++ b/cmd/mattermost/commands/db.go @@ -5,7 +5,6 @@ package commands import ( "fmt" - "strconv" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -52,21 +51,13 @@ var MigrateCmd = &cobra.Command{ RunE: migrateCmdF, } -var DBVersionCmd = &cobra.Command{ - Use: "version", - Short: "Returns the recent applied version number", - RunE: dbVersionCmdF, -} - func init() { ResetCmd.Flags().Bool("confirm", false, "Confirm you really want to delete everything and a DB backup has been performed.") - DBVersionCmd.Flags().Bool("all", false, "Returns all applied migrations") DbCmd.AddCommand( InitDbCmd, ResetCmd, MigrateCmd, - DBVersionCmd, ) RootCmd.AddCommand( @@ -146,35 +137,3 @@ func migrateCmdF(command *cobra.Command, args []string) error { return nil } - -func dbVersionCmdF(command *cobra.Command, args []string) error { - cfgDSN := getConfigDSN(command, config.GetEnvironment()) - cfgStore, err := config.NewStoreFromDSN(cfgDSN, true, nil, true) - if err != nil { - return errors.Wrap(err, "failed to load configuration") - } - config := cfgStore.Get() - - store := sqlstore.New(config.SqlSettings, nil) - defer store.Close() - - allFlag, _ := command.Flags().GetBool("all") - if allFlag { - applied, err2 := store.GetAppliedMigrations() - if err2 != nil { - return errors.Wrap(err2, "failed to get applied migrations") - } - for _, migration := range applied { - CommandPrettyPrintln(fmt.Sprintf("Varsion: %d, Name: %s", migration.Version, migration.Name)) - } - return nil - } - - v, err := store.GetDBSchemaVersion() - if err != nil { - return errors.Wrap(err, "failed to get schema version") - } - CommandPrettyPrintln("Current database schema version is: " + strconv.Itoa(v)) - - return nil -} diff --git a/config/client.go b/config/client.go index 5e83ffa115b..c1918a14267 100644 --- a/config/client.go +++ b/config/client.go @@ -215,6 +215,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li func GenerateLimitedClientConfig(c *model.Config, telemetryID string, license *model.License) map[string]string { props := make(map[string]string) + props["Version"] = model.CurrentVersion props["BuildNumber"] = model.BuildNumber props["BuildDate"] = model.BuildDate props["BuildHash"] = model.BuildHash diff --git a/i18n/en.json b/i18n/en.json index 92dbe47b307..d7febf48ed7 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -6155,10 +6155,6 @@ "id": "app.submit_interactive_dialog.json_error", "translation": "Encountered an error encoding JSON for the interactive dialog." }, - { - "id": "app.system.applied_migrations.not_authorized", - "translation": "You don't have the appropriate permissions." - }, { "id": "app.system.complete_onboarding_request.app_error", "translation": "Failed to decode the complete onboarding request." diff --git a/jobs/jobs_test.go b/jobs/jobs_test.go index bb28db69a8a..fb17d45b6c1 100644 --- a/jobs/jobs_test.go +++ b/jobs/jobs_test.go @@ -28,11 +28,7 @@ func makeJobServer(t *testing.T) (*JobServer, *storetest.Store, *mocks.MetricsIn mockMetrics.AssertExpectations(t) }) - jobServer := &JobServer{ - ConfigService: configService, - Store: mockStore, - metrics: mockMetrics, - } + jobServer := NewJobServer(configService, mockStore, mockMetrics) return jobServer, mockStore, mockMetrics } diff --git a/jobs/jobs_watcher.go b/jobs/jobs_watcher.go index e0f57a5cf65..d8fcf71d557 100644 --- a/jobs/jobs_watcher.go +++ b/jobs/jobs_watcher.go @@ -26,6 +26,8 @@ type Watcher struct { func (srv *JobServer) MakeWatcher(workers *Workers, pollingInterval int) *Watcher { return &Watcher{ + stop: make(chan struct{}), + stopped: make(chan struct{}), pollingInterval: pollingInterval, workers: workers, srv: srv, @@ -34,8 +36,7 @@ func (srv *JobServer) MakeWatcher(workers *Workers, pollingInterval int) *Watche func (watcher *Watcher) Start() { mlog.Debug("Watcher Started") - watcher.stop = make(chan struct{}) - watcher.stopped = make(chan struct{}) + // Delay for some random number of milliseconds before starting to ensure that multiple // instances of the jobserver don't poll at a time too close to each other. rand.Seed(time.Now().UTC().UnixNano()) diff --git a/jobs/schedulers.go b/jobs/schedulers.go index f692b08451c..01aca21baac 100644 --- a/jobs/schedulers.go +++ b/jobs/schedulers.go @@ -32,6 +32,30 @@ var ( ErrSchedulersUninitialized = errors.New("job schedulers are not initialized") ) +func (srv *JobServer) InitSchedulers() error { + srv.mut.Lock() + defer srv.mut.Unlock() + if srv.schedulers != nil && srv.schedulers.running { + return ErrSchedulersRunning + } + mlog.Debug("Initialising schedulers.") + + schedulers := &Schedulers{ + stop: make(chan bool), + stopped: make(chan bool), + configChanged: make(chan *model.Config), + clusterLeaderChanged: make(chan bool, 1), + jobs: srv, + isLeader: true, + schedulers: make(map[string]model.Scheduler), + nextRunTimes: make(map[string]*time.Time), + } + + srv.schedulers = schedulers + + return nil +} + func (schedulers *Schedulers) AddScheduler(name string, scheduler model.Scheduler) { schedulers.schedulers[name] = scheduler } @@ -39,8 +63,6 @@ func (schedulers *Schedulers) AddScheduler(name string, scheduler model.Schedule // Start starts the schedulers. This call is not safe for concurrent use. // Synchronization should be implemented by the caller. func (schedulers *Schedulers) Start() { - schedulers.stop = make(chan bool) - schedulers.stopped = make(chan bool) schedulers.listenerId = schedulers.jobs.ConfigService.AddConfigListener(schedulers.handleConfigChange) go func() { diff --git a/jobs/schedulers_test.go b/jobs/schedulers_test.go index 8421d4f0f48..12f68a81b36 100644 --- a/jobs/schedulers_test.go +++ b/jobs/schedulers_test.go @@ -61,7 +61,7 @@ func TestScheduler(t *testing.T) { }, } - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.RegisterJobType(model.JobTypeDataRetention, nil, new(MockScheduler)) jobServer.RegisterJobType(model.JobTypeMessageExport, nil, new(MockScheduler)) @@ -77,7 +77,7 @@ func TestScheduler(t *testing.T) { }) t.Run("ClusterLeaderChanged", func(t *testing.T) { - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.StartSchedulers() time.Sleep(time.Second) jobServer.HandleClusterLeaderChange(false) @@ -89,7 +89,7 @@ func TestScheduler(t *testing.T) { }) t.Run("ClusterLeaderChangedBeforeStart", func(t *testing.T) { - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.HandleClusterLeaderChange(false) jobServer.StartSchedulers() time.Sleep(time.Second) @@ -100,7 +100,7 @@ func TestScheduler(t *testing.T) { }) t.Run("DoubleClusterLeaderChangedBeforeStart", func(t *testing.T) { - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.HandleClusterLeaderChange(false) jobServer.HandleClusterLeaderChange(true) jobServer.StartSchedulers() @@ -112,7 +112,7 @@ func TestScheduler(t *testing.T) { }) t.Run("ConfigChanged", func(t *testing.T) { - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.StartSchedulers() time.Sleep(time.Second) jobServer.HandleClusterLeaderChange(false) @@ -125,7 +125,7 @@ func TestScheduler(t *testing.T) { }) t.Run("ConfigChangedDeadlock", func(t *testing.T) { - jobServer.initSchedulers() + jobServer.InitSchedulers() jobServer.StartSchedulers() time.Sleep(time.Second) diff --git a/jobs/server.go b/jobs/server.go index 9c5870027cc..b7e7dbe341b 100644 --- a/jobs/server.go +++ b/jobs/server.go @@ -5,7 +5,6 @@ package jobs import ( "sync" - "time" "github.com/mattermost/mattermost-server/v6/einterfaces" "github.com/mattermost/mattermost-server/v6/model" @@ -25,33 +24,11 @@ type JobServer struct { } func NewJobServer(configService configservice.ConfigService, store store.Store, metrics einterfaces.MetricsInterface) *JobServer { - srv := &JobServer{ + return &JobServer{ ConfigService: configService, Store: store, metrics: metrics, } - srv.initWorkers() - srv.initSchedulers() - return srv -} - -func (srv *JobServer) initWorkers() { - workers := NewWorkers(srv.ConfigService) - workers.Watcher = srv.MakeWatcher(workers, DefaultWatcherPollingInterval) - srv.workers = workers -} - -func (srv *JobServer) initSchedulers() { - schedulers := &Schedulers{ - configChanged: make(chan *model.Config), - clusterLeaderChanged: make(chan bool, 1), - jobs: srv, - isLeader: true, - schedulers: make(map[string]model.Scheduler), - nextRunTimes: make(map[string]*time.Time), - } - - srv.schedulers = schedulers } func (srv *JobServer) Config() *model.Config { diff --git a/jobs/server_test.go b/jobs/server_test.go index 9c550c54d9b..391fc48d9bc 100644 --- a/jobs/server_test.go +++ b/jobs/server_test.go @@ -5,11 +5,44 @@ package jobs import ( "testing" - "time" "github.com/stretchr/testify/require" ) +func TestInitWorkers(t *testing.T) { + t.Run("initialize", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitWorkers() + require.NoError(t, err) + }) + + t.Run("re-initialize", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitWorkers() + require.NoError(t, err) + err = jobServer.InitWorkers() + require.NoError(t, err) + }) + + t.Run("re-initialize already running", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitWorkers() + require.NoError(t, err) + + err = jobServer.StartWorkers() + require.NoError(t, err) + + err = jobServer.InitWorkers() + require.Equal(t, ErrWorkersRunning, err) + + err = jobServer.StopWorkers() + require.NoError(t, err) + + err = jobServer.InitWorkers() + require.NoError(t, err) + }) +} + func TestStartWorkers(t *testing.T) { t.Run("uninitialized", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) @@ -19,24 +52,22 @@ func TestStartWorkers(t *testing.T) { t.Run("already running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initWorkers() - err := jobServer.StartWorkers() + err := jobServer.InitWorkers() + require.NoError(t, err) + err = jobServer.StartWorkers() require.NoError(t, err) err = jobServer.StartWorkers() require.Equal(t, ErrWorkersRunning, err) - // Parking the go routing to let the worker watcher start - time.Sleep(1 * time.Millisecond) err = jobServer.StopWorkers() require.NoError(t, err) }) t.Run("not running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initWorkers() - err := jobServer.StartWorkers() + err := jobServer.InitWorkers() + require.NoError(t, err) + err = jobServer.StartWorkers() require.NoError(t, err) - // Parking the go routing to let the worker watcher start - time.Sleep(1 * time.Millisecond) err = jobServer.StopWorkers() require.NoError(t, err) }) @@ -51,23 +82,57 @@ func TestStopWorkers(t *testing.T) { t.Run("not running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initWorkers() - err := jobServer.StopWorkers() + err := jobServer.InitWorkers() + require.NoError(t, err) + err = jobServer.StopWorkers() require.Equal(t, ErrWorkersNotRunning, err) }) t.Run("running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initWorkers() - err := jobServer.StartWorkers() + err := jobServer.InitWorkers() + require.NoError(t, err) + err = jobServer.StartWorkers() require.NoError(t, err) - // Parking the go routing to let the worker watcher start - time.Sleep(1 * time.Millisecond) err = jobServer.StopWorkers() require.NoError(t, err) }) } +func TestInitSchedulers(t *testing.T) { + t.Run("initialize", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitSchedulers() + require.NoError(t, err) + }) + + t.Run("re-initialize", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitSchedulers() + require.NoError(t, err) + err = jobServer.InitSchedulers() + require.NoError(t, err) + }) + + t.Run("re-initialize already running", func(t *testing.T) { + jobServer, _, _ := makeJobServer(t) + err := jobServer.InitSchedulers() + require.NoError(t, err) + + err = jobServer.StartSchedulers() + require.NoError(t, err) + + err = jobServer.InitSchedulers() + require.Equal(t, ErrSchedulersRunning, err) + + err = jobServer.StopSchedulers() + require.NoError(t, err) + + err = jobServer.InitSchedulers() + require.NoError(t, err) + }) +} + func TestStartSchedulers(t *testing.T) { t.Run("uninitialized", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) @@ -77,8 +142,9 @@ func TestStartSchedulers(t *testing.T) { t.Run("initialized", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initSchedulers() - err := jobServer.StartSchedulers() + err := jobServer.InitSchedulers() + require.NoError(t, err) + err = jobServer.StartSchedulers() require.NoError(t, err) err = jobServer.StopSchedulers() @@ -87,8 +153,9 @@ func TestStartSchedulers(t *testing.T) { t.Run("already running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initSchedulers() - err := jobServer.StartSchedulers() + err := jobServer.InitSchedulers() + require.NoError(t, err) + err = jobServer.StartSchedulers() require.NoError(t, err) err = jobServer.StartSchedulers() require.Equal(t, ErrSchedulersRunning, err) @@ -107,15 +174,17 @@ func TestStopSchedulers(t *testing.T) { t.Run("not running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initSchedulers() - err := jobServer.StopSchedulers() + err := jobServer.InitSchedulers() + require.NoError(t, err) + err = jobServer.StopSchedulers() require.Equal(t, ErrSchedulersNotRunning, err) }) t.Run("running", func(t *testing.T) { jobServer, _, _ := makeJobServer(t) - jobServer.initSchedulers() - err := jobServer.StartSchedulers() + err := jobServer.InitSchedulers() + require.NoError(t, err) + err = jobServer.StartSchedulers() require.NoError(t, err) err = jobServer.StopSchedulers() require.NoError(t, err) diff --git a/jobs/workers.go b/jobs/workers.go index 009b040f8d4..aa6251ec72d 100644 --- a/jobs/workers.go +++ b/jobs/workers.go @@ -27,6 +27,23 @@ var ( ErrWorkersUninitialized = errors.New("job workers are not initialized") ) +func (srv *JobServer) InitWorkers() error { + srv.mut.Lock() + defer srv.mut.Unlock() + + if srv.workers != nil && srv.workers.running { + return ErrWorkersRunning + } + + workers := NewWorkers(srv.ConfigService) + + workers.Watcher = srv.MakeWatcher(workers, DefaultWatcherPollingInterval) + + srv.workers = workers + + return nil +} + func NewWorkers(configService configservice.ConfigService) *Workers { return &Workers{ ConfigService: configService, diff --git a/model/client4.go b/model/client4.go index 000c0b41466..beabda82c9f 100644 --- a/model/client4.go +++ b/model/client4.go @@ -7931,16 +7931,3 @@ func (c *Client4) GetUsersWithInvalidEmails(page, perPage int) ([]*User, *Respon } return list, BuildResponse(r), nil } - -func (c *Client4) GetAppliedSchemaMigrations() ([]AppliedMigration, *Response, error) { - r, err := c.DoAPIGet(c.systemRoute()+"/schema/version", "") - if err != nil { - return nil, BuildResponse(r), err - } - defer closeBody(r) - var list []AppliedMigration - if jsonErr := json.NewDecoder(r.Body).Decode(&list); jsonErr != nil { - return nil, nil, NewAppError("GetUsers", "api.unmarshal_error", nil, jsonErr.Error(), http.StatusInternalServerError) - } - return list, BuildResponse(r), nil -} diff --git a/model/system.go b/model/system.go index 9e6c34b8ef6..b1b9ca1969d 100644 --- a/model/system.go +++ b/model/system.go @@ -173,8 +173,3 @@ type WarnMetricStatus struct { type SendWarnMetricAck struct { ForceAck bool `json:"forceAck"` } - -type AppliedMigration struct { - Version int `json:"version"` - Name string `json:"name"` -} diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index e5e701fc10b..0f2de5eb368 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -11781,6 +11781,10 @@ func (s *OpenTracingLayer) DropAllTables() { s.Store.DropAllTables() } +func (s *OpenTracingLayer) GetCurrentSchemaVersion() string { + return s.Store.GetCurrentSchemaVersion() +} + func (s *OpenTracingLayer) LockToMaster() { s.Store.LockToMaster() } diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 8baf14aa2c3..50116b5d62c 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -13422,6 +13422,10 @@ func (s *RetryLayer) DropAllTables() { s.Store.DropAllTables() } +func (s *RetryLayer) GetCurrentSchemaVersion() string { + return s.Store.GetCurrentSchemaVersion() +} + func (s *RetryLayer) LockToMaster() { s.Store.LockToMaster() } diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index 8a6c35bb006..cc436260171 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -353,10 +353,10 @@ func (ss *SqlStore) DriverName() string { return *ss.settings.DriverName } -func (ss *SqlStore) getCurrentSchemaVersion() (string, error) { +func (ss *SqlStore) GetCurrentSchemaVersion() string { var version string - err := ss.GetMasterX().Get(&version, "SELECT Value FROM Systems WHERE Name='Version'") - return version, err + _ = ss.GetMasterX().Get(&version, "SELECT Value FROM Systems WHERE Name='Version'") + return version } // GetDbVersion returns the version of the database being used. @@ -948,7 +948,6 @@ func (ss *SqlStore) DropAllTables() { FROM pg_class WHERE relkind = 'r' -- only tables AND relnamespace = 'public'::regnamespace - AND NOT relname = 'db_migrations' ); END $func$;`) @@ -956,9 +955,7 @@ func (ss *SqlStore) DropAllTables() { tables := []string{} ss.masterX.Select(&tables, `show tables`) for _, t := range tables { - if t != "db_migrations" { - ss.masterX.Exec(`TRUNCATE TABLE ` + t) - } + ss.masterX.Exec(`TRUNCATE TABLE ` + t) } } } @@ -1198,20 +1195,3 @@ func (ss *SqlStore) toReserveCase(str string) string { return fmt.Sprintf("`%s`", strings.Title(str)) } - -func (ss *SqlStore) GetDBSchemaVersion() (int, error) { - var version int - if err := ss.GetMasterX().Get(&version, "SELECT Version FROM db_migrations ORDER BY Version DESC LIMIT 1"); err != nil { - return 0, errors.Wrap(err, "unable to select from db_migrations") - } - return version, nil -} - -func (ss *SqlStore) GetAppliedMigrations() ([]model.AppliedMigration, error) { - migrations := []model.AppliedMigration{} - if err := ss.GetMasterX().Select(&migrations, "SELECT Version, Name FROM db_migrations ORDER BY Version DESC"); err != nil { - return nil, errors.Wrap(err, "unable to select from db_migrations") - } - - return migrations, nil -} diff --git a/store/sqlstore/store_test.go b/store/sqlstore/store_test.go index 20cd684da3d..0ac631b6fbc 100644 --- a/store/sqlstore/store_test.go +++ b/store/sqlstore/store_test.go @@ -6,11 +6,7 @@ package sqlstore import ( "fmt" "os" - "path/filepath" "regexp" - "sort" - "strconv" - "strings" "sync" "testing" "time" @@ -22,7 +18,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/mattermost/mattermost-server/v6/db/migrations" "github.com/mattermost/mattermost-server/v6/einterfaces/mocks" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/store" @@ -789,71 +784,3 @@ func TestMySQLReadTimeout(t *testing.T) { _, err = store.GetMasterX().ExecNoTimeout(`SELECT SLEEP(3)`) require.NoError(t, err) } - -func TestGetDBSchemaVersion(t *testing.T) { - testDrivers := []string{ - model.DatabaseDriverPostgres, - model.DatabaseDriverMysql, - } - - for _, driver := range testDrivers { - t.Run("Should return latest version number of applied migrations for "+driver, func(t *testing.T) { - t.Parallel() - settings := makeSqlSettings(driver) - store := New(*settings, nil) - - var assetNamesForDriver []string - for _, assetName := range migrations.AssetNames() { - if strings.HasPrefix(assetName, store.DriverName()) { - assetNamesForDriver = append(assetNamesForDriver, filepath.Base(assetName)) - } - } - sort.Strings(assetNamesForDriver) - - require.NotEmpty(t, assetNamesForDriver) - lastMigration := assetNamesForDriver[len(assetNamesForDriver)-1] - expectedVersion := strings.Split(lastMigration, "_")[0] - - version, err := store.GetDBSchemaVersion() - require.NoError(t, err) - require.Equal(t, expectedVersion, fmt.Sprintf("%06d", version)) - }) - } -} - -func TestGetAppliedMigrations(t *testing.T) { - testDrivers := []string{ - model.DatabaseDriverPostgres, - model.DatabaseDriverMysql, - } - - for _, driver := range testDrivers { - t.Run("Should return db applied migrations for "+driver, func(t *testing.T) { - t.Parallel() - settings := makeSqlSettings(driver) - store := New(*settings, nil) - - var migrationsFromFiles []model.AppliedMigration - for _, assetName := range migrations.AssetNames() { - if strings.HasPrefix(assetName, store.DriverName()) && strings.HasSuffix(assetName, ".up.sql") { - versionString := strings.Split(filepath.Base(assetName), "_")[0] - version, err := strconv.Atoi(versionString) - require.NoError(t, err) - - name := strings.TrimSuffix(strings.TrimLeft(filepath.Base(assetName), versionString+"_"), ".up.sql") - - migrationsFromFiles = append(migrationsFromFiles, model.AppliedMigration{ - Version: version, - Name: name, - }) - } - } - - require.NotEmpty(t, migrationsFromFiles) - - migrations, err := store.GetAppliedMigrations() - require.NoError(t, err) - require.ElementsMatch(t, migrationsFromFiles, migrations) - }) - } -} diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index def7262e391..084d3ce61b0 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -114,12 +114,8 @@ func upgradeDatabase(sqlStore *SqlStore, currentModelVersionString string) error return errors.Wrapf(err, "failed to parse oldest supported version %s", OldestSupportedVersion) } - currentSchemaVersionString, err := sqlStore.getCurrentSchemaVersion() - if err != nil { - mlog.Warn("could not receive the schema version from systems table", mlog.Err(err)) - } - var currentSchemaVersion *semver.Version + currentSchemaVersionString := sqlStore.GetCurrentSchemaVersion() if currentSchemaVersionString != "" { currentSchemaVersion, err = semver.New(currentSchemaVersionString) if err != nil { @@ -236,11 +232,7 @@ func saveSchemaVersion(sqlStore *SqlStore, version string) { } func shouldPerformUpgrade(sqlStore *SqlStore, currentSchemaVersion string, expectedSchemaVersion string) bool { - storedSchemaVersion, err := sqlStore.getCurrentSchemaVersion() - if err != nil { - mlog.Error("could not receive the schema version from systems table", mlog.Err(err)) - return false - } + storedSchemaVersion := sqlStore.GetCurrentSchemaVersion() storedVersion, err := semver.Parse(storedSchemaVersion) if err != nil { diff --git a/store/sqlstore/upgrade_test.go b/store/sqlstore/upgrade_test.go index 60ae37893fd..f21f1212647 100644 --- a/store/sqlstore/upgrade_test.go +++ b/store/sqlstore/upgrade_test.go @@ -17,11 +17,7 @@ func TestStoreUpgradeDotRelease(t *testing.T) { saveSchemaVersion(sqlStore, "5.33.1") err := upgradeDatabase(sqlStore, CurrentSchemaVersion) require.NoError(t, err) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) } @@ -38,44 +34,28 @@ func TestStoreUpgrade(t *testing.T) { saveSchemaVersion(sqlStore, "invalid") err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "failed to parse database schema version invalid: No Major.Minor.Patch elements found") - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, "invalid", currentVersion) + require.Equal(t, "invalid", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from unsupported version", func(t *testing.T) { saveSchemaVersion(sqlStore, "2.0.0") err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "Database schema version 2.0.0 is no longer supported. This Mattermost server supports automatic upgrades from schema version 3.0.0 through schema version 5.8.0. Please manually upgrade to at least version 3.0.0 before continuing.") - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, "2.0.0", currentVersion) + require.Equal(t, "2.0.0", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from earliest supported version", func(t *testing.T) { saveSchemaVersion(sqlStore, Version300) err := upgradeDatabase(sqlStore, CurrentSchemaVersion) require.NoError(t, err) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from no existing version", func(t *testing.T) { saveSchemaVersion(sqlStore, "") err := upgradeDatabase(sqlStore, CurrentSchemaVersion) require.NoError(t, err) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running earlier minor version", func(t *testing.T) { @@ -84,44 +64,28 @@ func TestStoreUpgrade(t *testing.T) { require.NoError(t, err) // Assert CurrentSchemaVersion, not 5.8.0, since the migrations will move // past 5.8.0 regardless of the input parameter. - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running later minor version", func(t *testing.T) { saveSchemaVersion(sqlStore, "5.99.0") err := upgradeDatabase(sqlStore, "5.8.0") require.NoError(t, err) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, "5.99.0", currentVersion) + require.Equal(t, "5.99.0", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running earlier major version", func(t *testing.T) { saveSchemaVersion(sqlStore, "4.1.0") err := upgradeDatabase(sqlStore, CurrentSchemaVersion) require.NoError(t, err) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running later major version", func(t *testing.T) { saveSchemaVersion(sqlStore, "6.0.0") err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "Database schema version 6.0.0 is not supported. This Mattermost server supports only >=5.8.0, <6.0.0. Please upgrade to at least version 6.0.0 before continuing.") - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, "6.0.0", currentVersion) + require.Equal(t, "6.0.0", sqlStore.GetCurrentSchemaVersion()) }) }) } @@ -136,11 +100,7 @@ func TestSaveSchemaVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, Version300, props["Version"]) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, Version300, currentVersion) + require.Equal(t, Version300, sqlStore.GetCurrentSchemaVersion()) }) t.Run("set current version", func(t *testing.T) { @@ -149,11 +109,7 @@ func TestSaveSchemaVersion(t *testing.T) { require.NoError(t, err) require.Equal(t, CurrentSchemaVersion, props["Version"]) - - currentVersion, err := sqlStore.getCurrentSchemaVersion() - require.NoError(t, err) - - require.Equal(t, CurrentSchemaVersion, currentVersion) + require.Equal(t, CurrentSchemaVersion, sqlStore.GetCurrentSchemaVersion()) }) }) } diff --git a/store/store.go b/store/store.go index e9e0fec3cca..de83e6e4974 100644 --- a/store/store.go +++ b/store/store.go @@ -63,8 +63,7 @@ type Store interface { UnlockFromMaster() DropAllTables() RecycleDBConnections(d time.Duration) - GetDBSchemaVersion() (int, error) - GetAppliedMigrations() ([]model.AppliedMigration, error) + GetCurrentSchemaVersion() string GetDbVersion(numerical bool) (string, error) TotalMasterDbConnections() int TotalReadDbConnections() int diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index 88abc607ea9..508e698ec71 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -222,48 +222,18 @@ func (_m *Store) FileInfo() store.FileInfoStore { return r0 } -// GetAppliedMigrations provides a mock function with given fields: -func (_m *Store) GetAppliedMigrations() ([]model.AppliedMigration, error) { +// GetCurrentSchemaVersion provides a mock function with given fields: +func (_m *Store) GetCurrentSchemaVersion() string { ret := _m.Called() - var r0 []model.AppliedMigration - if rf, ok := ret.Get(0).(func() []model.AppliedMigration); ok { + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { r0 = rf() } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]model.AppliedMigration) - } + r0 = ret.Get(0).(string) } - var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// GetDBSchemaVersion provides a mock function with given fields: -func (_m *Store) GetDBSchemaVersion() (int, error) { - ret := _m.Called() - - var r0 int - if rf, ok := ret.Get(0).(func() int); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(int) - } - - var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return r0 } // GetDbVersion provides a mock function with given fields: numerical diff --git a/store/storetest/store.go b/store/storetest/store.go index f814f0a9b65..d2217f5192f 100644 --- a/store/storetest/store.go +++ b/store/storetest/store.go @@ -104,13 +104,10 @@ func (s *Store) UnlockFromMaster() { /* do nothing */ } func (s *Store) DropAllTables() { /* do nothing */ } func (s *Store) GetDbVersion(bool) (string, error) { return "", nil } func (s *Store) RecycleDBConnections(time.Duration) {} -func (s *Store) GetDBSchemaVersion() (int, error) { return 1, nil } -func (s *Store) GetAppliedMigrations() ([]model.AppliedMigration, error) { - return []model.AppliedMigration{}, nil -} -func (s *Store) TotalMasterDbConnections() int { return 1 } -func (s *Store) TotalReadDbConnections() int { return 1 } -func (s *Store) TotalSearchDbConnections() int { return 1 } +func (s *Store) TotalMasterDbConnections() int { return 1 } +func (s *Store) TotalReadDbConnections() int { return 1 } +func (s *Store) TotalSearchDbConnections() int { return 1 } +func (s *Store) GetCurrentSchemaVersion() string { return "" } func (s *Store) CheckIntegrity() <-chan model.IntegrityCheckResult { return make(chan model.IntegrityCheckResult) } diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index cbe546182ae..c1ae4fbd641 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -10616,6 +10616,10 @@ func (s *TimerLayer) DropAllTables() { s.Store.DropAllTables() } +func (s *TimerLayer) GetCurrentSchemaVersion() string { + return s.Store.GetCurrentSchemaVersion() +} + func (s *TimerLayer) LockToMaster() { s.Store.LockToMaster() } diff --git a/testlib/store.go b/testlib/store.go index ee2cb31d237..688da733fe3 100644 --- a/testlib/store.go +++ b/testlib/store.go @@ -110,7 +110,5 @@ func GetMockStoreForSetupFunctions() *mocks.Store { mockStore.On("Session").Return(&sessionStore) mockStore.On("OAuth").Return(&oAuthStore) mockStore.On("Group").Return(&groupStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) - return &mockStore } diff --git a/web/context_test.go b/web/context_test.go index 9cb71cf0b51..f47049b0ab0 100644 --- a/web/context_test.go +++ b/web/context_test.go @@ -68,7 +68,6 @@ func TestMfaRequired(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.Srv().SetLicense(model.NewTestLicense("mfa")) diff --git a/web/handlers_test.go b/web/handlers_test.go index 5ae26292c61..2d93dd9db41 100644 --- a/web/handlers_test.go +++ b/web/handlers_test.go @@ -79,7 +79,6 @@ func TestHandlerServeHTTPSecureTransport(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(config *model.Config) { *config.ServiceSettings.TLSStrictTransport = true @@ -322,7 +321,6 @@ func TestHandlerServeCSPHeader(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = *cfg.ServiceSettings.SiteURL + "/subpath" @@ -643,7 +641,6 @@ func TestCheckCSRFToken(t *testing.T) { mockStore.On("User").Return(&mockUserStore) mockStore.On("Post").Return(&mockPostStore) mockStore.On("System").Return(&mockSystemStore) - mockStore.On("GetDBSchemaVersion").Return(1, nil) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = true