Skip to content

Commit

Permalink
Refactor the setting to make unit test easier (#22405)
Browse files Browse the repository at this point in the history
Some bugs caused by less unit tests in fundamental packages. This PR
refactor `setting` package so that create a unit test will be easier
than before.

- All `LoadFromXXX` files has been splited as two functions, one is
`InitProviderFromXXX` and `LoadCommonSettings`. The first functions will
only include the code to create or new a ini file. The second function
will load common settings.
- It also renames all functions in setting from `newXXXService` to
`loadXXXSetting` or `loadXXXFrom` to make the function name less
confusing.
- Move `XORMLog` to `SQLLog` because it's a better name for that.

Maybe we should finally move these `loadXXXSetting` into the `XXXInit`
function? Any idea?

---------

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: delvh <dev.lh@web.de>
  • Loading branch information
3 people authored Feb 19, 2023
1 parent 2b02343 commit c53ad05
Show file tree
Hide file tree
Showing 86 changed files with 1,692 additions and 1,462 deletions.
7 changes: 4 additions & 3 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ func confirm() (bool, error) {
}

func initDB(ctx context.Context) error {
setting.LoadFromExisting()
setting.InitDBConfig()
setting.NewXORMLogService(false)
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
setting.LoadDBSetting()
setting.InitSQLLog(false)

if setting.Database.Type == "" {
log.Fatal(`Database settings are missing from the configuration file: %q.
Expand Down
2 changes: 1 addition & 1 deletion cmd/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func runConvert(ctx *cli.Context) error {
log.Info("AppPath: %s", setting.AppPath)
log.Info("AppWorkPath: %s", setting.AppWorkPath)
log.Info("Custom path: %s", setting.CustomPath)
log.Info("Log path: %s", setting.LogRootPath)
log.Info("Log path: %s", setting.Log.RootPath)
log.Info("Configuration file: %s", setting.CustomConf)

if !setting.Database.UseMySQL {
Expand Down
12 changes: 7 additions & 5 deletions cmd/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,16 @@ func runRecreateTable(ctx *cli.Context) error {
golog.SetPrefix("")
golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT)))

setting.LoadFromExisting()
setting.InitDBConfig()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
setting.LoadDBSetting()

setting.EnableXORMLog = ctx.Bool("debug")
setting.Log.EnableXORMLog = ctx.Bool("debug")
setting.Database.LogSQL = ctx.Bool("debug")
setting.Cfg.Section("log").Key("XORM").SetValue(",")
// FIXME: don't use CfgProvider directly
setting.CfgProvider.Section("log").Key("XORM").SetValue(",")

setting.NewXORMLogService(!ctx.Bool("debug"))
setting.InitSQLLog(!ctx.Bool("debug"))
stdCtx, cancel := installSignals()
defer cancel()

Expand Down
20 changes: 11 additions & 9 deletions cmd/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,20 +181,22 @@ func runDump(ctx *cli.Context) error {
}
fileName += "." + outType
}
setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()

// make sure we are logging to the console no matter what the configuration tells us do to
if _, err := setting.Cfg.Section("log").NewKey("MODE", "console"); err != nil {
// FIXME: don't use CfgProvider directly
if _, err := setting.CfgProvider.Section("log").NewKey("MODE", "console"); err != nil {
fatal("Setting logging mode to console failed: %v", err)
}
if _, err := setting.Cfg.Section("log.console").NewKey("STDERR", "true"); err != nil {
if _, err := setting.CfgProvider.Section("log.console").NewKey("STDERR", "true"); err != nil {
fatal("Setting console logger to stderr failed: %v", err)
}
if !setting.InstallLock {
log.Error("Is '%s' really the right config path?\n", setting.CustomConf)
return fmt.Errorf("gitea is not initialized")
}
setting.NewServices() // cannot access session settings otherwise
setting.LoadSettings() // cannot access session settings otherwise

stdCtx, cancel := installSignals()
defer cancel()
Expand Down Expand Up @@ -322,7 +324,7 @@ func runDump(ctx *cli.Context) error {
log.Info("Packing data directory...%s", setting.AppDataPath)

var excludes []string
if setting.Cfg.Section("session").Key("PROVIDER").Value() == "file" {
if setting.SessionConfig.OriginalProvider == "file" {
var opts session.Options
if err = json.Unmarshal([]byte(setting.SessionConfig.ProviderConfig), &opts); err != nil {
return err
Expand All @@ -339,7 +341,7 @@ func runDump(ctx *cli.Context) error {
excludes = append(excludes, setting.LFS.Path)
excludes = append(excludes, setting.Attachment.Path)
excludes = append(excludes, setting.Packages.Path)
excludes = append(excludes, setting.LogRootPath)
excludes = append(excludes, setting.Log.RootPath)
excludes = append(excludes, absFileName)
if err := addRecursiveExclude(w, "data", setting.AppDataPath, excludes, verbose); err != nil {
fatal("Failed to include data directory: %v", err)
Expand Down Expand Up @@ -378,12 +380,12 @@ func runDump(ctx *cli.Context) error {
if ctx.IsSet("skip-log") && ctx.Bool("skip-log") {
log.Info("Skip dumping log files")
} else {
isExist, err := util.IsExist(setting.LogRootPath)
isExist, err := util.IsExist(setting.Log.RootPath)
if err != nil {
log.Error("Unable to check if %s exists. Error: %v", setting.LogRootPath, err)
log.Error("Unable to check if %s exists. Error: %v", setting.Log.RootPath, err)
}
if isExist {
if err := addRecursiveExclude(w, "log", setting.LogRootPath, []string{absFileName}, verbose); err != nil {
if err := addRecursiveExclude(w, "log", setting.Log.RootPath, []string{absFileName}, verbose); err != nil {
fatal("Failed to include log: %v", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/dump_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func runDumpRepository(ctx *cli.Context) error {
log.Info("AppPath: %s", setting.AppPath)
log.Info("AppWorkPath: %s", setting.AppWorkPath)
log.Info("Custom path: %s", setting.CustomPath)
log.Info("Log path: %s", setting.LogRootPath)
log.Info("Log path: %s", setting.Log.RootPath)
log.Info("Configuration file: %s", setting.CustomConf)

var (
Expand Down
3 changes: 2 additions & 1 deletion cmd/embedded.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ func initEmbeddedExtractor(c *cli.Context) error {
log.DelNamedLogger(log.DEFAULT)

// Read configuration file
setting.LoadAllowEmpty()
setting.InitProviderAllowEmpty()
setting.LoadCommonSettings()

pats, err := getPatterns(c.Args())
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion cmd/mailer.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ func runSendMail(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()

setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()

if err := argsSet(c, "title"); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

func init() {
setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
}

func TestMain(m *testing.M) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func runMigrate(ctx *cli.Context) error {
log.Info("AppPath: %s", setting.AppPath)
log.Info("AppWorkPath: %s", setting.AppWorkPath)
log.Info("Custom path: %s", setting.CustomPath)
log.Info("Log path: %s", setting.LogRootPath)
log.Info("Log path: %s", setting.Log.RootPath)
log.Info("Configuration file: %s", setting.CustomConf)

if err := db.InitEngineWithMigration(context.Background(), migrations.Migrate); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/migrate_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func runMigrateStorage(ctx *cli.Context) error {
log.Info("AppPath: %s", setting.AppPath)
log.Info("AppWorkPath: %s", setting.AppWorkPath)
log.Info("Custom path: %s", setting.CustomPath)
log.Info("Log path: %s", setting.LogRootPath)
log.Info("Log path: %s", setting.Log.RootPath)
log.Info("Configuration file: %s", setting.CustomConf)

if err := db.InitEngineWithMigration(context.Background(), migrations.Migrate); err != nil {
Expand Down
3 changes: 2 additions & 1 deletion cmd/restore_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ func runRestoreRepository(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()

setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
var units []string
if s := c.String("units"); s != "" {
units = strings.Split(s, ",")
Expand Down
3 changes: 2 additions & 1 deletion cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func setup(logPath string, debug bool) {
} else {
_ = log.NewLogger(1000, "console", "console", `{"level":"fatal","stacktracelevel":"NONE","stderr":true}`)
}
setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
if debug {
setting.RunMode = "dev"
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ func runWeb(ctx *cli.Context) error {

log.Info("Global init")
// Perform global initialization
setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
routers.GlobalInitInstalled(graceful.GetManager().HammerContext())

// We check that AppDataPath exists here (it should have been created during installation)
Expand Down
5 changes: 3 additions & 2 deletions contrib/pr/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func runPR() {
log.Fatal(err)
}
setting.SetCustomPathAndConf("", "", "")
setting.LoadAllowEmpty()
setting.InitProviderAllowEmpty()
setting.LoadCommonSettings()

setting.RepoRootPath, err = os.MkdirTemp(os.TempDir(), "repos")
if err != nil {
Expand Down Expand Up @@ -82,7 +83,7 @@ func runPR() {
setting.Database.Path = ":memory:"
setting.Database.Timeout = 500
*/
dbCfg := setting.Cfg.Section("database")
dbCfg := setting.CfgProvider.Section("database")
dbCfg.NewKey("DB_TYPE", "sqlite3")
dbCfg.NewKey("PATH", ":memory:")

Expand Down
2 changes: 1 addition & 1 deletion models/asymkey/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func init() {
setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
}

func TestMain(m *testing.M) {
Expand Down
2 changes: 1 addition & 1 deletion models/dbfs/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func init() {
setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
}

func TestMain(m *testing.M) {
Expand Down
2 changes: 1 addition & 1 deletion models/issues/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

func init() {
setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
}

func TestFixturesAreConsistent(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion models/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

func init() {
setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
}

// TestFixturesAreConsistent assert that test fixtures are consistent
Expand Down
6 changes: 3 additions & 3 deletions models/migrations/base/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,13 @@ func MainTest(m *testing.M) {
setting.AppDataPath = tmpDataPath

setting.SetCustomPathAndConf("", "", "")
setting.LoadForTest()
setting.InitProviderAndLoadCommonSettingsForTest()
if err = git.InitFull(context.Background()); err != nil {
fmt.Printf("Unable to InitFull: %v\n", err)
os.Exit(1)
}
setting.InitDBConfig()
setting.NewLogServices(true)
setting.LoadDBSetting()
setting.InitLogs(true)

exitStatus := m.Run()

Expand Down
2 changes: 1 addition & 1 deletion modules/context/access_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var signedUserNameStringPointerKey interface{} = "signedUserNameStringPointerKey
// AccessLogger returns a middleware to log access logger
func AccessLogger() func(http.Handler) http.Handler {
logger := log.GetLogger("access")
logTemplate, _ := template.New("log").Parse(setting.AccessLogTemplate)
logTemplate, _ := template.New("log").Parse(setting.Log.AccessLogTemplate)
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
start := time.Now()
Expand Down
10 changes: 5 additions & 5 deletions modules/doctor/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ func (w *wrappedLevelLogger) Log(skip int, level log.Level, format string, v ...
}

func initDBDisableConsole(ctx context.Context, disableConsole bool) error {
setting.LoadFromExisting()
setting.InitDBConfig()

setting.NewXORMLogService(disableConsole)
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()
setting.LoadDBSetting()
setting.InitSQLLog(disableConsole)
if err := db.InitEngine(ctx); err != nil {
return fmt.Errorf("db.InitEngine: %w", err)
}
Expand All @@ -71,7 +71,7 @@ func RunChecks(ctx context.Context, logger log.Logger, autofix bool, checks []*C
for i, check := range checks {
if !dbIsInit && !check.SkipDatabaseInitialization {
// Only open database after the most basic configuration check
setting.EnableXORMLog = false
setting.Log.EnableXORMLog = false
if err := initDBDisableConsole(ctx, true); err != nil {
logger.Error("Error whilst initializing the database: %v", err)
logger.Error("Check if you are using the right config file. You can use a --config directive to specify one.")
Expand Down
5 changes: 3 additions & 2 deletions modules/doctor/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,16 @@ func checkConfigurationFiles(ctx context.Context, logger log.Logger, autofix boo
return err
}

setting.LoadFromExisting()
setting.InitProviderFromExistingFile()
setting.LoadCommonSettings()

configurationFiles := []configurationFile{
{"Configuration File Path", setting.CustomConf, false, true, false},
{"Repository Root Path", setting.RepoRootPath, true, true, true},
{"Data Root Path", setting.AppDataPath, true, true, true},
{"Custom File Root Path", setting.CustomPath, true, false, false},
{"Work directory", setting.AppWorkPath, true, true, false},
{"Log Root Path", setting.LogRootPath, true, true, true},
{"Log Root Path", setting.Log.RootPath, true, true, true},
}

if options.IsDynamic() {
Expand Down
8 changes: 2 additions & 6 deletions modules/highlight/highlight.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,8 @@ var (
// NewContext loads custom highlight map from local config
func NewContext() {
once.Do(func() {
if setting.Cfg != nil {
keys := setting.Cfg.Section("highlight.mapping").Keys()
for i := range keys {
highlightMapping[keys[i].Name()] = keys[i].Value()
}
}
highlightMapping = setting.GetHighlightMapping()

// The size 512 is simply a conservative rule of thumb
c, err := lru.New2Q(512)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions modules/indexer/issues/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ func TestMain(m *testing.M) {

func TestBleveSearchIssues(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
setting.Cfg = ini.Empty()
setting.CfgProvider = ini.Empty()

tmpIndexerDir := t.TempDir()

setting.Cfg.Section("queue.issue_indexer").Key("DATADIR").MustString(path.Join(tmpIndexerDir, "issues.queue"))
setting.CfgProvider.Section("queue.issue_indexer").Key("DATADIR").MustString(path.Join(tmpIndexerDir, "issues.queue"))

oldIssuePath := setting.Indexer.IssuePath
setting.Indexer.IssuePath = path.Join(tmpIndexerDir, "issues.queue")
Expand All @@ -40,7 +40,7 @@ func TestBleveSearchIssues(t *testing.T) {
}()

setting.Indexer.IssueType = "bleve"
setting.NewQueueService()
setting.LoadQueueSettings()
InitIssueIndexer(true)
defer func() {
indexer := holder.get()
Expand Down
4 changes: 2 additions & 2 deletions modules/indexer/stats/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func TestMain(m *testing.M) {

func TestRepoStatsIndex(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
setting.Cfg = ini.Empty()
setting.CfgProvider = ini.Empty()

setting.NewQueueService()
setting.LoadQueueSettings()

err := Init()
assert.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion modules/markup/html_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ var localMetas = map[string]string{
}

func TestMain(m *testing.M) {
setting.LoadAllowEmpty()
setting.InitProviderAllowEmpty()
setting.LoadCommonSettings()
if err := git.InitSimple(context.Background()); err != nil {
log.Fatal("git init failed, err: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion modules/markup/markdown/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ var localMetas = map[string]string{
}

func TestMain(m *testing.M) {
setting.LoadAllowEmpty()
setting.InitProviderAllowEmpty()
setting.LoadCommonSettings()
if err := git.InitSimple(context.Background()); err != nil {
log.Fatal("git init failed, err: %v", err)
}
Expand Down
Loading

0 comments on commit c53ad05

Please sign in to comment.