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.
This commit is contained in:
wxiaoguang 2023-04-12 22:05:10 +08:00 committed by GitHub
parent 5482602ba8
commit d562b419b6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 135 additions and 89 deletions

View file

@ -30,31 +30,23 @@ func IsErrTemplateLoad(err error) bool {
} }
func (err ErrTemplateLoad) Error() string { 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, // LoadTemplateFile loads the label template file by given file name, returns a slice of Label structs.
// then parses and returns a list of name-color pairs and optionally description. func LoadTemplateFile(fileName string) ([]*Label, error) {
func GetTemplateFile(name string) ([]*Label, error) { data, err := options.Labels(fileName)
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)
if err != nil { 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{} lf := &labelFile{}
if err := yaml.Unmarshal(data, lf); err != nil { 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 { for _, l := range lf.Labels {
l.Color = strings.TrimSpace(l.Color) l.Color = strings.TrimSpace(l.Color)
if len(l.Name) == 0 || len(l.Color) == 0 { 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) color, err := NormalizeColor(l.Color)
if err != nil { 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 l.Color = color
} }
@ -77,7 +69,7 @@ func parseYamlFormat(name string, data []byte) ([]*Label, error) {
return lf.Labels, nil 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") lines := strings.Split(string(data), "\n")
list := make([]*Label, 0, len(lines)) list := make([]*Label, 0, len(lines))
for i := 0; i < len(lines); i++ { for i := 0; i < len(lines); i++ {
@ -88,18 +80,18 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) {
parts, description, _ := strings.Cut(line, ";") parts, description, _ := strings.Cut(line, ";")
color, name, ok := strings.Cut(parts, " ") color, labelName, ok := strings.Cut(parts, " ")
if !ok { 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) color, err := NormalizeColor(color)
if err != nil { 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{ list = append(list, &Label{
Name: strings.TrimSpace(name), Name: strings.TrimSpace(labelName),
Color: color, Color: color,
Description: strings.TrimSpace(description), Description: strings.TrimSpace(description),
}) })
@ -108,10 +100,10 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) {
return list, nil return list, nil
} }
// LoadFormatted loads the labels' list of a template file as a string separated by comma // LoadTemplateDescription loads the labels from a template file, returns a description string by joining each Label.Name with comma
func LoadFormatted(name string) (string, error) { func LoadTemplateDescription(fileName string) (string, error) {
var buf strings.Builder var buf strings.Builder
list, err := GetTemplateFile(name) list, err := LoadTemplateFile(fileName)
if err != nil { if err != nil {
return "", err return "", err
} }

View file

@ -23,7 +23,6 @@ import (
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/label"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" 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 // Check if label template exist
if len(opts.IssueLabels) > 0 { if len(opts.IssueLabels) > 0 {
if _, err := label.GetTemplateFile(opts.IssueLabels); err != nil { if _, err := LoadTemplateLabelsByDisplayName(opts.IssueLabels); err != nil {
return nil, err return nil, err
} }
} }

View file

@ -8,7 +8,6 @@ import (
"context" "context"
"fmt" "fmt"
"os" "os"
"path"
"path/filepath" "path/filepath"
"sort" "sort"
"strings" "strings"
@ -27,6 +26,11 @@ import (
asymkey_service "code.gitea.io/gitea/services/asymkey" asymkey_service "code.gitea.io/gitea/services/asymkey"
) )
type OptionFile struct {
DisplayName string
Description string
}
var ( var (
// Gitignores contains the gitiginore files // Gitignores contains the gitiginore files
Gitignores []string Gitignores []string
@ -37,65 +41,73 @@ var (
// Readmes contains the readme files // Readmes contains the readme files
Readmes []string Readmes []string
// LabelTemplates contains the label template files and the list of labels for each file // LabelTemplateFiles contains the label template files, each item has its DisplayName and Description
LabelTemplates map[string]string LabelTemplateFiles []OptionFile
labelTemplateFileMap = map[string]string{} // DisplayName => FileName mapping
) )
// LoadRepoConfig loads the repository config type optionFileList struct {
func LoadRepoConfig() { all []string // all files provided by bindata & custom-path. Sorted.
// Load .gitignore and license files and readme templates. custom []string // custom files provided by custom-path. Non-sorted, internal use only.
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)
}
for _, f := range customFiles { // mergeCustomLabelFiles merges the custom label files. Always use the file's main name (DisplayName) as the key to de-duplicate.
if !util.SliceContainsString(files, f, true) { 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) 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] Gitignores = typeFiles[0].all
Licenses = typeFiles[1] Licenses = typeFiles[1].all
Readmes = typeFiles[2] Readmes = typeFiles[2].all
LabelTemplatesFiles := typeFiles[3]
sort.Strings(Gitignores)
sort.Strings(Licenses)
sort.Strings(Readmes)
sort.Strings(LabelTemplatesFiles)
// Load label templates // Load label templates
LabelTemplates = make(map[string]string) LabelTemplateFiles = nil
for _, templateFile := range LabelTemplatesFiles { labelTemplateFileMap = map[string]string{}
labels, err := label.LoadFormatted(templateFile) for _, file := range mergeCustomLabelFiles(typeFiles[3]) {
description, err := label.LoadTemplateDescription(file)
if err != nil { 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. // Filter out invalid names and promote preferred licenses.
@ -111,6 +123,7 @@ func LoadRepoConfig() {
} }
} }
Licenses = sortedLicenses Licenses = sortedLicenses
return nil
} }
func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, repoPath string, opts CreateRepoOptions) error { 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 // InitializeLabels adds a label set to a repository using a template
func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg bool) error { func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg bool) error {
list, err := label.GetTemplateFile(labelTemplate) list, err := LoadTemplateLabelsByDisplayName(labelTemplate)
if err != nil { if err != nil {
return err return err
} }
@ -370,3 +383,11 @@ func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg
} }
return nil 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)}
}

View file

@ -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")
}

