From f8a89a52fc09cd5710de26e77fa1494cb61c127e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 21 May 2019 18:22:27 +0200 Subject: [PATCH] Migrate System store to Sync by default (#10838) --- app/app.go | 30 +++--- app/config.go | 38 +++---- app/config_test.go | 9 +- app/diagnostics.go | 4 +- app/import_functions_test.go | 20 ++-- app/license.go | 14 ++- app/migrations.go | 23 ++--- app/permissions.go | 4 +- app/permissions_migrations.go | 6 +- app/permissions_test.go | 6 +- app/scheme.go | 4 +- app/security_update_check.go | 148 ++++++++++++++------------- migrations/migrations.go | 2 +- migrations/migrations_test.go | 8 +- migrations/worker.go | 4 +- store/sqlstore/supplier.go | 16 +-- store/sqlstore/system_store.go | 98 ++++++++---------- store/sqlstore/upgrade.go | 6 +- store/sqlstore/upgrade_test.go | 10 +- store/store.go | 12 +-- store/storetest/mocks/SystemStore.go | 82 ++++++++++----- store/storetest/system_store.go | 61 ++++++----- 22 files changed, 310 insertions(+), 295 deletions(-) diff --git a/app/app.go b/app/app.go index 6d0775f3ab1..4f884342bac 100644 --- a/app/app.go +++ b/app/app.go @@ -110,18 +110,19 @@ func (a *App) EnsureDiagnosticId() { if a.Srv.diagnosticId != "" { return } - if result := <-a.Srv.Store.System().Get(); result.Err == nil { - props := result.Data.(model.StringMap) - - id := props[model.SYSTEM_DIAGNOSTIC_ID] - if len(id) == 0 { - id = model.NewId() - systemId := &model.System{Name: model.SYSTEM_DIAGNOSTIC_ID, Value: id} - <-a.Srv.Store.System().Save(systemId) - } - - a.Srv.diagnosticId = id + props, err := a.Srv.Store.System().Get() + if err != nil { + return } + + id := props[model.SYSTEM_DIAGNOSTIC_ID] + if len(id) == 0 { + id = model.NewId() + systemId := &model.System{Name: model.SYSTEM_DIAGNOSTIC_ID, Value: id} + a.Srv.Store.System().Save(systemId) + } + + a.Srv.diagnosticId = id } func (a *App) HTMLTemplates() *template.Template { @@ -144,11 +145,10 @@ func (a *App) Handle404(w http.ResponseWriter, r *http.Request) { } func (a *App) getSystemInstallDate() (int64, *model.AppError) { - result := <-a.Srv.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) - if result.Err != nil { - return 0, result.Err + systemData, appErr := a.Srv.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) + if appErr != nil { + return 0, appErr } - systemData := result.Data.(*model.System) value, err := strconv.ParseInt(systemData.Value, 10, 64) if err != nil { return 0, model.NewAppError("getSystemInstallDate", "app.system_install_date.parse_int.app_error", nil, err.Error(), http.StatusInternalServerError) diff --git a/app/config.go b/app/config.go index f65911eec5f..106afc163d0 100644 --- a/app/config.go +++ b/app/config.go @@ -113,9 +113,9 @@ func (a *App) ensurePostActionCookieSecret() error { var secret *model.SystemPostActionCookieSecret - result := <-a.Srv.Store.System().GetByName(model.SYSTEM_POST_ACTION_COOKIE_SECRET) - if result.Err == nil { - if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &secret); err != nil { + value, err := a.Srv.Store.System().GetByName(model.SYSTEM_POST_ACTION_COOKIE_SECRET) + if err == nil { + if err := json.Unmarshal([]byte(value.Value), &secret); err != nil { return err } } @@ -138,7 +138,7 @@ func (a *App) ensurePostActionCookieSecret() error { return err } system.Value = string(v) - if result = <-a.Srv.Store.System().Save(system); result.Err == nil { + if err = a.Srv.Store.System().Save(system); err == nil { // If we were able to save the key, use it, otherwise ignore the error. secret = newSecret } @@ -147,12 +147,12 @@ func (a *App) ensurePostActionCookieSecret() error { // If we weren't able to save a new key above, another server must have beat us to it. Get the // key from the database, and if that fails, error out. if secret == nil { - result := <-a.Srv.Store.System().GetByName(model.SYSTEM_POST_ACTION_COOKIE_SECRET) - if result.Err != nil { - return result.Err + value, err := a.Srv.Store.System().GetByName(model.SYSTEM_POST_ACTION_COOKIE_SECRET) + if err != nil { + return err } - if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &secret); err != nil { + if err := json.Unmarshal([]byte(value.Value), &secret); err != nil { return err } } @@ -170,9 +170,9 @@ func (a *App) ensureAsymmetricSigningKey() error { var key *model.SystemAsymmetricSigningKey - result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) - if result.Err == nil { - if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + value, err := a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if err == nil { + if err := json.Unmarshal([]byte(value.Value), &key); err != nil { return err } } @@ -199,7 +199,7 @@ func (a *App) ensureAsymmetricSigningKey() error { return err } system.Value = string(v) - if result = <-a.Srv.Store.System().Save(system); result.Err == nil { + if err = a.Srv.Store.System().Save(system); err == nil { // If we were able to save the key, use it, otherwise ignore the error. key = newKey } @@ -208,12 +208,12 @@ func (a *App) ensureAsymmetricSigningKey() error { // If we weren't able to save a new key above, another server must have beat us to it. Get the // key from the database, and if that fails, error out. if key == nil { - result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) - if result.Err != nil { - return result.Err + value, err := a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if err != nil { + return err } - if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + if err := json.Unmarshal([]byte(value.Value), &key); err != nil { return err } } @@ -251,12 +251,12 @@ func (a *App) ensureInstallationDate() error { installationDate = utils.MillisFromTime(time.Now()) } - result = <-a.Srv.Store.System().SaveOrUpdate(&model.System{ + err = a.Srv.Store.System().SaveOrUpdate(&model.System{ Name: model.SYSTEM_INSTALLATION_DATE_KEY, Value: strconv.FormatInt(installationDate, 10), }) - if result.Err != nil { - return result.Err + if err != nil { + return err } return nil } diff --git a/app/config_test.go b/app/config_test.go index bca7e691cfb..838456e6858 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -124,9 +124,9 @@ func TestEnsureInstallationDate(t *testing.T) { } if tc.PrevInstallationDate == nil { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.SYSTEM_INSTALLATION_DATE_KEY) + th.App.Srv.Store.System().PermanentDeleteByName(model.SYSTEM_INSTALLATION_DATE_KEY) } else { - <-th.App.Srv.Store.System().SaveOrUpdate(&model.System{ + th.App.Srv.Store.System().SaveOrUpdate(&model.System{ Name: model.SYSTEM_INSTALLATION_DATE_KEY, Value: strconv.FormatInt(*tc.PrevInstallationDate, 10), }) @@ -139,9 +139,8 @@ func TestEnsureInstallationDate(t *testing.T) { } else { assert.NoError(t, err) - result := <-th.App.Srv.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) - assert.Nil(t, result.Err) - data, _ := result.Data.(*model.System) + data, err := th.App.Srv.Store.System().GetByName(model.SYSTEM_INSTALLATION_DATE_KEY) + assert.Nil(t, err) value, _ := strconv.ParseInt(data.Value, 10, 64) assert.True(t, *tc.ExpectedInstallationDate <= value && *tc.ExpectedInstallationDate+1000 >= value) } diff --git a/app/diagnostics.go b/app/diagnostics.go index 81e5c141108..1d008dbbbae 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -703,12 +703,12 @@ func (a *App) trackServer() { func (a *App) trackPermissions() { phase1Complete := false - if ph1res := <-a.Srv.Store.System().GetByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); ph1res.Err == nil { + if _, err := a.Srv.Store.System().GetByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); err == nil { phase1Complete = true } phase2Complete := false - if ph2res := <-a.Srv.Store.System().GetByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2); ph2res.Err == nil { + if _, err := a.Srv.Store.System().GetByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2); err == nil { phase2Complete = true } diff --git a/app/import_functions_test.go b/app/import_functions_test.go index e041434e601..d91213fd7c1 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -22,10 +22,10 @@ func TestImportImportScheme(t *testing.T) { defer th.TearDown() // Mark the phase 2 permissions migration as completed. - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() // Try importing an invalid scheme in dryRun mode. @@ -243,10 +243,10 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) { defer th.TearDown() // Mark the phase 2 permissions migration as completed. - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() // Try importing an invalid scheme in dryRun mode. @@ -558,10 +558,10 @@ func TestImportImportTeam(t *testing.T) { defer th.TearDown() // Mark the phase 2 permissions migration as completed. - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() scheme1 := th.SetupTeamScheme() @@ -659,10 +659,10 @@ func TestImportImportChannel(t *testing.T) { defer th.TearDown() // Mark the phase 2 permissions migration as completed. - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() scheme1 := th.SetupChannelScheme() @@ -1505,10 +1505,10 @@ func TestImportImportUser(t *testing.T) { // to the appropriate scheme-managed-role booleans. // Mark the phase 2 permissions migration as completed. - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() teamSchemeData := &SchemeImportData{ diff --git a/app/license.go b/app/license.go index 2cc1ffd3136..421f01ac1b7 100644 --- a/app/license.go +++ b/app/license.go @@ -18,8 +18,8 @@ func (a *App) LoadLicense() { a.SetLicense(nil) licenseId := "" - if result := <-a.Srv.Store.System().Get(); result.Err == nil { - props := result.Data.(model.StringMap) + props, err := a.Srv.Store.System().Get() + if err == nil { licenseId = props[model.SYSTEM_ACTIVE_LICENSE_ID] } @@ -28,7 +28,7 @@ func (a *App) LoadLicense() { license, licenseBytes := utils.GetAndValidateLicenseFileFromDisk(*a.Config().ServiceSettings.LicenseFileLocation) if license != nil { - if _, err := a.SaveLicense(licenseBytes); err != nil { + if _, err = a.SaveLicense(licenseBytes); err != nil { mlog.Info(fmt.Sprintf("Failed to save license key loaded from disk err=%v", err.Error())) } else { licenseId = license.Id @@ -84,9 +84,7 @@ func (a *App) SaveLicense(licenseBytes []byte) (*model.License, *model.AppError) sysVar := &model.System{} sysVar.Name = model.SYSTEM_ACTIVE_LICENSE_ID sysVar.Value = license.Id - schan := a.Srv.Store.System().SaveOrUpdate(sysVar) - - if result := <-schan; result.Err != nil { + if err := a.Srv.Store.System().SaveOrUpdate(sysVar); err != nil { a.RemoveLicense() return nil, model.NewAppError("addLicense", "api.license.add_license.save_active.app_error", nil, "", http.StatusInternalServerError) } @@ -162,8 +160,8 @@ func (a *App) RemoveLicense() *model.AppError { sysVar.Name = model.SYSTEM_ACTIVE_LICENSE_ID sysVar.Value = "" - if result := <-a.Srv.Store.System().SaveOrUpdate(sysVar); result.Err != nil { - return result.Err + if err := a.Srv.Store.System().SaveOrUpdate(sysVar); err != nil { + return err } a.SetLicense(nil) diff --git a/app/migrations.go b/app/migrations.go index 61817871ee8..2412a85d29b 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -19,7 +19,7 @@ const GUEST_ROLES_CREATION_MIGRATION_KEY = "GuestRolesCreationMigrationComplete" // This function migrates the default built in roles from code/config to the database. func (a *App) DoAdvancedPermissionsMigration() { // If the migration is already marked as completed, don't do it again. - if result := <-a.Srv.Store.System().GetByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); result.Err == nil { + if _, err := a.Srv.Store.System().GetByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); err == nil { return } @@ -74,16 +74,15 @@ func (a *App) DoAdvancedPermissionsMigration() { Value: "true", } - if result := <-a.Srv.Store.System().Save(&system); result.Err != nil { - mlog.Critical("Failed to mark advanced permissions migration as completed.", mlog.Err(result.Err)) + if err := a.Srv.Store.System().Save(&system); err != nil { + mlog.Critical("Failed to mark advanced permissions migration as completed.", mlog.Err(err)) } } func (a *App) SetPhase2PermissionsMigrationStatus(isComplete bool) error { if !isComplete { - res := <-a.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) - if res.Err != nil { - return res.Err + if _, err := a.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2); err != nil { + return err } } a.Srv.phase2PermissionsMigrationComplete = isComplete @@ -92,7 +91,7 @@ func (a *App) SetPhase2PermissionsMigrationStatus(isComplete bool) error { func (a *App) DoEmojisPermissionsMigration() { // If the migration is already marked as completed, don't do it again. - if result := <-a.Srv.Store.System().GetByName(EMOJIS_PERMISSIONS_MIGRATION_KEY); result.Err == nil { + if _, err := a.Srv.Store.System().GetByName(EMOJIS_PERMISSIONS_MIGRATION_KEY); err == nil { return } @@ -147,14 +146,14 @@ func (a *App) DoEmojisPermissionsMigration() { Value: "true", } - if result := <-a.Srv.Store.System().Save(&system); result.Err != nil { - mlog.Critical("Failed to mark emojis permissions migration as completed.", mlog.Err(result.Err)) + if err := a.Srv.Store.System().Save(&system); err != nil { + mlog.Critical("Failed to mark emojis permissions migration as completed.", mlog.Err(err)) } } func (a *App) DoGuestRolesCreationMigration() { // If the migration is already marked as completed, don't do it again. - if result := <-a.Srv.Store.System().GetByName(GUEST_ROLES_CREATION_MIGRATION_KEY); result.Err == nil { + if _, err := a.Srv.Store.System().GetByName(GUEST_ROLES_CREATION_MIGRATION_KEY); err == nil { return } @@ -235,8 +234,8 @@ func (a *App) DoGuestRolesCreationMigration() { Value: "true", } - if result := <-a.Srv.Store.System().Save(&system); result.Err != nil { - mlog.Critical("Failed to mark guest roles creation migration as completed.", mlog.Err(result.Err)) + if err := a.Srv.Store.System().Save(&system); err != nil { + mlog.Critical("Failed to mark guest roles creation migration as completed.", mlog.Err(err)) } } diff --git a/app/permissions.go b/app/permissions.go index 67904883fa0..61724d03718 100644 --- a/app/permissions.go +++ b/app/permissions.go @@ -53,8 +53,8 @@ func (a *App) ResetPermissionsSystem() *model.AppError { } // Remove the "System" table entry that marks the advanced permissions migration as done. - if result := <-a.Srv.Store.System().PermanentDeleteByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); result.Err != nil { - return result.Err + if _, err := a.Srv.Store.System().PermanentDeleteByName(ADVANCED_PERMISSIONS_MIGRATION_KEY); err != nil { + return err } // 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 ada7f51d984..d222dc733cc 100644 --- a/app/permissions_migrations.go +++ b/app/permissions_migrations.go @@ -124,7 +124,7 @@ func applyPermissionsMap(roleName string, roleMap map[string]map[string]bool, mi } func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *model.AppError { - if result := <-a.Srv.Store.System().GetByName(key); result.Err == nil { + if _, err := a.Srv.Store.System().GetByName(key); err == nil { return nil } @@ -148,8 +148,8 @@ func (a *App) doPermissionsMigration(key string, migrationMap permissionsMap) *m } } - if result := <-a.Srv.Store.System().Save(&model.System{Name: key, Value: "true"}); result.Err != nil { - return result.Err + if err := a.Srv.Store.System().Save(&model.System{Name: key, Value: "true"}); err != nil { + return err } return nil } diff --git a/app/permissions_test.go b/app/permissions_test.go index b6db159b4a9..943c42a0dc2 100644 --- a/app/permissions_test.go +++ b/app/permissions_test.go @@ -252,11 +252,11 @@ func TestImportPermissions_schemeDeletedOnRoleFailure(t *testing.T) { func withMigrationMarkedComplete(th *TestHelper, f func()) { // Mark the migration as done. - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) - <-th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().Save(&model.System{Name: model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2, Value: "true"}) // Un-mark the migration at the end of the test. defer func() { - <-th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) + th.App.Srv.Store.System().PermanentDeleteByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2) }() f() } diff --git a/app/scheme.go b/app/scheme.go index ea476c620a3..743081db61a 100644 --- a/app/scheme.go +++ b/app/scheme.go @@ -158,8 +158,8 @@ func (a *App) IsPhase2MigrationCompleted() *model.AppError { return nil } - if result := <-a.Srv.Store.System().GetByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2); result.Err != nil { - return model.NewAppError("App.IsPhase2MigrationCompleted", "app.schemes.is_phase_2_migration_completed.not_completed.app_error", nil, result.Err.Error(), http.StatusNotImplemented) + if _, err := a.Srv.Store.System().GetByName(model.MIGRATION_KEY_ADVANCED_PERMISSIONS_PHASE_2); err != nil { + return model.NewAppError("App.IsPhase2MigrationCompleted", "app.schemes.is_phase_2_migration_completed.not_completed.app_error", nil, err.Error(), http.StatusNotImplemented) } a.Srv.phase2PermissionsMigrationComplete = true diff --git a/app/security_update_check.go b/app/security_update_check.go index fb3bfcc8604..a8ebf0cd6aa 100644 --- a/app/security_update_check.go +++ b/app/security_update_check.go @@ -33,93 +33,97 @@ const ( ) func (s *Server) DoSecurityUpdateCheck() { - if *s.Config().ServiceSettings.EnableSecurityFixAlert { - if result := <-s.Store.System().Get(); result.Err == nil { - props := result.Data.(model.StringMap) - lastSecurityTime, _ := strconv.ParseInt(props[model.SYSTEM_LAST_SECURITY_TIME], 10, 0) - currentTime := model.GetMillis() + if !*s.Config().ServiceSettings.EnableSecurityFixAlert { + return + } - if (currentTime - lastSecurityTime) > SECURITY_UPDATE_PERIOD { - mlog.Debug("Checking for security update from Mattermost") + props, err := s.Store.System().Get() + if err != nil { + return + } - v := url.Values{} + lastSecurityTime, _ := strconv.ParseInt(props[model.SYSTEM_LAST_SECURITY_TIME], 10, 0) + currentTime := model.GetMillis() - v.Set(PROP_SECURITY_ID, s.diagnosticId) - v.Set(PROP_SECURITY_BUILD, model.CurrentVersion+"."+model.BuildNumber) - v.Set(PROP_SECURITY_ENTERPRISE_READY, model.BuildEnterpriseReady) - v.Set(PROP_SECURITY_DATABASE, *s.Config().SqlSettings.DriverName) - v.Set(PROP_SECURITY_OS, runtime.GOOS) + if (currentTime - lastSecurityTime) > SECURITY_UPDATE_PERIOD { + mlog.Debug("Checking for security update from Mattermost") - if len(props[model.SYSTEM_RAN_UNIT_TESTS]) > 0 { - v.Set(PROP_SECURITY_UNIT_TESTS, "1") - } else { - v.Set(PROP_SECURITY_UNIT_TESTS, "0") - } + v := url.Values{} - systemSecurityLastTime := &model.System{Name: model.SYSTEM_LAST_SECURITY_TIME, Value: strconv.FormatInt(currentTime, 10)} - if lastSecurityTime == 0 { - <-s.Store.System().Save(systemSecurityLastTime) - } else { - <-s.Store.System().Update(systemSecurityLastTime) - } + v.Set(PROP_SECURITY_ID, s.diagnosticId) + v.Set(PROP_SECURITY_BUILD, model.CurrentVersion+"."+model.BuildNumber) + v.Set(PROP_SECURITY_ENTERPRISE_READY, model.BuildEnterpriseReady) + v.Set(PROP_SECURITY_DATABASE, *s.Config().SqlSettings.DriverName) + v.Set(PROP_SECURITY_OS, runtime.GOOS) - if ucr := <-s.Store.User().Count(model.UserCountOptions{ - IncludeDeleted: true, - }); ucr.Err == nil { - v.Set(PROP_SECURITY_USER_COUNT, strconv.FormatInt(ucr.Data.(int64), 10)) - } + if len(props[model.SYSTEM_RAN_UNIT_TESTS]) > 0 { + v.Set(PROP_SECURITY_UNIT_TESTS, "1") + } else { + v.Set(PROP_SECURITY_UNIT_TESTS, "0") + } - if ucr := <-s.Store.Status().GetTotalActiveUsersCount(); ucr.Err == nil { - v.Set(PROP_SECURITY_ACTIVE_USER_COUNT, strconv.FormatInt(ucr.Data.(int64), 10)) - } + systemSecurityLastTime := &model.System{Name: model.SYSTEM_LAST_SECURITY_TIME, Value: strconv.FormatInt(currentTime, 10)} + if lastSecurityTime == 0 { + s.Store.System().Save(systemSecurityLastTime) + } else { + s.Store.System().Update(systemSecurityLastTime) + } - if tcr := <-s.Store.Team().AnalyticsTeamCount(); tcr.Err == nil { - v.Set(PROP_SECURITY_TEAM_COUNT, strconv.FormatInt(tcr.Data.(int64), 10)) - } + if ucr := <-s.Store.User().Count(model.UserCountOptions{ + IncludeDeleted: true, + }); ucr.Err == nil { + v.Set(PROP_SECURITY_USER_COUNT, strconv.FormatInt(ucr.Data.(int64), 10)) + } - res, err := http.Get(SECURITY_URL + "/security?" + v.Encode()) - if err != nil { - mlog.Error("Failed to get security update information from Mattermost.") - return - } + if ucr := <-s.Store.Status().GetTotalActiveUsersCount(); ucr.Err == nil { + v.Set(PROP_SECURITY_ACTIVE_USER_COUNT, strconv.FormatInt(ucr.Data.(int64), 10)) + } - defer res.Body.Close() + if tcr := <-s.Store.Team().AnalyticsTeamCount(); tcr.Err == nil { + v.Set(PROP_SECURITY_TEAM_COUNT, strconv.FormatInt(tcr.Data.(int64), 10)) + } - bulletins := model.SecurityBulletinsFromJson(res.Body) + res, err := http.Get(SECURITY_URL + "/security?" + v.Encode()) + if err != nil { + mlog.Error("Failed to get security update information from Mattermost.") + return + } - for _, bulletin := range bulletins { - if bulletin.AppliesToVersion == model.CurrentVersion { - if props["SecurityBulletin_"+bulletin.Id] == "" { - results := <-s.Store.User().GetSystemAdminProfiles() - if results.Err != nil { - mlog.Error("Failed to get system admins for security update information from Mattermost.") - return - } - users := results.Data.(map[string]*model.User) + defer res.Body.Close() - resBody, err := http.Get(SECURITY_URL + "/bulletins/" + bulletin.Id) - if err != nil { - mlog.Error("Failed to get security bulletin details") - return - } + bulletins := model.SecurityBulletinsFromJson(res.Body) - body, err := ioutil.ReadAll(resBody.Body) - res.Body.Close() - if err != nil || resBody.StatusCode != 200 { - mlog.Error("Failed to read security bulletin details") - return - } - - for _, user := range users { - mlog.Info(fmt.Sprintf("Sending security bulletin for %v to %v", bulletin.Id, user.Email)) - license := s.License() - mailservice.SendMailUsingConfig(user.Email, utils.T("mattermost.bulletin.subject"), string(body), s.Config(), license != nil && *license.Features.Compliance) - } - - bulletinSeen := &model.System{Name: "SecurityBulletin_" + bulletin.Id, Value: bulletin.Id} - <-s.Store.System().Save(bulletinSeen) - } + for _, bulletin := range bulletins { + if bulletin.AppliesToVersion == model.CurrentVersion { + if props["SecurityBulletin_"+bulletin.Id] == "" { + results := <-s.Store.User().GetSystemAdminProfiles() + if results.Err != nil { + mlog.Error("Failed to get system admins for security update information from Mattermost.") + return } + users := results.Data.(map[string]*model.User) + + resBody, err := http.Get(SECURITY_URL + "/bulletins/" + bulletin.Id) + if err != nil { + mlog.Error("Failed to get security bulletin details") + return + } + + body, err := ioutil.ReadAll(resBody.Body) + res.Body.Close() + if err != nil || resBody.StatusCode != 200 { + mlog.Error("Failed to read security bulletin details") + return + } + + for _, user := range users { + mlog.Info(fmt.Sprintf("Sending security bulletin for %v to %v", bulletin.Id, user.Email)) + license := s.License() + mailservice.SendMailUsingConfig(user.Email, utils.T("mattermost.bulletin.subject"), string(body), s.Config(), license != nil && *license.Features.Compliance) + } + + bulletinSeen := &model.System{Name: "SecurityBulletin_" + bulletin.Id, Value: bulletin.Id} + s.Store.System().Save(bulletinSeen) } } } diff --git a/migrations/migrations.go b/migrations/migrations.go index 94099283912..59d71f25d27 100644 --- a/migrations/migrations.go +++ b/migrations/migrations.go @@ -36,7 +36,7 @@ func MakeMigrationsList() []string { } func GetMigrationState(migration string, store store.Store) (string, *model.Job, *model.AppError) { - if result := <-store.System().GetByName(migration); result.Err == nil { + if _, err := store.System().GetByName(migration); err == nil { return MIGRATION_STATE_COMPLETED, nil, nil } diff --git a/migrations/migrations_test.go b/migrations/migrations_test.go index aabfd75a8b5..db07073c506 100644 --- a/migrations/migrations_test.go +++ b/migrations/migrations_test.go @@ -30,16 +30,16 @@ func TestGetMigrationState(t *testing.T) { Name: migrationKey, Value: "true", } - res1 := <-th.App.Srv.Store.System().Save(&system) - assert.Nil(t, res1.Err) + err = th.App.Srv.Store.System().Save(&system) + assert.Nil(t, err) state, job, err = GetMigrationState(migrationKey, th.App.Srv.Store) assert.Nil(t, err) assert.Nil(t, job) assert.Equal(t, "completed", state) - res2 := <-th.App.Srv.Store.System().PermanentDeleteByName(migrationKey) - assert.Nil(t, res2.Err) + _, err = th.App.Srv.Store.System().PermanentDeleteByName(migrationKey) + assert.Nil(t, err) // Test with a job scheduled in "pending" state. j1 := &model.Job{ diff --git a/migrations/worker.go b/migrations/worker.go index e50b5157826..d55e3b66f38 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 result := <-worker.app.Srv.Store.System().Save(&model.System{Name: key, Value: "true"}); result.Err != nil { - return false, "", result.Err + if saveErr := worker.app.Srv.Store.System().Save(&model.System{Name: key, Value: "true"}); saveErr != nil { + return false, "", saveErr } } diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 35b576dd7d0..ff9abb37d0b 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -346,13 +346,15 @@ func (ss *SqlSupplier) TotalSearchDbConnections() int { } func (ss *SqlSupplier) MarkSystemRanUnitTests() { - if result := <-ss.System().Get(); result.Err == nil { - props := result.Data.(model.StringMap) - unitTests := props[model.SYSTEM_RAN_UNIT_TESTS] - if len(unitTests) == 0 { - systemTests := &model.System{Name: model.SYSTEM_RAN_UNIT_TESTS, Value: "1"} - <-ss.System().Save(systemTests) - } + props, err := ss.System().Get() + if err != nil { + return + } + + unitTests := props[model.SYSTEM_RAN_UNIT_TESTS] + if len(unitTests) == 0 { + systemTests := &model.System{Name: model.SYSTEM_RAN_UNIT_TESTS, Value: "1"} + ss.System().Save(systemTests) } } diff --git a/store/sqlstore/system_store.go b/store/sqlstore/system_store.go index 7c189cbbea8..63e5317d6a2 100644 --- a/store/sqlstore/system_store.go +++ b/store/sqlstore/system_store.go @@ -29,70 +29,60 @@ func NewSqlSystemStore(sqlStore SqlStore) store.SystemStore { func (s SqlSystemStore) CreateIndexesIfNotExists() { } -func (s SqlSystemStore) Save(system *model.System) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if err := s.GetMaster().Insert(system); err != nil { - result.Err = model.NewAppError("SqlSystemStore.Save", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) - } - }) +func (s SqlSystemStore) Save(system *model.System) *model.AppError { + 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 nil } -func (s SqlSystemStore) SaveOrUpdate(system *model.System) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - if err := s.GetReplica().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 { - result.Err = model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError) - } - } else { - if err := s.GetMaster().Insert(system); err != nil { - result.Err = model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.save.app_error", nil, err.Error(), http.StatusInternalServerError) - } - } - }) -} - -func (s SqlSystemStore) Update(system *model.System) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { +func (s SqlSystemStore) SaveOrUpdate(system *model.System) *model.AppError { + if err := s.GetReplica().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 { - result.Err = model.NewAppError("SqlSystemStore.Update", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError) + return model.NewAppError("SqlSystemStore.SaveOrUpdate", "store.sql_system.update.app_error", nil, err.Error(), http.StatusInternalServerError) } - }) + } 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 nil } -func (s SqlSystemStore) Get() store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var systems []model.System - props := make(model.StringMap) - if _, err := s.GetReplica().Select(&systems, "SELECT * FROM Systems"); err != nil { - result.Err = model.NewAppError("SqlSystemStore.Get", "store.sql_system.get.app_error", nil, err.Error(), http.StatusInternalServerError) - } else { - for _, prop := range systems { - props[prop.Name] = prop.Value - } - - result.Data = props - } - }) +func (s SqlSystemStore) Update(system *model.System) *model.AppError { + 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 nil } -func (s SqlSystemStore) GetByName(name string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var system model.System - if err := s.GetReplica().SelectOne(&system, "SELECT * FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - result.Err = model.NewAppError("SqlSystemStore.GetByName", "store.sql_system.get_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) - } +func (s SqlSystemStore) Get() (model.StringMap, *model.AppError) { + 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) + } + for _, prop := range systems { + props[prop.Name] = prop.Value + } - result.Data = &system - }) + return props, nil } -func (s SqlSystemStore) PermanentDeleteByName(name string) store.StoreChannel { - return store.Do(func(result *store.StoreResult) { - var system model.System - if _, err := s.GetMaster().Exec("DELETE FROM Systems WHERE Name = :Name", map[string]interface{}{"Name": name}); err != nil { - result.Err = model.NewAppError("SqlSystemStore.PermanentDeleteByName", "store.sql_system.permanent_delete_by_name.app_error", nil, err.Error(), http.StatusInternalServerError) - } +func (s SqlSystemStore) GetByName(name string) (*model.System, *model.AppError) { + var system model.System + if err := s.GetReplica().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) + } - result.Data = &system - }) + return &system, nil +} + +func (s SqlSystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { + 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 &system, nil } diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 589a76b2382..9df462dfec3 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -95,7 +95,7 @@ func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error // Assume a fresh database if no schema version has been recorded. if currentSchemaVersion == nil { - if result := <-sqlStore.System().SaveOrUpdate(&model.System{Name: "Version", Value: currentModelVersion.String()}); result.Err != nil { + if err := sqlStore.System().SaveOrUpdate(&model.System{Name: "Version", Value: currentModelVersion.String()}); err != nil { return errors.Wrap(err, "failed to initialize schema version for fresh database") } @@ -160,8 +160,8 @@ func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error } func saveSchemaVersion(sqlStore SqlStore, version string) { - if result := <-sqlStore.System().SaveOrUpdate(&model.System{Name: "Version", Value: version}); result.Err != nil { - mlog.Critical(result.Err.Error()) + if err := sqlStore.System().SaveOrUpdate(&model.System{Name: "Version", Value: version}); err != nil { + mlog.Critical(err.Error()) time.Sleep(time.Second) os.Exit(EXIT_VERSION_SAVE) } diff --git a/store/sqlstore/upgrade_test.go b/store/sqlstore/upgrade_test.go index ab6847ec24a..eae894eff4e 100644 --- a/store/sqlstore/upgrade_test.go +++ b/store/sqlstore/upgrade_test.go @@ -86,20 +86,18 @@ func TestSaveSchemaVersion(t *testing.T) { t.Run("set earliest version", func(t *testing.T) { saveSchemaVersion(sqlStore, VERSION_3_0_0) - result := <-ss.System().Get() - require.Nil(t, result.Err) + props, err := ss.System().Get() + require.Nil(t, err) - props := result.Data.(model.StringMap) require.Equal(t, VERSION_3_0_0, props["Version"]) require.Equal(t, VERSION_3_0_0, sqlStore.GetCurrentSchemaVersion()) }) t.Run("set current version", func(t *testing.T) { saveSchemaVersion(sqlStore, model.CurrentVersion) - result := <-ss.System().Get() - require.Nil(t, result.Err) + props, err := ss.System().Get() + require.Nil(t, err) - props := result.Data.(model.StringMap) require.Equal(t, model.CurrentVersion, props["Version"]) require.Equal(t, model.CurrentVersion, sqlStore.GetCurrentSchemaVersion()) }) diff --git a/store/store.go b/store/store.go index 854f1f41eb8..3dbcb41ce68 100644 --- a/store/store.go +++ b/store/store.go @@ -373,12 +373,12 @@ type OAuthStore interface { } type SystemStore interface { - Save(system *model.System) StoreChannel - SaveOrUpdate(system *model.System) StoreChannel - Update(system *model.System) StoreChannel - Get() StoreChannel - GetByName(name string) StoreChannel - PermanentDeleteByName(name string) StoreChannel + 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) } type WebhookStore interface { diff --git a/store/storetest/mocks/SystemStore.go b/store/storetest/mocks/SystemStore.go index e36396fe168..f9eeea7df2b 100644 --- a/store/storetest/mocks/SystemStore.go +++ b/store/storetest/mocks/SystemStore.go @@ -6,7 +6,6 @@ package mocks import mock "github.com/stretchr/testify/mock" import model "github.com/mattermost/mattermost-server/model" -import store "github.com/mattermost/mattermost-server/store" // SystemStore is an autogenerated mock type for the SystemStore type type SystemStore struct { @@ -14,63 +13,90 @@ type SystemStore struct { } // Get provides a mock function with given fields: -func (_m *SystemStore) Get() store.StoreChannel { +func (_m *SystemStore) Get() (model.StringMap, *model.AppError) { ret := _m.Called() - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func() store.StoreChannel); ok { + var r0 model.StringMap + if rf, ok := ret.Get(0).(func() model.StringMap); ok { r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(model.StringMap) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func() *model.AppError); ok { + r1 = rf() + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // GetByName provides a mock function with given fields: name -func (_m *SystemStore) GetByName(name string) store.StoreChannel { +func (_m *SystemStore) GetByName(name string) (*model.System, *model.AppError) { ret := _m.Called(name) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.System + if rf, ok := ret.Get(0).(func(string) *model.System); ok { r0 = rf(name) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.System) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // PermanentDeleteByName provides a mock function with given fields: name -func (_m *SystemStore) PermanentDeleteByName(name string) store.StoreChannel { +func (_m *SystemStore) PermanentDeleteByName(name string) (*model.System, *model.AppError) { ret := _m.Called(name) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + var r0 *model.System + if rf, ok := ret.Get(0).(func(string) *model.System); ok { r0 = rf(name) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.System) } } - return r0 + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(name) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 } // Save provides a mock function with given fields: system -func (_m *SystemStore) Save(system *model.System) store.StoreChannel { +func (_m *SystemStore) Save(system *model.System) *model.AppError { ret := _m.Called(system) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.System) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { r0 = rf(system) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -78,15 +104,15 @@ func (_m *SystemStore) Save(system *model.System) store.StoreChannel { } // SaveOrUpdate provides a mock function with given fields: system -func (_m *SystemStore) SaveOrUpdate(system *model.System) store.StoreChannel { +func (_m *SystemStore) SaveOrUpdate(system *model.System) *model.AppError { ret := _m.Called(system) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.System) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { r0 = rf(system) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } @@ -94,15 +120,15 @@ func (_m *SystemStore) SaveOrUpdate(system *model.System) store.StoreChannel { } // Update provides a mock function with given fields: system -func (_m *SystemStore) Update(system *model.System) store.StoreChannel { +func (_m *SystemStore) Update(system *model.System) *model.AppError { ret := _m.Called(system) - var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.System) store.StoreChannel); ok { + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(*model.System) *model.AppError); ok { r0 = rf(system) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(store.StoreChannel) + r0 = ret.Get(0).(*model.AppError) } } diff --git a/store/storetest/system_store.go b/store/storetest/system_store.go index 6dc1efe4178..02964fd0466 100644 --- a/store/storetest/system_store.go +++ b/store/storetest/system_store.go @@ -21,37 +21,35 @@ func TestSystemStore(t *testing.T, ss store.Store) { func testSystemStore(t *testing.T, ss store.Store) { system := &model.System{Name: model.NewId(), Value: "value"} - store.Must(ss.System().Save(system)) + err := ss.System().Save(system) + require.Nil(t, err) - result := <-ss.System().Get() - systems := result.Data.(model.StringMap) + systems, _ := ss.System().Get() require.Equal(t, system.Value, systems[system.Name]) system.Value = "value2" - store.Must(ss.System().Update(system)) - - result2 := <-ss.System().Get() - systems2 := result2.Data.(model.StringMap) + err = ss.System().Update(system) + require.Nil(t, err) + systems2, _ := ss.System().Get() require.Equal(t, system.Value, systems2[system.Name]) - result3 := <-ss.System().GetByName(system.Name) - rsystem := result3.Data.(*model.System) + rsystem, _ := ss.System().GetByName(system.Name) require.Equal(t, system.Value, rsystem.Value) } func testSystemStoreSaveOrUpdate(t *testing.T, ss store.Store) { system := &model.System{Name: model.NewId(), Value: "value"} - if err := (<-ss.System().SaveOrUpdate(system)).Err; err != nil { + if err := ss.System().SaveOrUpdate(system); err != nil { t.Fatal(err) } system.Value = "value2" - if r := <-ss.System().SaveOrUpdate(system); r.Err != nil { - t.Fatal(r.Err) + if err := ss.System().SaveOrUpdate(system); err != nil { + t.Fatal(err) } } @@ -59,31 +57,32 @@ func testSystemStorePermanentDeleteByName(t *testing.T, ss store.Store) { s1 := &model.System{Name: model.NewId(), Value: "value"} s2 := &model.System{Name: model.NewId(), Value: "value"} - store.Must(ss.System().Save(s1)) - store.Must(ss.System().Save(s2)) + err := ss.System().Save(s1) + require.Nil(t, err) + err = ss.System().Save(s2) + require.Nil(t, err) - res1 := <-ss.System().GetByName(s1.Name) - assert.Nil(t, res1.Err) + _, err = ss.System().GetByName(s1.Name) + assert.Nil(t, err) - res2 := <-ss.System().GetByName(s2.Name) - assert.Nil(t, res2.Err) + _, err = ss.System().GetByName(s2.Name) + assert.Nil(t, err) - res3 := <-ss.System().PermanentDeleteByName(s1.Name) - assert.Nil(t, res3.Err) + _, err = ss.System().PermanentDeleteByName(s1.Name) + assert.Nil(t, err) - res4 := <-ss.System().GetByName(s1.Name) - assert.NotNil(t, res4.Err) + _, err = ss.System().GetByName(s1.Name) + assert.NotNil(t, err) - res5 := <-ss.System().GetByName(s2.Name) - assert.Nil(t, res5.Err) + _, err = ss.System().GetByName(s2.Name) + assert.Nil(t, err) - res6 := <-ss.System().PermanentDeleteByName(s2.Name) - assert.Nil(t, res6.Err) + _, err = ss.System().PermanentDeleteByName(s2.Name) + assert.Nil(t, err) - res7 := <-ss.System().GetByName(s1.Name) - assert.NotNil(t, res7.Err) - - res8 := <-ss.System().GetByName(s2.Name) - assert.NotNil(t, res8.Err) + _, err = ss.System().GetByName(s1.Name) + assert.NotNil(t, err) + _, err = ss.System().GetByName(s2.Name) + assert.NotNil(t, err) }