From d562b419b6e13ca5e1b601f0e967b00d6826c3de Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 12 Apr 2023 22:05:10 +0800 Subject: [PATCH] Make label templates have consistent behavior and priority (#23749) (#24071) Backport #23749 Fix https://github.com/go-gitea/gitea/issues/23715 Backported manually and tested manually. --- modules/label/parser.go | 48 +++---- modules/repository/create.go | 3 +- modules/repository/init.go | 121 ++++++++++-------- modules/repository/init_test.go | 30 +++++ routers/web/org/setting.go | 2 +- routers/web/repo/issue_label.go | 2 +- routers/web/repo/issue_label_test.go | 2 + routers/web/repo/repo.go | 4 +- services/repository/repository.go | 4 +- templates/repo/create.tmpl | 4 +- .../issue/labels/label_load_template.tmpl | 4 +- 11 files changed, 135 insertions(+), 89 deletions(-) create mode 100644 modules/repository/init_test.go diff --git a/modules/label/parser.go b/modules/label/parser.go index 768c72a61b..511bac823f 100644 --- a/modules/label/parser.go +++ b/modules/label/parser.go @@ -30,31 +30,23 @@ func IsErrTemplateLoad(err error) bool { } func (err ErrTemplateLoad) Error() string { - return fmt.Sprintf("Failed to load label template file '%s': %v", err.TemplateFile, err.OriginalError) + return fmt.Sprintf("failed to load label template file %q: %v", err.TemplateFile, err.OriginalError) } -// 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") - if err == nil && len(data) > 0 { - return parseYamlFormat(name+".yaml", data) - } - - data, err = options.GetRepoInitFile("label", name+".yml") - if err == nil && len(data) > 0 { - return parseYamlFormat(name+".yml", data) - } - - data, err = options.GetRepoInitFile("label", name) +// LoadTemplateFile loads the label template file by given file name, returns a slice of Label structs. +func LoadTemplateFile(fileName string) ([]*Label, error) { + data, err := options.Labels(fileName) if err != nil { - return nil, ErrTemplateLoad{name, fmt.Errorf("GetRepoInitFile: %w", err)} + return nil, ErrTemplateLoad{fileName, fmt.Errorf("LoadTemplateFile: %w", err)} } - return parseLegacyFormat(name, data) + if strings.HasSuffix(fileName, ".yaml") || strings.HasSuffix(fileName, ".yml") { + return parseYamlFormat(fileName, data) + } + return parseLegacyFormat(fileName, data) } -func parseYamlFormat(name string, data []byte) ([]*Label, error) { +func parseYamlFormat(fileName string, data []byte) ([]*Label, error) { lf := &labelFile{} if err := yaml.Unmarshal(data, lf); err != nil { @@ -65,11 +57,11 @@ func parseYamlFormat(name string, data []byte) ([]*Label, error) { for _, l := range lf.Labels { l.Color = strings.TrimSpace(l.Color) if len(l.Name) == 0 || len(l.Color) == 0 { - return nil, ErrTemplateLoad{name, errors.New("label name and color are required fields")} + return nil, ErrTemplateLoad{fileName, errors.New("label name and color are required fields")} } color, err := NormalizeColor(l.Color) if err != nil { - return nil, ErrTemplateLoad{name, fmt.Errorf("bad HTML color code '%s' in label: %s", l.Color, l.Name)} + return nil, ErrTemplateLoad{fileName, fmt.Errorf("bad HTML color code '%s' in label: %s", l.Color, l.Name)} } l.Color = color } @@ -77,7 +69,7 @@ func parseYamlFormat(name string, data []byte) ([]*Label, error) { return lf.Labels, nil } -func parseLegacyFormat(name string, data []byte) ([]*Label, error) { +func parseLegacyFormat(fileName string, data []byte) ([]*Label, error) { lines := strings.Split(string(data), "\n") list := make([]*Label, 0, len(lines)) for i := 0; i < len(lines); i++ { @@ -88,18 +80,18 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) { parts, description, _ := strings.Cut(line, ";") - color, name, ok := strings.Cut(parts, " ") + color, labelName, ok := strings.Cut(parts, " ") if !ok { - return nil, ErrTemplateLoad{name, fmt.Errorf("line is malformed: %s", line)} + return nil, ErrTemplateLoad{fileName, fmt.Errorf("line is malformed: %s", line)} } color, err := NormalizeColor(color) if err != nil { - return nil, ErrTemplateLoad{name, fmt.Errorf("bad HTML color code '%s' in line: %s", color, line)} + return nil, ErrTemplateLoad{fileName, fmt.Errorf("bad HTML color code '%s' in line: %s", color, line)} } list = append(list, &Label{ - Name: strings.TrimSpace(name), + Name: strings.TrimSpace(labelName), Color: color, Description: strings.TrimSpace(description), }) @@ -108,10 +100,10 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) { return list, nil } -// LoadFormatted loads the labels' list of a template file as a string separated by comma -func LoadFormatted(name string) (string, error) { +// LoadTemplateDescription loads the labels from a template file, returns a description string by joining each Label.Name with comma +func LoadTemplateDescription(fileName string) (string, error) { var buf strings.Builder - list, err := GetTemplateFile(name) + list, err := LoadTemplateFile(fileName) if err != nil { return "", err } diff --git a/modules/repository/create.go b/modules/repository/create.go index 6a1fa41b6b..c1395242c5 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -23,7 +23,6 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/label" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -190,7 +189,7 @@ func CreateRepository(doer, u *user_model.User, opts CreateRepoOptions) (*repo_m // Check if label template exist if len(opts.IssueLabels) > 0 { - if _, err := label.GetTemplateFile(opts.IssueLabels); err != nil { + if _, err := LoadTemplateLabelsByDisplayName(opts.IssueLabels); err != nil { return nil, err } } diff --git a/modules/repository/init.go b/modules/repository/init.go index 49c8d2a904..c579fac620 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -8,7 +8,6 @@ import ( "context" "fmt" "os" - "path" "path/filepath" "sort" "strings" @@ -27,6 +26,11 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" ) +type OptionFile struct { + DisplayName string + Description string +} + var ( // Gitignores contains the gitiginore files Gitignores []string @@ -37,65 +41,73 @@ var ( // Readmes contains the readme files Readmes []string - // LabelTemplates contains the label template files and the list of labels for each file - LabelTemplates map[string]string + // LabelTemplateFiles contains the label template files, each item has its DisplayName and Description + LabelTemplateFiles []OptionFile + labelTemplateFileMap = map[string]string{} // DisplayName => FileName mapping ) -// LoadRepoConfig loads the repository config -func LoadRepoConfig() { - // Load .gitignore and license files and readme templates. - types := []string{"gitignore", "license", "readme", "label"} - typeFiles := make([][]string, 4) - for i, t := range types { - files, err := options.Dir(t) - if err != nil { - log.Fatal("Failed to get %s files: %v", t, err) - } - if t == "label" { - for i, f := range files { - ext := strings.ToLower(filepath.Ext(f)) - if ext == ".yaml" || ext == ".yml" { - files[i] = f[:len(f)-len(ext)] - } - } - } - customPath := path.Join(setting.CustomPath, "options", t) - isDir, err := util.IsDir(customPath) - if err != nil { - log.Fatal("Failed to get custom %s files: %v", t, err) - } - if isDir { - customFiles, err := util.StatDir(customPath) - if err != nil { - log.Fatal("Failed to get custom %s files: %v", t, err) - } +type optionFileList struct { + all []string // all files provided by bindata & custom-path. Sorted. + custom []string // custom files provided by custom-path. Non-sorted, internal use only. +} - for _, f := range customFiles { - if !util.SliceContainsString(files, f, true) { - files = append(files, f) - } +// mergeCustomLabelFiles merges the custom label files. Always use the file's main name (DisplayName) as the key to de-duplicate. +func mergeCustomLabelFiles(fl optionFileList) []string { + exts := map[string]int{"": 0, ".yml": 1, ".yaml": 2} // "yaml" file has the highest priority to be used. + + m := map[string]string{} + merge := func(list []string) { + sort.Slice(list, func(i, j int) bool { return exts[filepath.Ext(list[i])] < exts[filepath.Ext(list[j])] }) + for _, f := range list { + m[strings.TrimSuffix(f, filepath.Ext(f))] = f + } + } + merge(fl.all) + merge(fl.custom) + + files := make([]string, 0, len(m)) + for _, f := range m { + files = append(files, f) + } + sort.Strings(files) + return files +} + +// LoadRepoConfig loads the repository config +func LoadRepoConfig() error { + types := []string{"gitignore", "license", "readme", "label"} // option file directories + typeFiles := make([]optionFileList, len(types)) + for i, t := range types { + var err error + if typeFiles[i].all, err = options.Dir(t); err != nil { + return fmt.Errorf("failed to list %s files: %w", t, err) + } + sort.Strings(typeFiles[i].all) + customPath := filepath.Join(setting.CustomPath, "options", t) + if isDir, err := util.IsDir(customPath); err != nil { + return fmt.Errorf("failed to check custom %s dir: %w", t, err) + } else if isDir { + if typeFiles[i].custom, err = util.StatDir(customPath); err != nil { + return fmt.Errorf("failed to list custom %s files: %w", t, err) } } - typeFiles[i] = files } - Gitignores = typeFiles[0] - Licenses = typeFiles[1] - Readmes = typeFiles[2] - LabelTemplatesFiles := typeFiles[3] - sort.Strings(Gitignores) - sort.Strings(Licenses) - sort.Strings(Readmes) - sort.Strings(LabelTemplatesFiles) + Gitignores = typeFiles[0].all + Licenses = typeFiles[1].all + Readmes = typeFiles[2].all // Load label templates - LabelTemplates = make(map[string]string) - for _, templateFile := range LabelTemplatesFiles { - labels, err := label.LoadFormatted(templateFile) + LabelTemplateFiles = nil + labelTemplateFileMap = map[string]string{} + for _, file := range mergeCustomLabelFiles(typeFiles[3]) { + description, err := label.LoadTemplateDescription(file) if err != nil { - log.Error("Failed to load labels: %v", err) + return fmt.Errorf("failed to load labels: %w", err) } - LabelTemplates[templateFile] = labels + displayName := strings.TrimSuffix(file, filepath.Ext(file)) + labelTemplateFileMap[displayName] = file + LabelTemplateFiles = append(LabelTemplateFiles, OptionFile{DisplayName: displayName, Description: description}) } // Filter out invalid names and promote preferred licenses. @@ -111,6 +123,7 @@ func LoadRepoConfig() { } } Licenses = sortedLicenses + return nil } func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, repoPath string, opts CreateRepoOptions) error { @@ -344,7 +357,7 @@ func initRepository(ctx context.Context, repoPath string, u *user_model.User, re // InitializeLabels adds a label set to a repository using a template func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg bool) error { - list, err := label.GetTemplateFile(labelTemplate) + list, err := LoadTemplateLabelsByDisplayName(labelTemplate) if err != nil { return err } @@ -370,3 +383,11 @@ func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg } return nil } + +// LoadTemplateLabelsByDisplayName loads a label template by its display name +func LoadTemplateLabelsByDisplayName(displayName string) ([]*label.Label, error) { + if fileName, ok := labelTemplateFileMap[displayName]; ok { + return label.LoadTemplateFile(fileName) + } + return nil, label.ErrTemplateLoad{TemplateFile: displayName, OriginalError: fmt.Errorf("label template %q not found", displayName)} +} diff --git a/modules/repository/init_test.go b/modules/repository/init_test.go new file mode 100644 index 0000000000..227efdc1db --- /dev/null +++ b/modules/repository/init_test.go @@ -0,0 +1,30 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeCustomLabels(t *testing.T) { + files := mergeCustomLabelFiles(optionFileList{ + all: []string{"a", "a.yaml", "a.yml"}, + custom: nil, + }) + assert.EqualValues(t, []string{"a.yaml"}, files, "yaml file should win") + + files = mergeCustomLabelFiles(optionFileList{ + all: []string{"a", "a.yaml"}, + custom: []string{"a"}, + }) + assert.EqualValues(t, []string{"a"}, files, "custom file should win") + + files = mergeCustomLabelFiles(optionFileList{ + all: []string{"a", "a.yml", "a.yaml"}, + custom: []string{"a", "a.yml"}, + }) + assert.EqualValues(t, []string{"a.yml"}, files, "custom yml file should win if no yaml") +} diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go index 5c9b7967c3..0363c228c9 100644 --- a/routers/web/org/setting.go +++ b/routers/web/org/setting.go @@ -247,6 +247,6 @@ func Labels(ctx *context.Context) { ctx.Data["PageIsOrgSettings"] = true ctx.Data["PageIsOrgSettingsLabels"] = true ctx.Data["RequireTribute"] = true - ctx.Data["LabelTemplates"] = repo_module.LabelTemplates + ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles ctx.HTML(http.StatusOK, tplSettingsLabels) } diff --git a/routers/web/repo/issue_label.go b/routers/web/repo/issue_label.go index 31bf85fedb..a4ea53d13d 100644 --- a/routers/web/repo/issue_label.go +++ b/routers/web/repo/issue_label.go @@ -29,7 +29,7 @@ func Labels(ctx *context.Context) { ctx.Data["PageIsIssueList"] = true ctx.Data["PageIsLabels"] = true ctx.Data["RequireTribute"] = true - ctx.Data["LabelTemplates"] = repo_module.LabelTemplates + ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles ctx.HTML(http.StatusOK, tplLabels) } diff --git a/routers/web/repo/issue_label_test.go b/routers/web/repo/issue_label_test.go index a62d2afaa8..c24fe898b6 100644 --- a/routers/web/repo/issue_label_test.go +++ b/routers/web/repo/issue_label_test.go @@ -10,6 +10,7 @@ import ( issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/forms" @@ -30,6 +31,7 @@ func int64SliceToCommaSeparated(a []int64) string { func TestInitializeLabels(t *testing.T) { unittest.PrepareTestEnv(t) + assert.NoError(t, repository.LoadRepoConfig()) ctx := test.MockContext(t, "user2/repo1/labels/initialize") test.LoadUser(t, ctx, 2) test.LoadRepo(t, ctx, 2) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 9f2add1fe6..30c2500200 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -132,7 +132,7 @@ func Create(ctx *context.Context) { // Give default value for template to render. ctx.Data["Gitignores"] = repo_module.Gitignores - ctx.Data["LabelTemplates"] = repo_module.LabelTemplates + ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles ctx.Data["Licenses"] = repo_module.Licenses ctx.Data["Readmes"] = repo_module.Readmes ctx.Data["readme"] = "Default" @@ -200,7 +200,7 @@ func CreatePost(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("new_repo") ctx.Data["Gitignores"] = repo_module.Gitignores - ctx.Data["LabelTemplates"] = repo_module.LabelTemplates + ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles ctx.Data["Licenses"] = repo_module.Licenses ctx.Data["Readmes"] = repo_module.Readmes diff --git a/services/repository/repository.go b/services/repository/repository.go index 3c3e7e82c3..275b511dc7 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -81,7 +81,9 @@ func PushCreateRepo(authUser, owner *user_model.User, repoName string) (*repo_mo // Init start repository service func Init() error { - repo_module.LoadRepoConfig() + if err := repo_module.LoadRepoConfig(); err != nil { + return err + } system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repository uploads", setting.Repository.Upload.TempPath) system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repositories", repo_module.LocalCopyPath()) return initPushQueue() diff --git a/templates/repo/create.tmpl b/templates/repo/create.tmpl index 3887706c27..49817e9a0c 100644 --- a/templates/repo/create.tmpl +++ b/templates/repo/create.tmpl @@ -117,8 +117,8 @@
{{.locale.Tr "repo.issue_labels_helper"}}
diff --git a/templates/repo/issue/labels/label_load_template.tmpl b/templates/repo/issue/labels/label_load_template.tmpl index 010b691638..bd55ce712f 100644 --- a/templates/repo/issue/labels/label_load_template.tmpl +++ b/templates/repo/issue/labels/label_load_template.tmpl @@ -10,8 +10,8 @@
{{.locale.Tr "repo.issues.label_templates.helper"}}
{{svg "octicon-triangle-down" 18 "dropdown icon"}}