View file

@ -247,6 +247,6 @@ func Labels(ctx *context.Context) {
ctx.Data["PageIsOrgSettings"] = true ctx.Data["PageIsOrgSettings"] = true
ctx.Data["PageIsOrgSettingsLabels"] = true ctx.Data["PageIsOrgSettingsLabels"] = true
ctx.Data["RequireTribute"] = true ctx.Data["RequireTribute"] = true
ctx.Data["LabelTemplates"] = repo_module.LabelTemplates ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.HTML(http.StatusOK, tplSettingsLabels) ctx.HTML(http.StatusOK, tplSettingsLabels)
} }

View file

@ -29,7 +29,7 @@ func Labels(ctx *context.Context) {
ctx.Data["PageIsIssueList"] = true ctx.Data["PageIsIssueList"] = true
ctx.Data["PageIsLabels"] = true ctx.Data["PageIsLabels"] = true
ctx.Data["RequireTribute"] = true ctx.Data["RequireTribute"] = true
ctx.Data["LabelTemplates"] = repo_module.LabelTemplates ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.HTML(http.StatusOK, tplLabels) ctx.HTML(http.StatusOK, tplLabels)
} }

View file

@ -10,6 +10,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues" issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms" "code.gitea.io/gitea/services/forms"
@ -30,6 +31,7 @@ func int64SliceToCommaSeparated(a []int64) string {
func TestInitializeLabels(t *testing.T) { func TestInitializeLabels(t *testing.T) {
unittest.PrepareTestEnv(t) unittest.PrepareTestEnv(t)
assert.NoError(t, repository.LoadRepoConfig())
ctx := test.MockContext(t, "user2/repo1/labels/initialize") ctx := test.MockContext(t, "user2/repo1/labels/initialize")
test.LoadUser(t, ctx, 2) test.LoadUser(t, ctx, 2)
test.LoadRepo(t, ctx, 2) test.LoadRepo(t, ctx, 2)

View file

@ -132,7 +132,7 @@ func Create(ctx *context.Context) {
// Give default value for template to render. // Give default value for template to render.
ctx.Data["Gitignores"] = repo_module.Gitignores 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["Licenses"] = repo_module.Licenses
ctx.Data["Readmes"] = repo_module.Readmes ctx.Data["Readmes"] = repo_module.Readmes
ctx.Data["readme"] = "Default" ctx.Data["readme"] = "Default"
@ -200,7 +200,7 @@ func CreatePost(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("new_repo") ctx.Data["Title"] = ctx.Tr("new_repo")
ctx.Data["Gitignores"] = repo_module.Gitignores 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["Licenses"] = repo_module.Licenses
ctx.Data["Readmes"] = repo_module.Readmes ctx.Data["Readmes"] = repo_module.Readmes

View file

@ -81,7 +81,9 @@ func PushCreateRepo(authUser, owner *user_model.User, repoName string) (*repo_mo
// Init start repository service // Init start repository service
func Init() error { 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 repository uploads", setting.Repository.Upload.TempPath)
system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repositories", repo_module.LocalCopyPath()) system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repositories", repo_module.LocalCopyPath())
return initPushQueue() return initPushQueue()

View file

@ -117,8 +117,8 @@
<div class="default text">{{.locale.Tr "repo.issue_labels_helper"}}</div> <div class="default text">{{.locale.Tr "repo.issue_labels_helper"}}</div>
<div class="menu"> <div class="menu">
<div class="item" data-value="">{{.locale.Tr "repo.issue_labels_helper"}}</div> <div class="item" data-value="">{{.locale.Tr "repo.issue_labels_helper"}}</div>
{{range $template, $labels := .LabelTemplates}} {{range .LabelTemplateFiles}}
<div class="item" data-value="{{$template}}">{{$template}}<br/><i>({{$labels}})</i></div> <div class="item" data-value="{{.DisplayName}}">{{.DisplayName}}<br><i>({{.Description}})</i></div>
{{end}} {{end}}
</div> </div>
</div> </div>

View file

@ -10,8 +10,8 @@
<input type="hidden" name="template_name" value="Default"> <input type="hidden" name="template_name" value="Default">
<div class="default text">{{.locale.Tr "repo.issues.label_templates.helper"}}</div> <div class="default text">{{.locale.Tr "repo.issues.label_templates.helper"}}</div>
<div class="menu"> <div class="menu">
{{range $template, $labels := .LabelTemplates}} {{range .LabelTemplateFiles}}
<div class="item" data-value="{{$template}}">{{$template}}<br/><i>({{$labels}})</i></div> <div class="item" data-value="{{.DisplayName}}">{{.DisplayName}}<br><i>({{.Description}})</i></div>
{{end}} {{end}}
</div> </div>
{{svg "octicon-triangle-down" 18 "dropdown icon"}} {{svg "octicon-triangle-down" 18 "dropdown icon"}}