diff --git a/app/app.go b/app/app.go index 5998105200..bf167d35e3 100644 --- a/app/app.go +++ b/app/app.go @@ -154,9 +154,9 @@ func (a *App) Handle404(w http.ResponseWriter, r *http.Request) { } func (s *Server) getSystemInstallDate() (int64, *model.AppError) { - systemData, appErr := s.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) - if appErr != nil { - return 0, appErr + systemData, err := s.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) + if err != nil { + return 0, model.NewAppError("getSystemInstallDate", "app.system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) } value, err := strconv.ParseInt(systemData.Value, 10, 64) if err != nil { @@ -166,9 +166,9 @@ func (s *Server) getSystemInstallDate() (int64, *model.AppError) { } func (s *Server) getFirstServerRunTimestamp() (int64, *model.AppError) { - systemData, appErr := s.Store.System().GetByName(model.SYSTEM_FIRST_SERVER_RUN_TIMESTAMP_KEY) - if appErr != nil { - return 0, appErr + systemData, err := s.Store.System().GetByName(model.SYSTEM_FIRST_SERVER_RUN_TIMESTAMP_KEY) + if err != nil { + return 0, model.NewAppError("getFirstServerRunTimestamp", "app.system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) } value, err := strconv.ParseInt(systemData.Value, 10, 64) if err != nil { @@ -178,9 +178,9 @@ func (s *Server) getFirstServerRunTimestamp() (int64, *model.AppError) { } func (a *App) GetWarnMetricsStatus() (map[string]*model.WarnMetricStatus, *model.AppError) { - systemDataList, appErr := a.Srv().Store.System().Get() - if appErr != nil { - return nil, appErr + systemDataList, nErr := a.Srv().Store.System().Get() + if nErr != nil { + return nil, model.NewAppError("GetWarnMetricsStatus", "app.system.get.app_error", nil, nErr.Error(), http.StatusInternalServerError) } result := map[string]*model.WarnMetricStatus{} @@ -345,8 +345,8 @@ func (a *App) notifyAdminsOfWarnMetricStatus(warnMetricId string) *model.AppErro func (a *App) NotifyAndSetWarnMetricAck(warnMetricId string, sender *model.User, forceAck bool, isBot bool) *model.AppError { if warnMetric, ok := model.WarnMetricsTable[warnMetricId]; ok { - data, err := a.Srv().Store.System().GetByName(warnMetric.Id) - if err == nil && data != nil && data.Value == model.WARN_METRIC_STATUS_ACK { + data, nErr := a.Srv().Store.System().GetByName(warnMetric.Id) + if nErr == nil && data != nil && data.Value == model.WARN_METRIC_STATUS_ACK { mlog.Debug("This metric warning has already been acknowledged") return nil } @@ -384,14 +384,14 @@ func (a *App) NotifyAndSetWarnMetricAck(warnMetricId string, sender *model.User, subject := T("api.templates.warn_metric_ack.subject") bodyPage.Props["Title"] = warnMetricDisplayTexts.EmailBody - if err = mailservice.SendMailUsingConfig(model.MM_SUPPORT_ADDRESS, subject, bodyPage.Render(), a.Config(), false, sender.Email); err != nil { + if err := mailservice.SendMailUsingConfig(model.MM_SUPPORT_ADDRESS, subject, bodyPage.Render(), a.Config(), false, sender.Email); err != nil { mlog.Error("Error while sending email", mlog.String("destination email", model.MM_SUPPORT_ADDRESS), mlog.Err(err)) return model.NewAppError("NotifyAndSetWarnMetricAck", "api.email.send_warn_metric_ack.failure.app_error", map[string]interface{}{"Error": err.Error()}, "", http.StatusInternalServerError) } } mlog.Debug("Disable the monitoring of all warn metrics") - err = a.setWarnMetricsStatus(model.WARN_METRIC_STATUS_ACK) + err := a.setWarnMetricsStatus(model.WARN_METRIC_STATUS_ACK) if err != nil { return err } diff --git a/app/config.go b/app/config.go index e5edfe6997..588a2e706b 100644 --- a/app/config.go +++ b/app/config.go @@ -139,8 +139,8 @@ func (s *Server) ensurePostActionCookieSecret() error { } system.Value = string(v) // If we were able to save the key, use it, otherwise log the error. - if appErr := s.Store.System().Save(system); appErr != nil { - mlog.Error("Failed to save PostActionCookieSecret", mlog.Err(appErr)) + if err = s.Store.System().Save(system); err != nil { + mlog.Error("Failed to save PostActionCookieSecret", mlog.Err(err)) } else { secret = newSecret } @@ -202,8 +202,8 @@ func (s *Server) ensureAsymmetricSigningKey() error { } system.Value = string(v) // If we were able to save the key, use it, otherwise log the error. - if appErr := s.Store.System().Save(system); appErr != nil { - mlog.Error("Failed to save AsymmetricSigningKey", mlog.Err(appErr)) + if err = s.Store.System().Save(system); err != nil { + mlog.Error("Failed to save AsymmetricSigningKey", mlog.Err(err)) } else { key = newKey } @@ -242,40 +242,38 @@ func (s *Server) ensureAsymmetricSigningKey() error { } func (s *Server) ensureInstallationDate() error { - _, err := s.getSystemInstallDate() - if err == nil { + _, appErr := s.getSystemInstallDate() + if appErr == nil { return nil } - installDate, err := s.Store.User().InferSystemInstallDate() + installDate, appErr := s.Store.User().InferSystemInstallDate() var installationDate int64 - if err == nil && installDate > 0 { + if appErr == nil && installDate > 0 { installationDate = installDate } else { installationDate = utils.MillisFromTime(time.Now()) } - err = s.Store.System().SaveOrUpdate(&model.System{ + if err := s.Store.System().SaveOrUpdate(&model.System{ Name: model.SYSTEM_INSTALLATION_DATE_KEY, Value: strconv.FormatInt(installationDate, 10), - }) - if err != nil { + }); err != nil { return err } return nil } func (s *Server) ensureFirstServerRunTimestamp() error { - _, err := s.getFirstServerRunTimestamp() - if err == nil { + _, appErr := s.getFirstServerRunTimestamp() + if appErr == nil { return nil } - err = s.Store.System().SaveOrUpdate(&model.System{ + if err := s.Store.System().SaveOrUpdate(&model.System{ Name: model.SYSTEM_FIRST_SERVER_RUN_TIMESTAMP_KEY, Value: strconv.FormatInt(utils.MillisFromTime(time.Now()), 10), - }) - if err != nil { + }); err != nil { return err } return nil diff --git a/app/diagnostics.go b/app/diagnostics.go index 50eb750806..2bbe8979b3 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -1043,8 +1043,8 @@ func (s *Server) trackChannelModeration() { } func (s *Server) trackWarnMetrics() { - systemDataList, appErr := s.Store.System().Get() - if appErr != nil { + systemDataList, nErr := s.Store.System().Get() + if nErr != nil { return } for key, value := range systemDataList { diff --git a/app/license.go b/app/license.go index e5d656748d..016c5161b0 100644 --- a/app/license.go +++ b/app/license.go @@ -31,8 +31,8 @@ func (s *Server) LoadLicense() { } licenseId := "" - props, err := s.Store.System().Get() - if err == nil { + props, nErr := s.Store.System().Get() + if nErr == nil { licenseId = props[model.SYSTEM_ACTIVE_LICENSE_ID] } @@ -41,7 +41,7 @@ func (s *Server) LoadLicense() { license, licenseBytes := utils.GetAndValidateLicenseFileFromDisk(*s.Config().ServiceSettings.LicenseFileLocation) if license != nil { - if _, err = s.SaveLicense(licenseBytes); err != nil { + if _, err := s.SaveLicense(licenseBytes); err != nil { mlog.Info("Failed to save license key loaded from disk.", mlog.Err(err)) } else { licenseId = license.Id @@ -184,7 +184,7 @@ func (s *Server) RemoveLicense() *model.AppError { sysVar.Value = "" if err := s.Store.System().SaveOrUpdate(sysVar); err != nil { - return err + return model.NewAppError("RemoveLicense", "app.system.save.app_error", nil, err.Error(), http.StatusInternalServerError) } s.SetLicense(nil) diff --git a/app/permissions.go b/app/permissions.go index d0281c6bef..9b719f3d8a 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -55,17 +55,17 @@ func (a *App) ResetPermissionsSystem() *model.AppError { // Remove the "System" table entry that marks the advanced permissions migration as done. if _, err := a.Srv().Store.System().PermanentDeleteByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); err != nil { - return err + return model.NewAppError("ResetPermissionSystem", "app.system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) } // Remove the "System" table entry that marks the emoji permissions migration as done. if _, err := a.Srv().Store.System().PermanentDeleteByName(EMOJIS_PERMISSIONS_MIGRATION_KEY); err != nil { - return err + return model.NewAppError("ResetPermissionSystem", "app.system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) } // Remove the "System" table entry that marks the guest roles permissions migration as done. if _, err := a.Srv().Store.System().PermanentDeleteByName(GUEST_ROLES_CREATION_MIGRATION_KEY); err != nil { - return err + return model.NewAppError("ResetPermissionSystem", "app.system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) } // Now that the permissions system has been reset, re-run the migration to reinitialise it. diff --git a/app/permissions_migrations.go b/app/permissions_migrations.go index 99b76edbd2..0062723c27 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -176,7 +176,7 @@ func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *m } if err := a.Srv().Store.System().Save(&model.System{Name: key, Value: "true"}); err != nil { - return err + return model.NewAppError("doPermissionsMigration", "app.system.save.app_error", nil, err.Error(), http.StatusInternalServerError) } return nil } diff --git a/app/server.go b/app/server.go index 71ed789672..ba86aab7ac 100644 --- a/app/server.go +++ b/app/server.go @@ -1152,14 +1152,14 @@ func doCheckNumberOfActiveUsersWarnMetricStatus(a *App) { } for _, warnMetric := range warnMetrics { - data, err := a.Srv().Store.System().GetByName(warnMetric.Id) - if err == nil && data != nil && (data.Value == model.WARN_METRIC_STATUS_ACK || data.Value == model.WARN_METRIC_STATUS_RUNONCE) { + data, nErr := a.Srv().Store.System().GetByName(warnMetric.Id) + if nErr == nil && data != nil && (data.Value == model.WARN_METRIC_STATUS_ACK || data.Value == model.WARN_METRIC_STATUS_RUNONCE) { mlog.Debug("This metric warning has already been acked or should only run once") continue } - if err = a.Srv().Store.System().SaveOrUpdate(&model.System{Name: warnMetric.Id, Value: model.WARN_METRIC_STATUS_LIMIT_REACHED}); err != nil { - mlog.Error("Unable to write to database.", mlog.String("id", warnMetric.Id), mlog.Err(err)) + if nErr = a.Srv().Store.System().SaveOrUpdate(&model.System{Name: warnMetric.Id, Value: model.WARN_METRIC_STATUS_LIMIT_REACHED}); nErr != nil { + mlog.Error("Unable to write to database.", mlog.String("id", warnMetric.Id), mlog.Err(nErr)) continue } warnMetricStatus, _ := a.getWarnMetricStatusAndDisplayTextsForId(warnMetric.Id, nil) diff --git a/i18n/en.json b/i18n/en.json index f0eed1a5c3..add982db05 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4470,6 +4470,22 @@ "id": "app.submit_interactive_dialog.json_error", "translation": "Encountered an error encoding JSON for the interactive dialog." }, + { + "id": "app.system.get.app_error", + "translation": "We encountered an error finding the system properties." + }, + { + "id": "app.system.get_by_name.app_error", + "translation": "Unable to find the system variable." + }, + { + "id": "app.system.permanent_delete_by_name.app_error", + "translation": "We could not permanently delete the system table entry." + }, + { + "id": "app.system.save.app_error", + "translation": "We encountered an error saving the system property." + }, { "id": "app.system.warn_metric.bot_description", "translation": "[Learn more about the Mattermost Advisor](https://about.mattermost.com/default-channel-handle-documentation)" @@ -5502,6 +5518,10 @@ "id": "mfa.validate_token.authenticate.app_error", "translation": "Invalid MFA token." }, + { + "id": "migrations.system.save.app_error", + "translation": "We encountered an error saving the system property." + }, { "id": "migrations.worker.run_advanced_permissions_phase_2_migration.invalid_progress", "translation": "Migration failed due to invalid progress data." @@ -7310,30 +7330,6 @@ "id": "store.sql_post.search.disabled", "translation": "Searching has been disabled on this server. Please contact your System Administrator." }, - { - "id": "store.sql_system.get.app_error", - "translation": "We encountered an error finding the system properties." - }, - { - "id": "store.sql_system.get_by_name.app_error", - "translation": "Unable to find the system variable." - }, - { - "id": "store.sql_system.permanent_delete_by_name.app_error", - "translation": "We could not permanently delete the system table entry." - }, - { - "id": "store.sql_system.save.app_error", - "translation": "We encountered an error saving the system property." - }, - { - "id": "store.sql_system.save.commit_transaction.app_error", - "translation": "Failed to commit the database transaction." - }, - { - "id": "store.sql_system.update.app_error", - "translation": "We encountered an error updating the system property." - }, { "id": "store.sql_team.analytics_get_team_count_for_scheme.app_error", "translation": "Unable to get the channel count for the scheme." diff --git a/migrations/migrations_test.go b/migrations/migrations_test.go index e2227c5abd..4075b56895 100644 --- a/migrations/migrations_test.go +++ b/migrations/migrations_test.go @@ -34,16 +34,16 @@ func TestGetMigrationState(t *testing.T) { Name: migrationKey, Value: "true", } - err = th.App.Srv().Store.System().Save(&system) - assert.Nil(t, err) + nErr := th.App.Srv().Store.System().Save(&system) + assert.Nil(t, nErr) state, job, err = GetMigrationState(migrationKey, th.App.Srv().Store) assert.Nil(t, err) assert.Nil(t, job) assert.Equal(t, "completed", state) - _, err = th.App.Srv().Store.System().PermanentDeleteByName(migrationKey) - assert.Nil(t, err) + _, nErr = th.App.Srv().Store.System().PermanentDeleteByName(migrationKey) + assert.Nil(t, nErr) // Test with a job scheduled in "pending" state. j1 := &model.Job{ diff --git a/migrations/worker.go b/migrations/worker.go index f1067aaea7..a4386a66d0 100644 --- a/migrations/worker.go +++ b/migrations/worker.go @@ -157,8 +157,8 @@ func (worker *Worker) runMigration(key string, lastDone string) (bool, string, * } if done { - if saveErr := worker.srv.Store.System().Save(&model.System{Name: key, Value: "true"}); saveErr != nil { - return false, "", saveErr + if nErr := worker.srv.Store.System().Save(&model.System{Name: key, Value: "true"}); nErr != nil { + return false, "", model.NewAppError("runMigration", "migrations.system.save.app_error", nil, nErr.Error(), http.StatusInternalServerError) } } diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index b3e14486a6..09258cf832 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -6332,7 +6332,7 @@ func (s *OpenTracingLayerStatusStore) UpdateLastActivityAt(userId string, lastAc return err } -func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, *model.AppError) { +func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Get") s.Root.Store.SetContext(newCtx) @@ -6350,7 +6350,7 @@ func (s *OpenTracingLayerSystemStore) Get() (model.StringMap, *model.AppError) { return result, err } -func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) { +func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.GetByName") s.Root.Store.SetContext(newCtx) @@ -6368,7 +6368,7 @@ func (s *OpenTracingLayerSystemStore) GetByName(name string) (*model.System, *mo return result, err } -func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) { +func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.InsertIfExists") s.Root.Store.SetContext(newCtx) @@ -6386,7 +6386,7 @@ func (s *OpenTracingLayerSystemStore) InsertIfExists(system *model.System) (*mod return result, err } -func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { +func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.PermanentDeleteByName") s.Root.Store.SetContext(newCtx) @@ -6404,7 +6404,7 @@ func (s *OpenTracingLayerSystemStore) PermanentDeleteByName(name string) (*model return result, err } -func (s *OpenTracingLayerSystemStore) Save(system *model.System) *model.AppError { +func (s *OpenTracingLayerSystemStore) Save(system *model.System) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Save") s.Root.Store.SetContext(newCtx) @@ -6422,7 +6422,7 @@ func (s *OpenTracingLayerSystemStore) Save(system *model.System) *model.AppError return err } -func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError { +func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.SaveOrUpdate") s.Root.Store.SetContext(newCtx) @@ -6440,7 +6440,7 @@ func (s *OpenTracingLayerSystemStore) SaveOrUpdate(system *model.System) *model. return err } -func (s *OpenTracingLayerSystemStore) Update(system *model.System) *model.AppError { +func (s *OpenTracingLayerSystemStore) Update(system *model.System) error { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SystemStore.Update") s.Root.Store.SetContext(newCtx) diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 5b0b907a6d..b0fc19b959 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -4630,45 +4630,143 @@ func (s *RetryLayerStatusStore) UpdateLastActivityAt(userId string, lastActivity } -func (s *RetryLayerSystemStore) Get() (model.StringMap, *model.AppError) { +func (s *RetryLayerSystemStore) Get() (model.StringMap, error) { - return s.SystemStore.Get() + tries := 0 + for { + result, err := s.SystemStore.Get() + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) { +func (s *RetryLayerSystemStore) GetByName(name string) (*model.System, error) { - return s.SystemStore.GetByName(name) + tries := 0 + for { + result, err := s.SystemStore.GetByName(name) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) { +func (s *RetryLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) { - return s.SystemStore.InsertIfExists(system) + tries := 0 + for { + result, err := s.SystemStore.InsertIfExists(system) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { +func (s *RetryLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) { - return s.SystemStore.PermanentDeleteByName(name) + tries := 0 + for { + result, err := s.SystemStore.PermanentDeleteByName(name) + if err == nil { + return result, err + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + } } -func (s *RetryLayerSystemStore) Save(system *model.System) *model.AppError { +func (s *RetryLayerSystemStore) Save(system *model.System) error { - return s.SystemStore.Save(system) + tries := 0 + for { + err := s.SystemStore.Save(system) + if err == nil { + return err + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + } } -func (s *RetryLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError { +func (s *RetryLayerSystemStore) SaveOrUpdate(system *model.System) error { - return s.SystemStore.SaveOrUpdate(system) + tries := 0 + for { + err := s.SystemStore.SaveOrUpdate(system) + if err == nil { + return err + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + } } -func (s *RetryLayerSystemStore) Update(system *model.System) *model.AppError { +func (s *RetryLayerSystemStore) Update(system *model.System) error { - return s.SystemStore.Update(system) + tries := 0 + for { + err := s.SystemStore.Update(system) + if err == nil { + return err + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + } } diff --git a/store/sqlstore/system_store.go b/store/sqlstore/system_store.go index dc4634e0ab..ad756201bb 100644 --- a/store/sqlstore/system_store.go +++ b/store/sqlstore/system_store.go @@ -6,10 +6,11 @@ package sqlstore import ( "context" "database/sql" - "net/http" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/store" + + "github.com/pkg/errors" ) type SqlSystemStore struct { @@ -31,38 +32,38 @@ func newSqlSystemStore(sqlStore SqlStore) store.SystemStore { func (s SqlSystemStore) createIndexesIfNotExists() { } -func (s SqlSystemStore) Save(system *model.System) *model.AppError { +func (s SqlSystemStore) Save(system *model.System) error { if err := s.GetMaster().Insert(system); err != nil { - return model.NewAppError("SqlSystemStore.Save", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrapf(err, "failed to save system property with name=%s", system.Name) } return nil } -func (s SqlSystemStore) SaveOrUpdate(system *model.System) *model.AppError { +func (s SqlSystemStore) SaveOrUpdate(system *model.System) error { if err := s.GetMaster().SelectOne(&model.System{}, "SELECT * FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": system.Name}); err == nil { if _, err := s.GetMaster().Update(system); err != nil { - return model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrapf(err, "failed to update system property with name=%s", system.Name) } } else { if err := s.GetMaster().Insert(system); err != nil { - return model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrapf(err, "failed to save system property with name=%s", system.Name) } } return nil } -func (s SqlSystemStore) Update(system *model.System) *model.AppError { +func (s SqlSystemStore) Update(system *model.System) error { if _, err := s.GetMaster().Update(system); err != nil { - return model.NewAppError("SqlSystemStore.Update", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError) + return errors.Wrapf(err, "failed to update system property with name=%s", system.Name) } return nil } -func (s SqlSystemStore) Get() (model.StringMap, *model.AppError) { +func (s SqlSystemStore) Get() (model.StringMap, error) { var systems []model.System props := make(model.StringMap) if _, err := s.GetReplica().Select(&systems, "SELECT * FROM Systems"); err != nil { - return nil, model.NewAppError("SqlSystemStore.Get", "store.sql_system.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "failed to system properties") } for _, prop := range systems { props[prop.Name] = prop.Value @@ -71,19 +72,19 @@ func (s SqlSystemStore) Get() (model.StringMap, *model.AppError) { return props, nil } -func (s SqlSystemStore) GetByName(name string) (*model.System, *model.AppError) { +func (s SqlSystemStore) GetByName(name string) (*model.System, error) { var system model.System if err := s.GetMaster().SelectOne(&system, "SELECT * FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - return nil, model.NewAppError("SqlSystemStore.GetByName", "store.sql_system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get system property with name=%s", system.Name) } return &system, nil } -func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { +func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, error) { var system model.System if _, err := s.GetMaster().Exec("DELETE FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - return nil, model.NewAppError("SqlSystemStore.PermanentDeleteByName", "store.sql_system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to permanent delete system property with name=%s", system.Name) } return &system, nil @@ -91,12 +92,12 @@ func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, *mode // InsertIfExists inserts a given system value if it does not already exist. If a value // already exists, it returns the old one, else returns the new one. -func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) { +func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, error) { tx, err := s.GetMaster().BeginTx(context.Background(), &sql.TxOptions{ Isolation: sql.LevelSerializable, }) if err != nil { - return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "begin_transaction") } defer finalizeTransaction(tx) @@ -104,7 +105,7 @@ func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *mo if err := tx.SelectOne(&origSystem, `SELECT * FROM Systems WHERE Name = :Name`, map[string]interface{}{"Name": system.Name}); err != nil && err != sql.ErrNoRows { - return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to get system property with name=%s", system.Name) } if origSystem.Value != "" { @@ -114,11 +115,11 @@ func (s SqlSystemStore) InsertIfExists(system *model.System) (*model.System, *mo // Key does not exist, need to insert. if err := tx.Insert(system); err != nil { - return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrapf(err, "failed to save system property with name=%s", system.Name) } if err := tx.Commit(); err != nil { - return nil, model.NewAppError("SqlSystemStore.InsertIfExists", "store.sql_system.save.commit_transaction.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, errors.Wrap(err, "commit_transaction") } return system, nil } diff --git a/store/store.go b/store/store.go index ae84c97d8d..302039595f 100644 --- a/store/store.go +++ b/store/store.go @@ -429,13 +429,13 @@ type OAuthStore interface { } type SystemStore interface { - Save(system *model.System) *model.AppError - SaveOrUpdate(system *model.System) *model.AppError - Update(system *model.System) *model.AppError - Get() (model.StringMap, *model.AppError) - GetByName(name string) (*model.System, *model.AppError) - PermanentDeleteByName(name string) (*model.System, *model.AppError) - InsertIfExists(system *model.System) (*model.System, *model.AppError) + Save(system *model.System) error + SaveOrUpdate(system *model.System) error + Update(system *model.System) error + Get() (model.StringMap, error) + GetByName(name string) (*model.System, error) + PermanentDeleteByName(name string) (*model.System, error) + InsertIfExists(system *model.System) (*model.System, error) } type WebhookStore interface { diff --git a/store/storetest/mocks/SystemStore.go b/store/storetest/mocks/SystemStore.go index 048182861b..5c7b5cb6e9 100644 --- a/store/storetest/mocks/SystemStore.go +++ b/store/storetest/mocks/SystemStore.go @@ -15,7 +15,7 @@ type SystemStore struct { } // Get provides a mock function with given fields: -func (_m *SystemStore) Get() (model.StringMap, *model.AppError) { +func (_m *SystemStore) Get() (model.StringMap, error) { ret := _m.Called() var r0 model.StringMap @@ -27,20 +27,18 @@ func (_m *SystemStore) Get() (model.StringMap, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func() *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { r1 = rf() } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // GetByName provides a mock function with given fields: name -func (_m *SystemStore) GetByName(name string) (*model.System, *model.AppError) { +func (_m *SystemStore) GetByName(name string) (*model.System, error) { ret := _m.Called(name) var r0 *model.System @@ -52,20 +50,18 @@ func (_m *SystemStore) GetByName(name string) (*model.System, *model.AppError) { } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(name) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // InsertIfExists provides a mock function with given fields: system -func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) { +func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, error) { ret := _m.Called(system) var r0 *model.System @@ -77,20 +73,18 @@ func (_m *SystemStore) InsertIfExists(system *model.System) (*model.System, *mod } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(*model.System) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(*model.System) error); ok { r1 = rf(system) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // PermanentDeleteByName provides a mock function with given fields: name -func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { +func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, error) { ret := _m.Called(name) var r0 *model.System @@ -102,61 +96,53 @@ func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, *model } } - var r1 *model.AppError - if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { r1 = rf(name) } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AppError) - } + r1 = ret.Error(1) } return r0, r1 } // Save provides a mock function with given fields: system -func (_m *SystemStore) Save(system *model.System) *model.AppError { +func (_m *SystemStore) Save(system *model.System) error { ret := _m.Called(system) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(*model.System) error); ok { r0 = rf(system) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // SaveOrUpdate provides a mock function with given fields: system -func (_m *SystemStore) SaveOrUpdate(system *model.System) *model.AppError { +func (_m *SystemStore) SaveOrUpdate(system *model.System) error { ret := _m.Called(system) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(*model.System) error); ok { r0 = rf(system) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 } // Update provides a mock function with given fields: system -func (_m *SystemStore) Update(system *model.System) *model.AppError { +func (_m *SystemStore) Update(system *model.System) error { ret := _m.Called(system) - var r0 *model.AppError - if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { + var r0 error + if rf, ok := ret.Get(0).(func(*model.System) error); ok { r0 = rf(system) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.AppError) - } + r0 = ret.Error(0) } return r0 diff --git a/store/storetest/system_store.go b/store/storetest/system_store.go index bb034c3cd8..2e51c66b8a 100644 --- a/store/storetest/system_store.go +++ b/store/storetest/system_store.go @@ -111,7 +111,7 @@ func testInsertIfExists(t *testing.T, ss store.Store) { go func() { defer wg.Done() s1 := &model.System{Name: model.SYSTEM_CLUSTER_ENCRYPTION_KEY, Value: "firstKey"} - var err *model.AppError + var err error s2, err = ss.System().InsertIfExists(s1) require.Nil(t, err) }() @@ -119,7 +119,7 @@ func testInsertIfExists(t *testing.T, ss store.Store) { go func() { defer wg.Done() s1 := &model.System{Name: model.SYSTEM_CLUSTER_ENCRYPTION_KEY, Value: "secondKey"} - var err *model.AppError + var err error s3, err = ss.System().InsertIfExists(s1) require.Nil(t, err) }() diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index e6e5c31aea..8f56094be1 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -5724,7 +5724,7 @@ func (s *TimerLayerStatusStore) UpdateLastActivityAt(userId string, lastActivity return err } -func (s *TimerLayerSystemStore) Get() (model.StringMap, *model.AppError) { +func (s *TimerLayerSystemStore) Get() (model.StringMap, error) { start := timemodule.Now() result, err := s.SystemStore.Get() @@ -5740,7 +5740,7 @@ func (s *TimerLayerSystemStore) Get() (model.StringMap, *model.AppError) { return result, err } -func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, *model.AppError) { +func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, error) { start := timemodule.Now() result, err := s.SystemStore.GetByName(name) @@ -5756,7 +5756,7 @@ func (s *TimerLayerSystemStore) GetByName(name string) (*model.System, *model.Ap return result, err } -func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.System, *model.AppError) { +func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.System, error) { start := timemodule.Now() result, err := s.SystemStore.InsertIfExists(system) @@ -5772,7 +5772,7 @@ func (s *TimerLayerSystemStore) InsertIfExists(system *model.System) (*model.Sys return result, err } -func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { +func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.System, error) { start := timemodule.Now() result, err := s.SystemStore.PermanentDeleteByName(name) @@ -5788,7 +5788,7 @@ func (s *TimerLayerSystemStore) PermanentDeleteByName(name string) (*model.Syste return result, err } -func (s *TimerLayerSystemStore) Save(system *model.System) *model.AppError { +func (s *TimerLayerSystemStore) Save(system *model.System) error { start := timemodule.Now() err := s.SystemStore.Save(system) @@ -5804,7 +5804,7 @@ func (s *TimerLayerSystemStore) Save(system *model.System) *model.AppError { return err } -func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppError { +func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) error { start := timemodule.Now() err := s.SystemStore.SaveOrUpdate(system) @@ -5820,7 +5820,7 @@ func (s *TimerLayerSystemStore) SaveOrUpdate(system *model.System) *model.AppErr return err } -func (s *TimerLayerSystemStore) Update(system *model.System) *model.AppError { +func (s *TimerLayerSystemStore) Update(system *model.System) error { start := timemodule.Now() err := s.SystemStore.Update(system) diff --git a/testlib/store.go b/testlib/store.go index 94a6bfa4fb..3702502703 100644 --- a/testlib/store.go +++ b/testlib/store.go @@ -24,8 +24,8 @@ func (s *TestStore) Close() { func GetMockStoreForSetupFunctions() *mocks.Store { mockStore := mocks.Store{} systemStore := mocks.SystemStore{} - systemStore.On("GetByName", "AsymmetricSigningKey").Return(nil, model.NewAppError("FakeError", "store.sql_system.get_by_name.app_error", nil, "", http.StatusInternalServerError)) - systemStore.On("GetByName", "PostActionCookieSecret").Return(nil, model.NewAppError("FakeError", "store.sql_system.get_by_name.app_error", nil, "", http.StatusInternalServerError)) + systemStore.On("GetByName", "AsymmetricSigningKey").Return(nil, model.NewAppError("FakeError", "app.system.get_by_name.app_error", nil, "", http.StatusInternalServerError)) + systemStore.On("GetByName", "PostActionCookieSecret").Return(nil, model.NewAppError("FakeError", "app.system.get_by_name.app_error", nil, "", http.StatusInternalServerError)) systemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: strconv.FormatInt(model.GetMillis(), 10)}, nil) systemStore.On("GetByName", "FirstServerRunTimestamp").Return(&model.System{Name: "FirstServerRunTimestamp", Value: "10"}, nil) systemStore.On("GetByName", "AdvancedPermissionsMigrationComplete").Return(&model.System{Name: "AdvancedPermissionsMigrationComplete", Value: "true"}, nil)