Refactor issue template parsing and fix API endpoint (#29069) (#29140)

Backport #29069

The old code `GetTemplatesFromDefaultBranch(...) ([]*api.IssueTemplate,
map[string]error)` doesn't really follow Golang's habits, then the
second returned value might be misused. For example, the API function
`GetIssueTemplates` incorrectly checked the second returned value and
always responds 500 error.

This PR refactors GetTemplatesFromDefaultBranch to
ParseTemplatesFromDefaultBranch and clarifies its behavior, and fixes
the API endpoint bug, and adds some tests.

And by the way, add proper prefix `X-` for the header generated in
`checkDeprecatedAuthMethods`, because non-standard HTTP headers should
have `X-` prefix, and it is also consistent with the new code in
`GetIssueTemplates`
This commit is contained in:
wxiaoguang 2024-02-14 09:32:31 +08:00 committed by GitHub
parent 0ac3186267
commit dd8bc1d61d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 88 additions and 33 deletions

View File

@ -810,7 +810,7 @@ func individualPermsChecker(ctx *context.APIContext) {
// check for and warn against deprecated authentication options
func checkDeprecatedAuthMethods(ctx *context.APIContext) {
if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
ctx.Resp.Header().Set("X-Gitea-Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
}
}

View File

@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"slices"
"strconv"
"strings"
"time"
@ -1159,12 +1160,11 @@ func GetIssueTemplates(ctx *context.APIContext) {
// "$ref": "#/responses/IssueTemplates"
// "404":
// "$ref": "#/responses/notFound"
ret, err := issue.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetTemplatesFromDefaultBranch", err)
return
ret := issue.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
if cnt := len(ret.TemplateErrors); cnt != 0 {
ctx.Resp.Header().Add("X-Gitea-Warning", "error occurs when parsing issue template: count="+strconv.Itoa(cnt))
}
ctx.JSON(http.StatusOK, ret)
ctx.JSON(http.StatusOK, ret.IssueTemplates)
}
// GetIssueConfig returns the issue config for a repo

View File

@ -963,19 +963,17 @@ func NewIssue(ctx *context.Context) {
}
ctx.Data["Tags"] = tags
_, templateErrs := issue_service.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
templateLoaded, errs := setTemplateIfExists(ctx, issueTemplateKey, IssueTemplateCandidates)
if len(errs) > 0 {
for k, v := range errs {
templateErrs[k] = v
}
for k, v := range errs {
ret.TemplateErrors[k] = v
}
if ctx.Written() {
return
}
if len(templateErrs) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, templateErrs), true)
if len(ret.TemplateErrors) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, ret.TemplateErrors), true)
}
ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.CanWrite(unit.TypeIssues)
@ -1018,11 +1016,11 @@ func NewIssueChooseTemplate(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.issues.new")
ctx.Data["PageIsIssueList"] = true
issueTemplates, errs := issue_service.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["IssueTemplates"] = issueTemplates
ret := issue_service.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["IssueTemplates"] = ret.IssueTemplates
if len(errs) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, errs), true)
if len(ret.TemplateErrors) > 0 {
ctx.Flash.Warning(renderErrorOfTemplates(ctx, ret.TemplateErrors), true)
}
if !issue_service.HasTemplatesOrContactLinks(ctx.Repo.Repository, ctx.Repo.GitRepo) {

View File

@ -294,8 +294,8 @@ func MilestoneIssuesAndPulls(ctx *context.Context) {
issues(ctx, milestoneID, projectID, util.OptionalBoolNone)
ret, _ := issue.GetTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["NewIssueChooseTemplate"] = len(ret) > 0
ret := issue.ParseTemplatesFromDefaultBranch(ctx.Repo.Repository, ctx.Repo.GitRepo)
ctx.Data["NewIssueChooseTemplate"] = len(ret.IssueTemplates) > 0
ctx.Data["CanWriteIssues"] = ctx.Repo.CanWriteIssuesOrPulls(false)
ctx.Data["CanWritePulls"] = ctx.Repo.CanWriteIssuesOrPulls(true)

View File

@ -109,21 +109,23 @@ func IsTemplateConfig(path string) bool {
return false
}
// GetTemplatesFromDefaultBranch checks for issue templates in the repo's default branch,
// returns valid templates and the errors of invalid template files.
func GetTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repository) ([]*api.IssueTemplate, map[string]error) {
var issueTemplates []*api.IssueTemplate
// ParseTemplatesFromDefaultBranch parses the issue templates in the repo's default branch,
// returns valid templates and the errors of invalid template files (the errors map is guaranteed to be non-nil).
func ParseTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repository) (ret struct {
IssueTemplates []*api.IssueTemplate
TemplateErrors map[string]error
},
) {
ret.TemplateErrors = map[string]error{}
if repo.IsEmpty {
return issueTemplates, nil
return ret
}
commit, err := gitRepo.GetBranchCommit(repo.DefaultBranch)
if err != nil {
return issueTemplates, nil
return ret
}
invalidFiles := map[string]error{}
for _, dirName := range templateDirCandidates {
tree, err := commit.SubTree(dirName)
if err != nil {
@ -133,7 +135,7 @@ func GetTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repositor
entries, err := tree.ListEntries()
if err != nil {
log.Debug("list entries in %s: %v", dirName, err)
return issueTemplates, nil
return ret
}
for _, entry := range entries {
if !template.CouldBe(entry.Name()) {
@ -141,16 +143,16 @@ func GetTemplatesFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repositor
}
fullName := path.Join(dirName, entry.Name())
if it, err := template.UnmarshalFromEntry(entry, dirName); err != nil {
invalidFiles[fullName] = err
ret.TemplateErrors[fullName] = err
} else {
if !strings.HasPrefix(it.Ref, "refs/") { // Assume that the ref intended is always a branch - for tags users should use refs/tags/<ref>
it.Ref = git.BranchPrefix + it.Ref
}
issueTemplates = append(issueTemplates, it)
ret.IssueTemplates = append(ret.IssueTemplates, it)
}
}
}
return issueTemplates, invalidFiles
return ret
}
// GetTemplateConfigFromDefaultBranch returns the issue config for this repo.
@ -179,8 +181,8 @@ func GetTemplateConfigFromDefaultBranch(repo *repo.Repository, gitRepo *git.Repo
}
func HasTemplatesOrContactLinks(repo *repo.Repository, gitRepo *git.Repository) bool {
ret, _ := GetTemplatesFromDefaultBranch(repo, gitRepo)
if len(ret) > 0 {
ret := ParseTemplatesFromDefaultBranch(repo, gitRepo)
if len(ret.IssueTemplates) > 0 {
return true
}

View File

@ -0,0 +1,55 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
"net/http"
"net/url"
"testing"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
"github.com/stretchr/testify/assert"
)
func TestAPIIssueTemplateList(t *testing.T) {
onGiteaRun(t, func(*testing.T, *url.URL) {
var issueTemplates []*api.IssueTemplate
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
// no issue template
req := NewRequest(t, "GET", "/api/v1/repos/user2/repo1/issue_templates")
resp := MakeRequest(t, req, http.StatusOK)
issueTemplates = nil
DecodeJSON(t, resp, &issueTemplates)
assert.Empty(t, issueTemplates)
// one correct issue template and some incorrect issue templates
err := createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-ok.md", repo.DefaultBranch, `----
name: foo
about: bar
----
`)
assert.NoError(t, err)
err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-err1.yml", repo.DefaultBranch, `name: '`)
assert.NoError(t, err)
err = createOrReplaceFileInBranch(user, repo, ".gitea/ISSUE_TEMPLATE/tmpl-err2.yml", repo.DefaultBranch, `other: `)
assert.NoError(t, err)
req = NewRequest(t, "GET", "/api/v1/repos/user2/repo1/issue_templates")
resp = MakeRequest(t, req, http.StatusOK)
issueTemplates = nil
DecodeJSON(t, resp, &issueTemplates)
assert.Len(t, issueTemplates, 1)
assert.Equal(t, "foo", issueTemplates[0].Name)
assert.Equal(t, "error occurs when parsing issue template: count=2", resp.Header().Get("X-Gitea-Warning"))
})
}