From ce2b2be5de578bd9eb44b26e04db75ca61d67ca5 Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 31 Oct 2017 09:39:31 -0500 Subject: Refactoring cfg refs and load / save functions (#7749) * refactoring cfg refs and load / save functions * improve error output --- app/admin.go | 15 ++++-------- app/analytics.go | 4 ++-- app/app.go | 19 +++++++++++++++ app/app_test.go | 7 +----- app/apptestlib.go | 12 +++------- app/plugins.go | 14 ++++++----- app/post_test.go | 9 +++---- app/saml.go | 12 +++++----- app/user.go | 70 +++++++++++++++++++++++++++---------------------------- app/user_test.go | 5 +--- 10 files changed, 83 insertions(+), 84 deletions(-) (limited to 'app') diff --git a/app/admin.go b/app/admin.go index a181fa251..ef5a1c5d5 100644 --- a/app/admin.go +++ b/app/admin.go @@ -131,14 +131,6 @@ func (a *App) GetConfig() *model.Config { return cfg } -func (a *App) ReloadConfig() { - debug.FreeOSMemory() - utils.LoadConfig(a.ConfigFileName()) - - // start/restart email batching job if necessary - a.InitEmailBatching() -} - func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) *model.AppError { oldCfg := a.Config() cfg.SetDefaults() @@ -157,8 +149,11 @@ func (a *App) SaveConfig(cfg *model.Config, sendConfigChangeClusterMessage bool) } utils.DisableConfigWatch() - utils.SaveConfig(a.ConfigFileName(), cfg) - utils.LoadConfig(a.ConfigFileName()) + a.UpdateConfig(func(update *model.Config) { + *update = *cfg + }) + a.PersistConfig() + a.ReloadConfig() utils.EnableConfigWatch() if a.Metrics != nil { diff --git a/app/analytics.go b/app/analytics.go index 5d30ad426..f4e6fec95 100644 --- a/app/analytics.go +++ b/app/analytics.go @@ -251,7 +251,7 @@ func (a *App) GetRecentlyActiveUsersForTeamPage(teamId string, page, perPage int users = result.Data.([]*model.User) } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetNewUsersForTeamPage(teamId string, page, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { @@ -262,5 +262,5 @@ func (a *App) GetNewUsersForTeamPage(teamId string, page, perPage int, asAdmin b users = result.Data.([]*model.User) } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } diff --git a/app/app.go b/app/app.go index 002a6a272..a8d2977b5 100644 --- a/app/app.go +++ b/app/app.go @@ -5,6 +5,7 @@ package app import ( "net/http" + "runtime/debug" "sync/atomic" l4g "github.com/alecthomas/log4go" @@ -62,6 +63,12 @@ func New(options ...Option) *App { panic("Only one App should exist at a time. Did you forget to call Shutdown()?") } + if utils.T == nil { + utils.TranslationsPreInit() + } + utils.LoadGlobalConfig("config.json") + utils.InitTranslations(utils.Cfg.LocalizationSettings) + l4g.Info(utils.T("api.server.new_server.init.info")) app := &App{ @@ -246,6 +253,18 @@ func (a *App) UpdateConfig(f func(*model.Config)) { f(utils.Cfg) } +func (a *App) PersistConfig() { + utils.SaveConfig(a.ConfigFileName(), a.Config()) +} + +func (a *App) ReloadConfig() { + debug.FreeOSMemory() + utils.LoadGlobalConfig(a.ConfigFileName()) + + // start/restart email batching job if necessary + a.InitEmailBatching() +} + func (a *App) ConfigFileName() string { return utils.CfgFileName } diff --git a/app/app_test.go b/app/app_test.go index 6f2a3a23a..ac39b49fe 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -16,20 +16,15 @@ import ( func TestMain(m *testing.M) { flag.Parse() + utils.TranslationsPreInit() // In the case where a dev just wants to run a single test, it's faster to just use the default // store. if filter := flag.Lookup("test.run").Value.String(); filter != "" && filter != "." { - utils.TranslationsPreInit() - utils.LoadConfig("config.json") l4g.Info("-test.run used, not creating temporary containers") os.Exit(m.Run()) } - utils.TranslationsPreInit() - utils.LoadConfig("config.json") - utils.InitTranslations(utils.Cfg.LocalizationSettings) - status := 0 container, settings, err := storetest.NewMySQLContainer() diff --git a/app/apptestlib.go b/app/apptestlib.go index 3557a8727..b34749be3 100644 --- a/app/apptestlib.go +++ b/app/apptestlib.go @@ -48,12 +48,6 @@ func StopTestStore() { } func setupTestHelper(enterprise bool) *TestHelper { - if utils.T == nil { - utils.TranslationsPreInit() - } - utils.LoadConfig("config.json") - utils.InitTranslations(utils.Cfg.LocalizationSettings) - var options []Option if testStore != nil { options = append(options, StoreOverride(testStore)) @@ -63,8 +57,8 @@ func setupTestHelper(enterprise bool) *TestHelper { App: New(options...), } - *utils.Cfg.TeamSettings.MaxUsersPerTeam = 50 - *utils.Cfg.RateLimitSettings.Enable = false + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.MaxUsersPerTeam = 50 }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.RateLimitSettings.Enable = false }) utils.DisableDebugLogForTest() prevListenAddress := *th.App.Config().ServiceSettings.ListenAddress if testStore != nil { @@ -76,7 +70,7 @@ func setupTestHelper(enterprise bool) *TestHelper { utils.EnableDebugLogForTest() th.App.Srv.Store.MarkSystemRanUnitTests() - *utils.Cfg.TeamSettings.EnableOpenServer = true + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = true }) utils.SetIsLicensed(enterprise) if enterprise { diff --git a/app/plugins.go b/app/plugins.go index c2f3c4785..c2e15b385 100644 --- a/app/plugins.go +++ b/app/plugins.go @@ -471,10 +471,11 @@ func (a *App) EnablePlugin(id string) *model.AppError { return model.NewAppError("EnablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) } - cfg := a.Config() - cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true} + a.UpdateConfig(func(cfg *model.Config) { + cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true} + }) - if err := a.SaveConfig(cfg, true); err != nil { + if err := a.SaveConfig(a.Config(), true); err != nil { return model.NewAppError("EnablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -504,10 +505,11 @@ func (a *App) DisablePlugin(id string) *model.AppError { return model.NewAppError("DisablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) } - cfg := a.Config() - cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: false} + a.UpdateConfig(func(cfg *model.Config) { + cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: false} + }) - if err := a.SaveConfig(cfg, true); err != nil { + if err := a.SaveConfig(a.Config(), true); err != nil { return model.NewAppError("DisablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/app/post_test.go b/app/post_test.go index 24910f964..e2e9a7261 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -15,7 +15,6 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" - "github.com/mattermost/mattermost-server/utils" ) func TestUpdatePostEditAt(t *testing.T) { @@ -82,11 +81,9 @@ func TestPostAction(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections - defer func() { - utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections - }() - *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" + }) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var request model.PostActionIntegrationRequest diff --git a/app/saml.go b/app/saml.go index 2fdb30e99..3df00a9e1 100644 --- a/app/saml.go +++ b/app/saml.go @@ -65,7 +65,7 @@ func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.Ap } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } @@ -83,7 +83,7 @@ func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.A } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } @@ -101,7 +101,7 @@ func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppEr } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } @@ -134,7 +134,7 @@ func (a *App) RemoveSamlPublicCertificate() *model.AppError { } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } @@ -153,7 +153,7 @@ func (a *App) RemoveSamlPrivateCertificate() *model.AppError { } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } @@ -172,7 +172,7 @@ func (a *App) RemoveSamlIdpCertificate() *model.AppError { } a.UpdateConfig(func(dest *model.Config) { *dest = *cfg }) - utils.SaveConfig(a.ConfigFileName(), cfg) + a.PersistConfig() return nil } diff --git a/app/user.go b/app/user.go index 3272781e7..e6ae7f174 100644 --- a/app/user.go +++ b/app/user.go @@ -39,7 +39,7 @@ const ( ) func (a *App) CreateUserWithHash(user *model.User, hash string, data string) (*model.User, *model.AppError) { - if err := IsUserSignUpAllowed(); err != nil { + if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } @@ -81,7 +81,7 @@ func (a *App) CreateUserWithHash(user *model.User, hash string, data string) (*m } func (a *App) CreateUserWithInviteId(user *model.User, inviteId string) (*model.User, *model.AppError) { - if err := IsUserSignUpAllowed(); err != nil { + if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } @@ -127,7 +127,7 @@ func (a *App) CreateUserAsAdmin(user *model.User) (*model.User, *model.AppError) } func (a *App) CreateUserFromSignup(user *model.User) (*model.User, *model.AppError) { - if err := IsUserSignUpAllowed(); err != nil { + if err := a.IsUserSignUpAllowed(); err != nil { return nil, err } @@ -150,8 +150,8 @@ func (a *App) CreateUserFromSignup(user *model.User) (*model.User, *model.AppErr return ruser, nil } -func IsUserSignUpAllowed() *model.AppError { - if !utils.Cfg.EmailSettings.EnableSignUpWithEmail || !utils.Cfg.TeamSettings.EnableUserCreation { +func (a *App) IsUserSignUpAllowed() *model.AppError { + if !a.Config().EmailSettings.EnableSignUpWithEmail || !a.Config().TeamSettings.EnableUserCreation { err := model.NewAppError("IsUserSignUpAllowed", "api.user.create_user.signup_email_disabled.app_error", nil, "", http.StatusNotImplemented) return err } @@ -421,7 +421,7 @@ func (a *App) GetUsersMap(offset int, limit int, asAdmin bool) (map[string]*mode userMap := make(map[string]*model.User, len(users)) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) userMap[user.Id] = user } @@ -434,7 +434,7 @@ func (a *App) GetUsersPage(page int, perPage int, asAdmin bool) ([]*model.User, return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersEtag() string { @@ -466,7 +466,7 @@ func (a *App) GetUsersInTeamMap(teamId string, offset int, limit int, asAdmin bo userMap := make(map[string]*model.User, len(users)) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) userMap[user.Id] = user } @@ -479,7 +479,7 @@ func (a *App) GetUsersInTeamPage(teamId string, page int, perPage int, asAdmin b return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersNotInTeamPage(teamId string, page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { @@ -488,7 +488,7 @@ func (a *App) GetUsersNotInTeamPage(teamId string, page int, perPage int, asAdmi return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersInTeamEtag(teamId string) string { @@ -516,7 +516,7 @@ func (a *App) GetUsersInChannelMap(channelId string, offset int, limit int, asAd userMap := make(map[string]*model.User, len(users)) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) userMap[user.Id] = user } @@ -529,7 +529,7 @@ func (a *App) GetUsersInChannelPage(channelId string, page int, perPage int, asA return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersNotInChannel(teamId string, channelId string, offset int, limit int) ([]*model.User, *model.AppError) { @@ -549,7 +549,7 @@ func (a *App) GetUsersNotInChannelMap(teamId string, channelId string, offset in userMap := make(map[string]*model.User, len(users)) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) userMap[user.Id] = user } @@ -562,7 +562,7 @@ func (a *App) GetUsersNotInChannelPage(teamId string, channelId string, page int return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool) ([]*model.User, *model.AppError) { @@ -571,7 +571,7 @@ func (a *App) GetUsersWithoutTeamPage(page int, perPage int, asAdmin bool) ([]*m return nil, err } - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } func (a *App) GetUsersWithoutTeam(offset int, limit int) ([]*model.User, *model.AppError) { @@ -587,7 +587,7 @@ func (a *App) GetUsersByIds(userIds []string, asAdmin bool) ([]*model.User, *mod return nil, result.Err } else { users := result.Data.([]*model.User) - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } } @@ -596,13 +596,13 @@ func (a *App) GetUsersByUsernames(usernames []string, asAdmin bool) ([]*model.Us return nil, result.Err } else { users := result.Data.([]*model.User) - return sanitizeProfiles(users, asAdmin), nil + return a.sanitizeProfiles(users, asAdmin), nil } } -func sanitizeProfiles(users []*model.User, asAdmin bool) []*model.User { +func (a *App) sanitizeProfiles(users []*model.User, asAdmin bool) []*model.User { for _, u := range users { - SanitizeProfile(u, asAdmin) + a.SanitizeProfile(u, asAdmin) } return users @@ -665,7 +665,7 @@ func (a *App) DeactivateMfa(userId string) *model.AppError { return nil } -func CreateProfileImage(username string, userId string) ([]byte, *model.AppError) { +func CreateProfileImage(username string, userId string, initialFont string) ([]byte, *model.AppError) { colors := []color.NRGBA{ {197, 8, 126, 255}, {227, 207, 18, 255}, @@ -702,7 +702,7 @@ func CreateProfileImage(username string, userId string) ([]byte, *model.AppError initial := string(strings.ToUpper(username)[0]) fontDir, _ := utils.FindDir("fonts") - fontBytes, err := ioutil.ReadFile(fontDir + utils.Cfg.FileSettings.InitialFont) + fontBytes, err := ioutil.ReadFile(fontDir + initialFont) if err != nil { return nil, model.NewAppError("CreateProfileImage", "api.user.create_profile_image.default_font.app_error", nil, err.Error(), http.StatusInternalServerError) } @@ -739,13 +739,13 @@ func CreateProfileImage(username string, userId string) ([]byte, *model.AppError } } -func GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) { +func (a *App) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) { var img []byte readFailed := false if len(*utils.Cfg.FileSettings.DriverName) == 0 { var err *model.AppError - if img, err = CreateProfileImage(user.Username, user.Id); err != nil { + if img, err = CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont); err != nil { return nil, false, err } } else { @@ -754,7 +754,7 @@ func GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) { if data, err := utils.ReadFile(path); err != nil { readFailed = true - if img, err = CreateProfileImage(user.Username, user.Id); err != nil { + if img, err = CreateProfileImage(user.Username, user.Id, a.Config().FileSettings.InitialFont); err != nil { return nil, false, err } @@ -932,8 +932,8 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A } } -func SanitizeProfile(user *model.User, asAdmin bool) { - options := utils.Cfg.GetSanitizeOptions() +func (a *App) SanitizeProfile(user *model.User, asAdmin bool) { + options := a.Config().GetSanitizeOptions() if asAdmin { options["email"] = true options["fullname"] = true @@ -972,7 +972,7 @@ func (a *App) PatchUser(userId string, patch *model.UserPatch, asAdmin bool) (*m } func (a *App) sendUpdatedUserEvent(user model.User, asAdmin bool) { - SanitizeProfile(&user, asAdmin) + a.SanitizeProfile(&user, asAdmin) omitUsers := make(map[string]bool, 1) omitUsers[user.Id] = true @@ -1373,7 +1373,7 @@ func (a *App) SearchUsersInChannel(channelId string, term string, searchOptions users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } return users, nil @@ -1387,7 +1387,7 @@ func (a *App) SearchUsersNotInChannel(teamId string, channelId string, term stri users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } return users, nil @@ -1401,7 +1401,7 @@ func (a *App) SearchUsersInTeam(teamId string, term string, searchOptions map[st users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } return users, nil @@ -1415,7 +1415,7 @@ func (a *App) SearchUsersNotInTeam(notInTeamId string, term string, searchOption users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } return users, nil @@ -1429,7 +1429,7 @@ func (a *App) SearchUsersWithoutTeam(term string, searchOptions map[string]bool, users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } return users, nil @@ -1448,7 +1448,7 @@ func (a *App) AutocompleteUsersInChannel(teamId string, channelId string, term s users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } autocomplete.InChannel = users @@ -1460,7 +1460,7 @@ func (a *App) AutocompleteUsersInChannel(teamId string, channelId string, term s users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } autocomplete.OutOfChannel = users @@ -1478,7 +1478,7 @@ func (a *App) AutocompleteUsersInTeam(teamId string, term string, searchOptions users := result.Data.([]*model.User) for _, user := range users { - SanitizeProfile(user, asAdmin) + a.SanitizeProfile(user, asAdmin) } autocomplete.InTeam = users diff --git a/app/user_test.go b/app/user_test.go index 282e2896d..51db207ef 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -16,7 +16,6 @@ import ( "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/model/gitlab" - "github.com/mattermost/mattermost-server/utils" ) func TestIsUsernameTaken(t *testing.T) { @@ -100,9 +99,7 @@ func TestCreateOAuthUser(t *testing.T) { } func TestCreateProfileImage(t *testing.T) { - utils.LoadConfig("config.json") - - b, err := CreateProfileImage("Corey Hulen", "eo1zkdr96pdj98pjmq8zy35wba") + b, err := CreateProfileImage("Corey Hulen", "eo1zkdr96pdj98pjmq8zy35wba", "luximbi.ttf") if err != nil { t.Fatal(err) } -- cgit v1.2.3-1-g7c22