From 85f5f239f2b19fcfc691736458e19cbbe36dc4f2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 28 Apr 2023 07:10:40 +0800 Subject: [PATCH 1/7] Use setting.Init instead of many functions --- cmd/actions.go | 2 +- cmd/cmd.go | 2 +- cmd/doctor.go | 2 +- cmd/dump.go | 2 +- cmd/embedded.go | 5 ++- cmd/mailer.go | 2 +- cmd/restore_repo.go | 2 +- cmd/serv.go | 2 +- cmd/web.go | 2 +- modules/doctor/doctor.go | 2 +- modules/doctor/paths.go | 2 +- modules/markup/html_test.go | 4 ++- modules/markup/markdown/markdown_test.go | 4 ++- modules/setting/config_provider.go | 46 +++++++++++++----------- modules/setting/setting.go | 44 ++++++++++------------- routers/install/setting.go | 6 ++-- 16 files changed, 69 insertions(+), 60 deletions(-) diff --git a/cmd/actions.go b/cmd/actions.go index 66ad336da508e..907eda9a6d899 100644 --- a/cmd/actions.go +++ b/cmd/actions.go @@ -42,7 +42,7 @@ func runGenerateActionsRunnerToken(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() scope := c.String("scope") diff --git a/cmd/cmd.go b/cmd/cmd.go index 18d5db3987bdd..c50bf1bc7a8ab 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -57,7 +57,7 @@ func confirm() (bool, error) { } func initDB(ctx context.Context) error { - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() setting.LoadDBSetting() setting.InitSQLLog(false) diff --git a/cmd/doctor.go b/cmd/doctor.go index e7baad60c1f94..efa9c682c112a 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -87,7 +87,7 @@ func runRecreateTable(ctx *cli.Context) error { golog.SetPrefix("") golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT))) - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() setting.LoadDBSetting() diff --git a/cmd/dump.go b/cmd/dump.go index 309bd01f6645f..f84a88a41885c 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -185,7 +185,7 @@ func runDump(ctx *cli.Context) error { } fileName += "." + outType } - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() // make sure we are logging to the console no matter what the configuration tells us do to diff --git a/cmd/embedded.go b/cmd/embedded.go index cee8928ce08db..03c7a5df1832c 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -106,7 +106,10 @@ func initEmbeddedExtractor(c *cli.Context) error { log.DelNamedLogger(log.DEFAULT) // Read configuration file - setting.InitProviderAllowEmpty() + setting.Init(&setting.Options{ + AllowEmpty: true, + IgnoreCheckRunUser: true, + }) setting.LoadCommonSettings() patterns, err := compileCollectPatterns(c.Args()) diff --git a/cmd/mailer.go b/cmd/mailer.go index 50ba4b474110b..73c1c3cabae61 100644 --- a/cmd/mailer.go +++ b/cmd/mailer.go @@ -16,7 +16,7 @@ func runSendMail(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() if err := argsSet(c, "title"); err != nil { diff --git a/cmd/restore_repo.go b/cmd/restore_repo.go index 887b59bba9e14..2ba2144f910b3 100644 --- a/cmd/restore_repo.go +++ b/cmd/restore_repo.go @@ -51,7 +51,7 @@ func runRestoreRepository(c *cli.Context) error { ctx, cancel := installSignals() defer cancel() - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() var units []string if s := c.String("units"); s != "" { diff --git a/cmd/serv.go b/cmd/serv.go index 72eb6370711e4..4544e85cb4d0b 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -62,7 +62,7 @@ func setup(ctx context.Context, debug bool) { } else { _ = log.NewLogger(1000, "console", "console", `{"level":"fatal","stacktracelevel":"NONE","stderr":true}`) } - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() if debug { setting.RunMode = "dev" diff --git a/cmd/web.go b/cmd/web.go index e451cf7dfa4fe..b273afa847c96 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -177,7 +177,7 @@ func runWeb(ctx *cli.Context) error { log.Info("Global init") // Perform global initialization - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() routers.GlobalInitInstalled(graceful.GetManager().HammerContext()) diff --git a/modules/doctor/doctor.go b/modules/doctor/doctor.go index b23805bc4c963..9af8afb5f0274 100644 --- a/modules/doctor/doctor.go +++ b/modules/doctor/doctor.go @@ -44,7 +44,7 @@ func (w *wrappedLevelLogger) Log(skip int, level log.Level, format string, v ... } func initDBDisableConsole(ctx context.Context, disableConsole bool) error { - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() setting.LoadDBSetting() setting.InitSQLLog(disableConsole) diff --git a/modules/doctor/paths.go b/modules/doctor/paths.go index 1558efc25b90c..f04f690e0c8c3 100644 --- a/modules/doctor/paths.go +++ b/modules/doctor/paths.go @@ -66,7 +66,7 @@ func checkConfigurationFiles(ctx context.Context, logger log.Logger, autofix boo return err } - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() configurationFiles := []configurationFile{ diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index cb1216ec946ee..cdd6a5ee86edc 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -28,7 +28,9 @@ var localMetas = map[string]string{ } func TestMain(m *testing.M) { - setting.InitProviderAllowEmpty() + setting.Init(&setting.Options{ + AllowEmpty: true, + }) setting.LoadCommonSettings() if err := git.InitSimple(context.Background()); err != nil { log.Fatal("git init failed, err: %v", err) diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 0c7650a5ffabc..96fe836f234d4 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -33,7 +33,9 @@ var localMetas = map[string]string{ } func TestMain(m *testing.M) { - setting.InitProviderAllowEmpty() + setting.Init(&setting.Options{ + AllowEmpty: true, + }) setting.LoadCommonSettings() if err := git.InitSimple(context.Background()); err != nil { log.Fatal("git init failed, err: %v", err) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 92c8c97fe9528..027f74848c635 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -35,10 +35,9 @@ type ConfigProvider interface { } type iniFileConfigProvider struct { + opts *Options *ini.File - filepath string // the ini file path - newFile bool // whether the file has not existed previously - allowEmpty bool // whether not finding configuration files is allowed (only true for the tests) + newFile bool // whether the file has not existed previously } // NewEmptyConfigProvider create a new empty config provider @@ -66,41 +65,48 @@ func newConfigProviderFromData(configContent string) (ConfigProvider, error) { }, nil } +type Options struct { + CustomConf string // the ini file path + AllowEmpty bool // whether not finding configuration files is allowed (only true for the tests) + ExtraConfig string + IgnoreCheckRunUser bool +} + // newConfigProviderFromFile load configuration from file. // NOTE: do not print any log except error. -func newConfigProviderFromFile(customConf string, allowEmpty bool, extraConfig string) (*iniFileConfigProvider, error) { +func newConfigProviderFromFile(opts *Options) (*iniFileConfigProvider, error) { cfg := ini.Empty() newFile := true - if customConf != "" { - isFile, err := util.IsFile(customConf) + if opts.CustomConf != "" { + isFile, err := util.IsFile(opts.CustomConf) if err != nil { - return nil, fmt.Errorf("unable to check if %s is a file. Error: %v", customConf, err) + return nil, fmt.Errorf("unable to check if %s is a file. Error: %v", opts.CustomConf, err) } if isFile { - if err := cfg.Append(customConf); err != nil { - return nil, fmt.Errorf("failed to load custom conf '%s': %v", customConf, err) + if err := cfg.Append(opts.CustomConf); err != nil { + return nil, fmt.Errorf("failed to load custom conf '%s': %v", opts.CustomConf, err) } newFile = false } } - if newFile && !allowEmpty { + if newFile && !opts.AllowEmpty { return nil, fmt.Errorf("unable to find configuration file: %q, please ensure you are running in the correct environment or set the correct configuration file with -c", CustomConf) } - if extraConfig != "" { - if err := cfg.Append([]byte(extraConfig)); err != nil { + if opts.ExtraConfig != "" { + if err := cfg.Append([]byte(opts.ExtraConfig)); err != nil { return nil, fmt.Errorf("unable to append more config: %v", err) } } cfg.NameMapper = ini.SnackCase + ignoreCheckRunUser = opts.IgnoreCheckRunUser return &iniFileConfigProvider{ - File: cfg, - filepath: customConf, - newFile: newFile, - allowEmpty: allowEmpty, + opts: opts, + File: cfg, + newFile: newFile, }, nil } @@ -123,8 +129,8 @@ func (p *iniFileConfigProvider) DeleteSection(name string) error { // Save save the content into file func (p *iniFileConfigProvider) Save() error { - if p.filepath == "" { - if !p.allowEmpty { + if p.opts.CustomConf == "" { + if !p.opts.AllowEmpty { return fmt.Errorf("custom config path must not be empty") } return nil @@ -135,8 +141,8 @@ func (p *iniFileConfigProvider) Save() error { return fmt.Errorf("failed to create '%s': %v", CustomConf, err) } } - if err := p.SaveTo(p.filepath); err != nil { - return fmt.Errorf("failed to save '%s': %v", p.filepath, err) + if err := p.SaveTo(p.opts.CustomConf); err != nil { + return fmt.Errorf("failed to save '%s': %v", p.opts.CustomConf, err) } // Change permissions to be more restrictive diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 7a1b7d17a64dc..29dd824599d26 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -39,13 +39,14 @@ var ( // Other global setting objects - CfgProvider ConfigProvider - CustomPath string // Custom directory path - CustomConf string - RunMode string - RunUser string - IsProd bool - IsWindows bool + CfgProvider ConfigProvider + CustomPath string // Custom directory path + CustomConf string + RunMode string + RunUser string + IsProd bool + IsWindows bool + ignoreCheckRunUser bool // IsInTesting indicates whether the testing is running. A lot of unreliable code causes a lot of nonsense error logs during testing // TODO: this is only a temporary solution, we should make the test code more reliable @@ -136,7 +137,7 @@ func forcePathSeparator(path string) { // This check is ignored under Windows since SSH remote login is not the main // method to login on Windows. func IsRunUserMatchCurrentUser(runUser string) (string, bool) { - if IsWindows || SSH.StartBuiltinServer { + if IsWindows || SSH.StartBuiltinServer || ignoreCheckRunUser { return "", true } @@ -203,31 +204,24 @@ func PrepareAppDataPath() error { return nil } -// InitProviderFromExistingFile initializes config provider from an existing config file (app.ini) -func InitProviderFromExistingFile() { - var err error - CfgProvider, err = newConfigProviderFromFile(CustomConf, false, "") - if err != nil { - log.Fatal("InitProviderFromExistingFile: %v", err) +func Init(opts *Options) { + if opts.CustomConf == "" { + opts.CustomConf = CustomConf } -} - -// InitProviderAllowEmpty initializes config provider from file, it's also fine that if the config file (app.ini) doesn't exist -func InitProviderAllowEmpty() { var err error - CfgProvider, err = newConfigProviderFromFile(CustomConf, true, "") + CfgProvider, err = newConfigProviderFromFile(opts) if err != nil { - log.Fatal("InitProviderAllowEmpty: %v", err) + log.Fatal("Init[%v]: %v", opts, err) } } // InitProviderAndLoadCommonSettingsForTest initializes config provider and load common setttings for tests func InitProviderAndLoadCommonSettingsForTest(extraConfigs ...string) { - var err error - CfgProvider, err = newConfigProviderFromFile(CustomConf, true, strings.Join(extraConfigs, "\n")) - if err != nil { - log.Fatal("InitProviderAndLoadCommonSettingsForTest: %v", err) - } + Init(&Options{ + AllowEmpty: true, + ExtraConfig: strings.Join(extraConfigs, "\n"), + }) + loadCommonSettingsFrom(CfgProvider) if err := PrepareAppDataPath(); err != nil { log.Fatal("Can not prepare APP_DATA_PATH: %v", err) diff --git a/routers/install/setting.go b/routers/install/setting.go index dadefa26a2930..dd32c60f7da1d 100644 --- a/routers/install/setting.go +++ b/routers/install/setting.go @@ -15,7 +15,9 @@ import ( // PreloadSettings preloads the configuration to check if we need to run install func PreloadSettings(ctx context.Context) bool { - setting.InitProviderAllowEmpty() + setting.Init(&setting.Options{ + AllowEmpty: true, + }) setting.LoadCommonSettings() if !setting.InstallLock { log.Info("AppPath: %s", setting.AppPath) @@ -38,7 +40,7 @@ func PreloadSettings(ctx context.Context) bool { // reloadSettings reloads the existing settings and starts up the database func reloadSettings(ctx context.Context) { - setting.InitProviderFromExistingFile() + setting.Init(&setting.Options{}) setting.LoadCommonSettings() setting.LoadDBSetting() if setting.InstallLock { From 70fde789dfd1e0761c9fcd90aed8b046f3165580 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 28 Apr 2023 07:21:33 +0800 Subject: [PATCH 2/7] Move init test setting to unittest package --- cmd/main_test.go | 6 ----- models/asymkey/main_test.go | 6 ----- models/dbfs/main_test.go | 6 ----- models/issues/main_test.go | 6 ----- models/main_test.go | 6 ----- models/migrations/base/tests.go | 2 +- models/unittest/testdb.go | 23 +++++++++++++++++++ modules/setting/setting.go | 18 --------------- routers/api/v1/repo/main_test.go | 7 +++--- services/webhook/main_test.go | 8 +++---- .../migration-test/migration_test.go | 2 +- tests/test_utils.go | 2 +- 12 files changed, 34 insertions(+), 58 deletions(-) diff --git a/cmd/main_test.go b/cmd/main_test.go index ba323af472178..6e20be69451c3 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -7,14 +7,8 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/setting" ) -func init() { - setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() -} - func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: "..", diff --git a/models/asymkey/main_test.go b/models/asymkey/main_test.go index 7f8657189fd18..701722be12ec8 100644 --- a/models/asymkey/main_test.go +++ b/models/asymkey/main_test.go @@ -8,14 +8,8 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/setting" ) -func init() { - setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() -} - func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), diff --git a/models/dbfs/main_test.go b/models/dbfs/main_test.go index 9dce663eeab99..62db3592bed2a 100644 --- a/models/dbfs/main_test.go +++ b/models/dbfs/main_test.go @@ -8,14 +8,8 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/setting" ) -func init() { - setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() -} - func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), diff --git a/models/issues/main_test.go b/models/issues/main_test.go index de84da30ecc0a..9fbe294f7067d 100644 --- a/models/issues/main_test.go +++ b/models/issues/main_test.go @@ -9,7 +9,6 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" - "code.gitea.io/gitea/modules/setting" _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/repo" @@ -18,11 +17,6 @@ import ( "github.com/stretchr/testify/assert" ) -func init() { - setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() -} - func TestFixturesAreConsistent(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) unittest.CheckConsistencyFor(t, diff --git a/models/main_test.go b/models/main_test.go index b5919bb28615f..d490507649a33 100644 --- a/models/main_test.go +++ b/models/main_test.go @@ -11,18 +11,12 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" _ "code.gitea.io/gitea/models/system" "github.com/stretchr/testify/assert" ) -func init() { - setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() -} - // TestFixturesAreConsistent assert that test fixtures are consistent func TestFixturesAreConsistent(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/migrations/base/tests.go b/models/migrations/base/tests.go index 14f374f1a7c3d..124111f51fb2f 100644 --- a/models/migrations/base/tests.go +++ b/models/migrations/base/tests.go @@ -150,7 +150,7 @@ func MainTest(m *testing.M) { setting.AppDataPath = tmpDataPath setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() + unittest.InitSettings() if err = git.InitFull(context.Background()); err != nil { fmt.Printf("Unable to InitFull: %v\n", err) os.Exit(1) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index cff1489a7c7bc..265f30ae720d9 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -6,12 +6,15 @@ package unittest import ( "context" "fmt" + "log" "os" "path/filepath" + "strings" "testing" "code.gitea.io/gitea/models/db" system_model "code.gitea.io/gitea/models/system" + "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" @@ -39,6 +42,23 @@ func fatalTestError(fmtStr string, args ...interface{}) { os.Exit(1) } +// InitSettings initializes config provider and load common setttings for tests +func InitSettings(extraConfigs ...string) { + setting.Init(&setting.Options{ + AllowEmpty: true, + ExtraConfig: strings.Join(extraConfigs, "\n"), + }) + + setting.LoadCommonSettings() + if err := setting.PrepareAppDataPath(); err != nil { + log.Fatal("Can not prepare APP_DATA_PATH: %v", err) + } + // register the dummy hash algorithm function used in the test fixtures + _ = hash.Register("dummy", hash.NewDummyHasher) + + setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy") +} + // TestOptions represents test options type TestOptions struct { GiteaRootPath string @@ -50,6 +70,9 @@ type TestOptions struct { // MainTest a reusable TestMain(..) function for unit tests that need to use a // test database. Creates the test database, and sets necessary settings. func MainTest(m *testing.M, testOpts *TestOptions) { + setting.SetCustomPathAndConf("", "", "") + InitSettings() + var err error giteaRoot = testOpts.GiteaRootPath diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 29dd824599d26..0c1fa13e45edd 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -15,7 +15,6 @@ import ( "strings" "time" - "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/user" ) @@ -215,23 +214,6 @@ func Init(opts *Options) { } } -// InitProviderAndLoadCommonSettingsForTest initializes config provider and load common setttings for tests -func InitProviderAndLoadCommonSettingsForTest(extraConfigs ...string) { - Init(&Options{ - AllowEmpty: true, - ExtraConfig: strings.Join(extraConfigs, "\n"), - }) - - loadCommonSettingsFrom(CfgProvider) - if err := PrepareAppDataPath(); err != nil { - log.Fatal("Can not prepare APP_DATA_PATH: %v", err) - } - // register the dummy hash algorithm function used in the test fixtures - _ = hash.Register("dummy", hash.NewDummyHasher) - - PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy") -} - // LoadCommonSettings loads common configurations from a configuration provider. func LoadCommonSettings() { loadCommonSettingsFrom(CfgProvider) diff --git a/routers/api/v1/repo/main_test.go b/routers/api/v1/repo/main_test.go index c7466c493f015..bc048505f472f 100644 --- a/routers/api/v1/repo/main_test.go +++ b/routers/api/v1/repo/main_test.go @@ -13,10 +13,11 @@ import ( ) func TestMain(m *testing.M) { - setting.InitProviderAndLoadCommonSettingsForTest() - setting.LoadQueueSettings() unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", "..", "..", ".."), - SetUp: webhook_service.Init, + SetUp: func() error { + setting.LoadQueueSettings() + return webhook_service.Init() + }, }) } diff --git a/services/webhook/main_test.go b/services/webhook/main_test.go index 210221b120413..0189e17840165 100644 --- a/services/webhook/main_test.go +++ b/services/webhook/main_test.go @@ -15,13 +15,13 @@ import ( ) func TestMain(m *testing.M) { - setting.InitProviderAndLoadCommonSettingsForTest() - setting.LoadQueueSettings() - // for tests, allow only loopback IPs setting.Webhook.AllowedHostList = hostmatcher.MatchBuiltinLoopback unittest.MainTest(m, &unittest.TestOptions{ GiteaRootPath: filepath.Join("..", ".."), - SetUp: Init, + SetUp: func() error { + setting.LoadQueueSettings() + return Init() + }, }) } diff --git a/tests/integration/migration-test/migration_test.go b/tests/integration/migration-test/migration_test.go index 4152379a9b80a..a68d458a0b1ea 100644 --- a/tests/integration/migration-test/migration_test.go +++ b/tests/integration/migration-test/migration_test.go @@ -57,7 +57,7 @@ func initMigrationTest(t *testing.T) func() { setting.CustomConf = giteaConf } - setting.InitProviderAndLoadCommonSettingsForTest() + unittest.InitSettings() assert.True(t, len(setting.RepoRootPath) != 0) assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) diff --git a/tests/test_utils.go b/tests/test_utils.go index 0f4aa26a60742..c22b2c356c656 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -74,7 +74,7 @@ func InitTest(requireGitea bool) { } setting.SetCustomPathAndConf("", "", "") - setting.InitProviderAndLoadCommonSettingsForTest() + unittest.InitSettings() setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath()) From 69a8b1a553b07609b4c8deb87f878f5eb22f8822 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 28 Apr 2023 07:28:35 +0800 Subject: [PATCH 3/7] Fix lint --- models/unittest/testdb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index 265f30ae720d9..c0e7d48a94fe6 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -51,7 +51,7 @@ func InitSettings(extraConfigs ...string) { setting.LoadCommonSettings() if err := setting.PrepareAppDataPath(); err != nil { - log.Fatal("Can not prepare APP_DATA_PATH: %v", err) + log.Fatalf("Can not prepare APP_DATA_PATH: %v", err) } // register the dummy hash algorithm function used in the test fixtures _ = hash.Register("dummy", hash.NewDummyHasher) From 628940806d9675f7055561494e227730e6053029 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 28 Apr 2023 11:55:09 +0800 Subject: [PATCH 4/7] Fix test --- routers/web/user/setting/account_test.go | 27 +++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/routers/web/user/setting/account_test.go b/routers/web/user/setting/account_test.go index 5fce41f065ba4..569d597722ee3 100644 --- a/routers/web/user/setting/account_test.go +++ b/routers/web/user/setting/account_test.go @@ -80,19 +80,22 @@ func TestChangePassword(t *testing.T) { PasswordComplexity: pcLU, }, } { - unittest.PrepareTestEnv(t) - ctx := test.MockContext(t, "user/settings/security") - test.LoadUser(t, ctx, 2) - test.LoadRepo(t, ctx, 1) + t.Run(req.OldPassword+"__"+req.NewPassword, func(t *testing.T) { + unittest.PrepareTestEnv(t) + setting.PasswordComplexity = req.PasswordComplexity + ctx := test.MockContext(t, "user/settings/security") + test.LoadUser(t, ctx, 2) + test.LoadRepo(t, ctx, 1) - web.SetForm(ctx, &forms.ChangePasswordForm{ - OldPassword: req.OldPassword, - Password: req.NewPassword, - Retype: req.Retype, - }) - AccountPost(ctx) + web.SetForm(ctx, &forms.ChangePasswordForm{ + OldPassword: req.OldPassword, + Password: req.NewPassword, + Retype: req.Retype, + }) + AccountPost(ctx) - assert.Contains(t, ctx.Flash.ErrorMsg, req.Message) - assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) + assert.Contains(t, ctx.Flash.ErrorMsg, req.Message) + assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) + }) } } From 0a2a9920a79404374334c3e2dc2fc65c182e901a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 29 Apr 2023 18:24:42 +0800 Subject: [PATCH 5/7] adjust ignore location --- modules/setting/setting.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 0c1fa13e45edd..35553554aa88f 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -136,7 +136,7 @@ func forcePathSeparator(path string) { // This check is ignored under Windows since SSH remote login is not the main // method to login on Windows. func IsRunUserMatchCurrentUser(runUser string) (string, bool) { - if IsWindows || SSH.StartBuiltinServer || ignoreCheckRunUser { + if IsWindows || SSH.StartBuiltinServer { return "", true } @@ -260,7 +260,7 @@ func loadRunModeFrom(rootCfg ConfigProvider) { IsProd = strings.EqualFold(RunMode, "prod") // Does not check run user when the install lock is off. installLock := rootCfg.Section("security").Key("INSTALL_LOCK").MustBool(false) - if installLock { + if installLock && !ignoreCheckRunUser { currentUser, match := IsRunUserMatchCurrentUser(RunUser) if !match { log.Fatal("Expect user '%s' but current user is: %s", RunUser, currentUser) From 055b9ec7bdf53c0e7e8a4dc60531319638ef4b73 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 29 Apr 2023 20:14:34 +0800 Subject: [PATCH 6/7] Move loadCommonSettingsFrom into Init --- cmd/actions.go | 1 - cmd/cmd.go | 1 - cmd/doctor.go | 1 - cmd/dump.go | 1 - cmd/embedded.go | 1 - cmd/mailer.go | 1 - cmd/restore_repo.go | 1 - cmd/serv.go | 1 - cmd/web.go | 1 - models/unittest/testdb.go | 1 - modules/doctor/doctor.go | 1 - modules/doctor/paths.go | 1 - modules/markup/html_test.go | 1 - modules/markup/markdown/markdown_test.go | 1 - modules/setting/config_provider.go | 9 +++++---- modules/setting/setting.go | 8 +++----- routers/install/setting.go | 2 -- 17 files changed, 8 insertions(+), 25 deletions(-) diff --git a/cmd/actions.go b/cmd/actions.go index 907eda9a6d899..346de5b21a6fc 100644 --- a/cmd/actions.go +++ b/cmd/actions.go @@ -43,7 +43,6 @@ func runGenerateActionsRunnerToken(c *cli.Context) error { defer cancel() setting.Init(&setting.Options{}) - setting.LoadCommonSettings() scope := c.String("scope") diff --git a/cmd/cmd.go b/cmd/cmd.go index c50bf1bc7a8ab..cf2d9ef89e831 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -58,7 +58,6 @@ func confirm() (bool, error) { func initDB(ctx context.Context) error { setting.Init(&setting.Options{}) - setting.LoadCommonSettings() setting.LoadDBSetting() setting.InitSQLLog(false) diff --git a/cmd/doctor.go b/cmd/doctor.go index efa9c682c112a..65c028c5ed19d 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -88,7 +88,6 @@ func runRecreateTable(ctx *cli.Context) error { golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT))) setting.Init(&setting.Options{}) - setting.LoadCommonSettings() setting.LoadDBSetting() setting.Log.EnableXORMLog = ctx.Bool("debug") diff --git a/cmd/dump.go b/cmd/dump.go index f84a88a41885c..32ccc5566c8a9 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -186,7 +186,6 @@ func runDump(ctx *cli.Context) error { fileName += "." + outType } setting.Init(&setting.Options{}) - setting.LoadCommonSettings() // make sure we are logging to the console no matter what the configuration tells us do to // FIXME: don't use CfgProvider directly diff --git a/cmd/embedded.go b/cmd/embedded.go index 03c7a5df1832c..fa25625774d4f 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -110,7 +110,6 @@ func initEmbeddedExtractor(c *cli.Context) error { AllowEmpty: true, IgnoreCheckRunUser: true, }) - setting.LoadCommonSettings() patterns, err := compileCollectPatterns(c.Args()) if err != nil { diff --git a/cmd/mailer.go b/cmd/mailer.go index 73c1c3cabae61..74bae1ab68c76 100644 --- a/cmd/mailer.go +++ b/cmd/mailer.go @@ -17,7 +17,6 @@ func runSendMail(c *cli.Context) error { defer cancel() setting.Init(&setting.Options{}) - setting.LoadCommonSettings() if err := argsSet(c, "title"); err != nil { return err diff --git a/cmd/restore_repo.go b/cmd/restore_repo.go index 2ba2144f910b3..5a7ede4939756 100644 --- a/cmd/restore_repo.go +++ b/cmd/restore_repo.go @@ -52,7 +52,6 @@ func runRestoreRepository(c *cli.Context) error { defer cancel() setting.Init(&setting.Options{}) - setting.LoadCommonSettings() var units []string if s := c.String("units"); s != "" { units = strings.Split(s, ",") diff --git a/cmd/serv.go b/cmd/serv.go index 4544e85cb4d0b..a79f314d00b69 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -63,7 +63,6 @@ func setup(ctx context.Context, debug bool) { _ = log.NewLogger(1000, "console", "console", `{"level":"fatal","stacktracelevel":"NONE","stderr":true}`) } setting.Init(&setting.Options{}) - setting.LoadCommonSettings() if debug { setting.RunMode = "dev" } diff --git a/cmd/web.go b/cmd/web.go index b273afa847c96..3a01d07b05f59 100644 --- a/cmd/web.go +++ b/cmd/web.go @@ -178,7 +178,6 @@ func runWeb(ctx *cli.Context) error { log.Info("Global init") // Perform global initialization setting.Init(&setting.Options{}) - setting.LoadCommonSettings() routers.GlobalInitInstalled(graceful.GetManager().HammerContext()) // We check that AppDataPath exists here (it should have been created during installation) diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index c0e7d48a94fe6..a5b126350d8e1 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -49,7 +49,6 @@ func InitSettings(extraConfigs ...string) { ExtraConfig: strings.Join(extraConfigs, "\n"), }) - setting.LoadCommonSettings() if err := setting.PrepareAppDataPath(); err != nil { log.Fatalf("Can not prepare APP_DATA_PATH: %v", err) } diff --git a/modules/doctor/doctor.go b/modules/doctor/doctor.go index 9af8afb5f0274..32eb5938c307c 100644 --- a/modules/doctor/doctor.go +++ b/modules/doctor/doctor.go @@ -45,7 +45,6 @@ func (w *wrappedLevelLogger) Log(skip int, level log.Level, format string, v ... func initDBDisableConsole(ctx context.Context, disableConsole bool) error { setting.Init(&setting.Options{}) - setting.LoadCommonSettings() setting.LoadDBSetting() setting.InitSQLLog(disableConsole) if err := db.InitEngine(ctx); err != nil { diff --git a/modules/doctor/paths.go b/modules/doctor/paths.go index f04f690e0c8c3..957152349c25f 100644 --- a/modules/doctor/paths.go +++ b/modules/doctor/paths.go @@ -67,7 +67,6 @@ func checkConfigurationFiles(ctx context.Context, logger log.Logger, autofix boo } setting.Init(&setting.Options{}) - setting.LoadCommonSettings() configurationFiles := []configurationFile{ {"Configuration File Path", setting.CustomConf, false, true, false}, diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index cdd6a5ee86edc..5e5e4fecbbb7f 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -31,7 +31,6 @@ func TestMain(m *testing.M) { setting.Init(&setting.Options{ AllowEmpty: true, }) - setting.LoadCommonSettings() if err := git.InitSimple(context.Background()); err != nil { log.Fatal("git init failed, err: %v", err) } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 96fe836f234d4..e81869d7a4436 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -36,7 +36,6 @@ func TestMain(m *testing.M) { setting.Init(&setting.Options{ AllowEmpty: true, }) - setting.LoadCommonSettings() if err := git.InitSimple(context.Background()); err != nil { log.Fatal("git init failed, err: %v", err) } diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index 027f74848c635..f01f7a4b5ac4a 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -66,10 +66,11 @@ func newConfigProviderFromData(configContent string) (ConfigProvider, error) { } type Options struct { - CustomConf string // the ini file path - AllowEmpty bool // whether not finding configuration files is allowed (only true for the tests) - ExtraConfig string - IgnoreCheckRunUser bool + CustomConf string // the ini file path + AllowEmpty bool // whether not finding configuration files is allowed (only true for the tests) + ExtraConfig string + IgnoreCheckRunUser bool + DisableLoadCommonSettings bool } // newConfigProviderFromFile load configuration from file. diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 35553554aa88f..370f762f8a69a 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -212,11 +212,9 @@ func Init(opts *Options) { if err != nil { log.Fatal("Init[%v]: %v", opts, err) } -} - -// LoadCommonSettings loads common configurations from a configuration provider. -func LoadCommonSettings() { - loadCommonSettingsFrom(CfgProvider) + if !opts.DisableLoadCommonSettings { + loadCommonSettingsFrom(CfgProvider) + } } // loadCommonSettingsFrom loads common configurations from a configuration provider. diff --git a/routers/install/setting.go b/routers/install/setting.go index dd32c60f7da1d..c14843d8ee4d3 100644 --- a/routers/install/setting.go +++ b/routers/install/setting.go @@ -18,7 +18,6 @@ func PreloadSettings(ctx context.Context) bool { setting.Init(&setting.Options{ AllowEmpty: true, }) - setting.LoadCommonSettings() if !setting.InstallLock { log.Info("AppPath: %s", setting.AppPath) log.Info("AppWorkPath: %s", setting.AppWorkPath) @@ -41,7 +40,6 @@ func PreloadSettings(ctx context.Context) bool { // reloadSettings reloads the existing settings and starts up the database func reloadSettings(ctx context.Context) { setting.Init(&setting.Options{}) - setting.LoadCommonSettings() setting.LoadDBSetting() if setting.InstallLock { if err := common.InitDBEngine(ctx); err == nil { From 1b75d060a54b9ee09847f1dafa92a3773a68eb8d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 2 May 2023 21:39:28 +0800 Subject: [PATCH 7/7] Remove ignoreCheckRunUser --- cmd/embedded.go | 3 +-- modules/setting/config_provider.go | 2 -- modules/setting/setting.go | 15 +++++++-------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/embedded.go b/cmd/embedded.go index fa25625774d4f..3f849bea0a264 100644 --- a/cmd/embedded.go +++ b/cmd/embedded.go @@ -107,8 +107,7 @@ func initEmbeddedExtractor(c *cli.Context) error { // Read configuration file setting.Init(&setting.Options{ - AllowEmpty: true, - IgnoreCheckRunUser: true, + AllowEmpty: true, }) patterns, err := compileCollectPatterns(c.Args()) diff --git a/modules/setting/config_provider.go b/modules/setting/config_provider.go index f01f7a4b5ac4a..1685958298635 100644 --- a/modules/setting/config_provider.go +++ b/modules/setting/config_provider.go @@ -69,7 +69,6 @@ type Options struct { CustomConf string // the ini file path AllowEmpty bool // whether not finding configuration files is allowed (only true for the tests) ExtraConfig string - IgnoreCheckRunUser bool DisableLoadCommonSettings bool } @@ -103,7 +102,6 @@ func newConfigProviderFromFile(opts *Options) (*iniFileConfigProvider, error) { } cfg.NameMapper = ini.SnackCase - ignoreCheckRunUser = opts.IgnoreCheckRunUser return &iniFileConfigProvider{ opts: opts, File: cfg, diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 0628010d7c57c..b085a7b321495 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -38,14 +38,13 @@ var ( // Other global setting objects - CfgProvider ConfigProvider - CustomPath string // Custom directory path - CustomConf string - RunMode string - RunUser string - IsProd bool - IsWindows bool - ignoreCheckRunUser bool + CfgProvider ConfigProvider + CustomPath string // Custom directory path + CustomConf string + RunMode string + RunUser string + IsProd bool + IsWindows bool // IsInTesting indicates whether the testing is running. A lot of unreliable code causes a lot of nonsense error logs during testing // TODO: this is only a temporary solution, we should make the test code more reliable