From 090e75392385041b3abb30d02564962a3ff687f6 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 8 Mar 2023 17:31:27 +0800 Subject: [PATCH] Reduce duplicate and useless code in options (#23369) Avoid maintaining two copies of code, some functions can be used with both `bindata` and `no bindata`. And removed `GetRepoInitFile`, it's useless now. `Readme`/`Gitignore`/`License`/`Labels` will clean the name and use custom files when available. --- modules/label/parser.go | 6 ++-- modules/options/base.go | 56 ++++++++++++++++++++++++++++++ modules/options/dynamic.go | 70 ++++---------------------------------- modules/options/repo.go | 44 ------------------------ modules/options/static.go | 50 ++++----------------------- modules/repository/init.go | 6 ++-- 6 files changed, 74 insertions(+), 158 deletions(-) delete mode 100644 modules/options/repo.go diff --git a/modules/label/parser.go b/modules/label/parser.go index 768c72a61..55bf570de 100644 --- a/modules/label/parser.go +++ b/modules/label/parser.go @@ -36,17 +36,17 @@ func (err ErrTemplateLoad) Error() string { // GetTemplateFile loads the label template file by given name, // then parses and returns a list of name-color pairs and optionally description. func GetTemplateFile(name string) ([]*Label, error) { - data, err := options.GetRepoInitFile("label", name+".yaml") + data, err := options.Labels(name + ".yaml") if err == nil && len(data) > 0 { return parseYamlFormat(name+".yaml", data) } - data, err = options.GetRepoInitFile("label", name+".yml") + data, err = options.Labels(name + ".yml") if err == nil && len(data) > 0 { return parseYamlFormat(name+".yml", data) } - data, err = options.GetRepoInitFile("label", name) + data, err = options.Labels(name) if err != nil { return nil, ErrTemplateLoad{name, fmt.Errorf("GetRepoInitFile: %w", err)} } diff --git a/modules/options/base.go b/modules/options/base.go index 039e934b3..3c140f643 100644 --- a/modules/options/base.go +++ b/modules/options/base.go @@ -7,11 +7,52 @@ import ( "fmt" "io/fs" "os" + "path" "path/filepath" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" ) +// Locale reads the content of a specific locale from static/bindata or custom path. +func Locale(name string) ([]byte, error) { + return fileFromDir(path.Join("locale", path.Clean("/"+name))) +} + +// Readme reads the content of a specific readme from static/bindata or custom path. +func Readme(name string) ([]byte, error) { + return fileFromDir(path.Join("readme", path.Clean("/"+name))) +} + +// Gitignore reads the content of a gitignore locale from static/bindata or custom path. +func Gitignore(name string) ([]byte, error) { + return fileFromDir(path.Join("gitignore", path.Clean("/"+name))) +} + +// License reads the content of a specific license from static/bindata or custom path. +func License(name string) ([]byte, error) { + return fileFromDir(path.Join("license", path.Clean("/"+name))) +} + +// Labels reads the content of a specific labels from static/bindata or custom path. +func Labels(name string) ([]byte, error) { + return fileFromDir(path.Join("label", path.Clean("/"+name))) +} + +// WalkLocales reads the content of a specific locale +func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error { + if IsDynamic() { + if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to walk locales. Error: %w", err) + } + } + + if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to walk locales. Error: %w", err) + } + return nil +} + func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, err error) error) error { if err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { // name is the path relative to the root @@ -37,3 +78,18 @@ func walkAssetDir(root string, callback func(path, name string, d fs.DirEntry, e } return nil } + +func statDirIfExist(dir string) ([]string, error) { + isDir, err := util.IsDir(dir) + if err != nil { + return nil, fmt.Errorf("unable to check if static directory %s is a directory. %w", dir, err) + } + if !isDir { + return nil, nil + } + files, err := util.StatDir(dir, true) + if err != nil { + return nil, fmt.Errorf("unable to read directory %q. %w", dir, err) + } + return files, nil +} diff --git a/modules/options/dynamic.go b/modules/options/dynamic.go index f9b3714b8..8c954492a 100644 --- a/modules/options/dynamic.go +++ b/modules/options/dynamic.go @@ -7,10 +7,8 @@ package options import ( "fmt" - "io/fs" "os" "path" - "path/filepath" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -27,76 +25,20 @@ func Dir(name string) ([]string, error) { var result []string - customDir := path.Join(setting.CustomPath, "options", name) - - isDir, err := util.IsDir(customDir) - if err != nil { - return []string{}, fmt.Errorf("Unabe to check if custom directory %s is a directory. %w", customDir, err) - } - if isDir { - files, err := util.StatDir(customDir, true) + for _, dir := range []string{ + path.Join(setting.CustomPath, "options", name), // custom dir + path.Join(setting.StaticRootPath, "options", name), // static dir + } { + files, err := statDirIfExist(dir) if err != nil { - return []string{}, fmt.Errorf("Failed to read custom directory. %w", err) + return nil, err } - - result = append(result, files...) - } - - staticDir := path.Join(setting.StaticRootPath, "options", name) - - isDir, err = util.IsDir(staticDir) - if err != nil { - return []string{}, fmt.Errorf("unable to check if static directory %s is a directory. %w", staticDir, err) - } - if isDir { - files, err := util.StatDir(staticDir, true) - if err != nil { - return []string{}, fmt.Errorf("Failed to read static directory. %w", err) - } - result = append(result, files...) } return directories.AddAndGet(name, result), nil } -// Locale reads the content of a specific locale from static or custom path. -func Locale(name string) ([]byte, error) { - return fileFromDir(path.Join("locale", name)) -} - -// WalkLocales reads the content of a specific locale from static or custom path. -func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error { - if err := walkAssetDir(filepath.Join(setting.StaticRootPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to walk locales. Error: %w", err) - } - - if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to walk locales. Error: %w", err) - } - return nil -} - -// Readme reads the content of a specific readme from static or custom path. -func Readme(name string) ([]byte, error) { - return fileFromDir(path.Join("readme", path.Clean("/"+name))) -} - -// Gitignore reads the content of a specific gitignore from static or custom path. -func Gitignore(name string) ([]byte, error) { - return fileFromDir(path.Join("gitignore", path.Clean("/"+name))) -} - -// License reads the content of a specific license from static or custom path. -func License(name string) ([]byte, error) { - return fileFromDir(path.Join("license", path.Clean("/"+name))) -} - -// Labels reads the content of a specific labels from static or custom path. -func Labels(name string) ([]byte, error) { - return fileFromDir(path.Join("label", path.Clean("/"+name))) -} - // fileFromDir is a helper to read files from static or custom path. func fileFromDir(name string) ([]byte, error) { customPath := path.Join(setting.CustomPath, "options", name) diff --git a/modules/options/repo.go b/modules/options/repo.go deleted file mode 100644 index 1480f7808..000000000 --- a/modules/options/repo.go +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package options - -import ( - "fmt" - "os" - "path" - "strings" - - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/util" -) - -// GetRepoInitFile returns repository init files -func GetRepoInitFile(tp, name string) ([]byte, error) { - cleanedName := strings.TrimLeft(path.Clean("/"+name), "/") - relPath := path.Join("options", tp, cleanedName) - - // Use custom file when available. - customPath := path.Join(setting.CustomPath, relPath) - isFile, err := util.IsFile(customPath) - if err != nil { - log.Error("Unable to check if %s is a file. Error: %v", customPath, err) - } - if isFile { - return os.ReadFile(customPath) - } - - switch tp { - case "readme": - return Readme(cleanedName) - case "gitignore": - return Gitignore(cleanedName) - case "license": - return License(cleanedName) - case "label": - return Labels(cleanedName) - default: - return []byte{}, fmt.Errorf("Invalid init file type") - } -} diff --git a/modules/options/static.go b/modules/options/static.go index 2405d658b..549f4e25b 100644 --- a/modules/options/static.go +++ b/modules/options/static.go @@ -8,10 +8,8 @@ package options import ( "fmt" "io" - "io/fs" "os" "path" - "path/filepath" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -28,17 +26,14 @@ func Dir(name string) ([]string, error) { var result []string - customDir := path.Join(setting.CustomPath, "options", name) - isDir, err := util.IsDir(customDir) - if err != nil { - return []string{}, fmt.Errorf("unable to check if custom directory %q is a directory. %w", customDir, err) - } - if isDir { - files, err := util.StatDir(customDir, true) + for _, dir := range []string{ + path.Join(setting.CustomPath, "options", name), // custom dir + // no static dir + } { + files, err := statDirIfExist(dir) if err != nil { - return []string{}, fmt.Errorf("unable to read custom directory %q. %w", customDir, err) + return nil, err } - result = append(result, files...) } @@ -69,39 +64,6 @@ func AssetDir(dirName string) ([]string, error) { return results, nil } -// Locale reads the content of a specific locale from bindata or custom path. -func Locale(name string) ([]byte, error) { - return fileFromDir(path.Join("locale", name)) -} - -// WalkLocales reads the content of a specific locale from static or custom path. -func WalkLocales(callback func(path, name string, d fs.DirEntry, err error) error) error { - if err := walkAssetDir(filepath.Join(setting.CustomPath, "options", "locale"), callback); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("failed to walk locales. Error: %w", err) - } - return nil -} - -// Readme reads the content of a specific readme from bindata or custom path. -func Readme(name string) ([]byte, error) { - return fileFromDir(path.Join("readme", path.Clean("/"+name))) -} - -// Gitignore reads the content of a gitignore locale from bindata or custom path. -func Gitignore(name string) ([]byte, error) { - return fileFromDir(path.Join("gitignore", path.Clean("/"+name))) -} - -// License reads the content of a specific license from bindata or custom path. -func License(name string) ([]byte, error) { - return fileFromDir(path.Join("license", path.Clean("/"+name))) -} - -// Labels reads the content of a specific labels from static or custom path. -func Labels(name string) ([]byte, error) { - return fileFromDir(path.Join("label", path.Clean("/"+name))) -} - // fileFromDir is a helper to read files from bindata or custom path. func fileFromDir(name string) ([]byte, error) { customPath := path.Join(setting.CustomPath, "options", name) diff --git a/modules/repository/init.go b/modules/repository/init.go index 49c8d2a90..f9a33cd4f 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -136,7 +136,7 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, } // README - data, err := options.GetRepoInitFile("readme", opts.Readme) + data, err := options.Readme(opts.Readme) if err != nil { return fmt.Errorf("GetRepoInitFile[%s]: %w", opts.Readme, err) } @@ -164,7 +164,7 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, var buf bytes.Buffer names := strings.Split(opts.Gitignores, ",") for _, name := range names { - data, err = options.GetRepoInitFile("gitignore", name) + data, err = options.Gitignore(name) if err != nil { return fmt.Errorf("GetRepoInitFile[%s]: %w", name, err) } @@ -182,7 +182,7 @@ func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, // LICENSE if len(opts.License) > 0 { - data, err = options.GetRepoInitFile("license", opts.License) + data, err = options.License(opts.License) if err != nil { return fmt.Errorf("GetRepoInitFile[%s]: %w", opts.License, err) }