From dbe5aef1b1e7ab0bf75a167f9ec7c6a3b247a379 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 6 Sep 2019 00:42:23 -0300 Subject: [PATCH 1/4] Add settings for CloseKeywords and ReopenKeywords --- custom/conf/app.ini.sample | 4 ++++ docs/content/doc/advanced/config-cheat-sheet.en-us.md | 4 ++++ models/action.go | 11 ++++------- modules/setting/repository.go | 8 ++++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index a2ac7248e770b..e1b62d42c29c3 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -69,6 +69,10 @@ MAX_FILES = 5 [repository.pull-request] ; List of prefixes used in Pull Request title to mark them as Work In Progress WORK_IN_PROGRESS_PREFIXES=WIP:,[WIP] +; List of keywords used in Pull Request comments to automatically close a related issue +CLOSE_KEYWORDS=close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved +; List of keywords used in Pull Request comments to automatically reopen a related issue +REOPEN_KEYWORDS=reopen,reopens,reopened [repository.issue] ; List of reasons why a Pull Request or Issue can be locked diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 10c6110f373f2..b7e01c6458c48 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -71,6 +71,10 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `WORK_IN_PROGRESS_PREFIXES`: **WIP:,\[WIP\]**: List of prefixes used in Pull Request title to mark them as Work In Progress +- `CLOSE_KEYWORDS`: **close**, **closes**, **closed**, **fix**, **fixes**, **fixed**, **resolve**, **resolves**, **resolved**: List of + keywords used in Pull Request comments to automatically close a related issue +- `REOPEN_KEYWORDS`: **reopen**, **reopens**, **reopened**: List of keywords used in Pull Request comments to automatically reopen + a related issue ### Repository - Issue (`repository.issue`) diff --git a/models/action.go b/models/action.go index 87088101f97fc..02911423081d6 100644 --- a/models/action.go +++ b/models/action.go @@ -55,11 +55,6 @@ const ( ) var ( - // Same as GitHub. See - // https://help.github.com/articles/closing-issues-via-commit-messages - issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} - issueReopenKeywords = []string{"reopen", "reopens", "reopened"} - issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp issueReferenceKeywordsPat *regexp.Regexp ) @@ -72,8 +67,10 @@ func assembleKeywordsPattern(words []string) string { } func init() { - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)) + fmt.Printf("GAP: CloseKeywords: %v\n", setting.Repository.PullRequest.CloseKeywords) + fmt.Printf("GAP: ReopenKeywords: %v\n", setting.Repository.PullRequest.ReopenKeywords) + issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(setting.Repository.PullRequest.CloseKeywords)) + issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(setting.Repository.PullRequest.ReopenKeywords)) issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStrNoKeyword) } diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 728741576dca1..e80a58e49fa3b 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -59,6 +59,8 @@ var ( // Pull request settings PullRequest struct { WorkInProgressPrefixes []string + CloseKeywords []string + ReopenKeywords []string } `ini:"repository.pull-request"` // Issue Setting @@ -112,8 +114,14 @@ var ( // Pull request settings PullRequest: struct { WorkInProgressPrefixes []string + CloseKeywords []string + ReopenKeywords []string }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, + // Same as GitHub. See + // https://help.github.com/articles/closing-issues-via-commit-messages + CloseKeywords: strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","), + ReopenKeywords: strings.Split("reopen,reopens,reopened", ","), }, // Issue settings From fd156ec8f9b2606ee161ecb05a6a75c74602208f Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 6 Sep 2019 17:27:09 -0300 Subject: [PATCH 2/4] Fix and improve tests --- models/action.go | 125 +++++++++++++++++++++++++----------------- models/action_test.go | 26 +++++++++ models/models.go | 5 ++ routers/init.go | 1 + 4 files changed, 108 insertions(+), 49 deletions(-) diff --git a/models/action.go b/models/action.go index 02911423081d6..7da8fc44d869c 100644 --- a/models/action.go +++ b/models/action.go @@ -62,18 +62,16 @@ var ( const issueRefRegexpStr = `(?:([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+))?(#[0-9]+)+` const issueRefRegexpStrNoKeyword = `(?:\s|^|\(|\[)(?:([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+))?(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))` -func assembleKeywordsPattern(words []string) string { - return fmt.Sprintf(`(?i)(?:%s)(?::?) %s`, strings.Join(words, "|"), issueRefRegexpStr) -} - func init() { - fmt.Printf("GAP: CloseKeywords: %v\n", setting.Repository.PullRequest.CloseKeywords) - fmt.Printf("GAP: ReopenKeywords: %v\n", setting.Repository.PullRequest.ReopenKeywords) - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(setting.Repository.PullRequest.CloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(setting.Repository.PullRequest.ReopenKeywords)) issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStrNoKeyword) } +// ActionPostConfigInit performs initialization that requires app.ini settings to work +func ActionPostConfigInit() { + issueCloseKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.CloseKeywords) + issueReopenKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.ReopenKeywords) +} + // Action represents user operation type and other information to // repository. It implemented interface base.Actioner so that can be // used in template render. @@ -603,61 +601,65 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra continue } refMarked = make(map[int64]bool) - for _, m := range issueCloseKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { - continue - } - ref := m[3] - - // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) - if err != nil { + if issueCloseKeywordsPat != nil { + for _, m := range issueCloseKeywordsPat.FindAllStringSubmatch(c.Message, -1) { + if len(m[3]) == 0 { continue } - } else { - refRepo = repo - } + ref := m[3] + + // issue is from another repo + if len(m[1]) > 0 && len(m[2]) > 0 { + refRepo, err = GetRepositoryFromMatch(m[1], m[2]) + if err != nil { + continue + } + } else { + refRepo = repo + } - perm, err := GetUserRepoPermission(refRepo, doer) - if err != nil { - return err - } - // only close issues in another repo if user has push access - if perm.CanWrite(UnitTypeCode) { - if err := changeIssueStatus(refRepo, doer, ref, refMarked, true); err != nil { + perm, err := GetUserRepoPermission(refRepo, doer) + if err != nil { return err } + // only close issues in another repo if user has push access + if perm.CanWrite(UnitTypeCode) { + if err := changeIssueStatus(refRepo, doer, ref, refMarked, true); err != nil { + return err + } + } } } // It is conflict to have close and reopen at same time, so refsMarked doesn't need to reinit here. - for _, m := range issueReopenKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { - continue - } - ref := m[3] - - // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) - if err != nil { + if issueReopenKeywordsPat != nil { + for _, m := range issueReopenKeywordsPat.FindAllStringSubmatch(c.Message, -1) { + if len(m[3]) == 0 { continue } - } else { - refRepo = repo - } - - perm, err := GetUserRepoPermission(refRepo, doer) - if err != nil { - return err - } + ref := m[3] + + // issue is from another repo + if len(m[1]) > 0 && len(m[2]) > 0 { + refRepo, err = GetRepositoryFromMatch(m[1], m[2]) + if err != nil { + continue + } + } else { + refRepo = repo + } - // only reopen issues in another repo if user has push access - if perm.CanWrite(UnitTypeCode) { - if err := changeIssueStatus(refRepo, doer, ref, refMarked, false); err != nil { + perm, err := GetUserRepoPermission(refRepo, doer) + if err != nil { return err } + + // only reopen issues in another repo if user has push access + if perm.CanWrite(UnitTypeCode) { + if err := changeIssueStatus(refRepo, doer, ref, refMarked, false); err != nil { + return err + } + } } } } @@ -834,3 +836,28 @@ func GetFeeds(opts GetFeedsOptions) ([]*Action, error) { return actions, nil } + +func parseKeywords(words []string) []string { + acceptedWords := make([]string, 0, 5) + wordPat := regexp.MustCompile(`^[\pL]+$`) + for _, word := range words { + word = strings.ToLower(strings.TrimSpace(word)) + // Accept Unicode letter class runes (a-z, á, à, ä, ) + if wordPat.MatchString(word) { + acceptedWords = append(acceptedWords, word) + } else { + log.Info("Invalid keyword: %s", word) + } + } + return acceptedWords +} + +func buildKeywordsRegexp(words []string) *regexp.Regexp { + acceptedWords := parseKeywords(words) + if len(acceptedWords) == 0 { + // Never match + return nil + } + return regexp.MustCompile(fmt.Sprintf(`(?i)(?:%s)(?::?) %s`, strings.Join(acceptedWords, "|"), issueRefRegexpStr)) +} + diff --git a/models/action_test.go b/models/action_test.go index 16fdc7adcc904..dd256bb0984e0 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -233,6 +233,7 @@ func Test_getIssueFromRef(t *testing.T) { func TestUpdateIssuesCommit(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) + ActionPostConfigInit() pushCommits := []*PushCommit{ { Sha1: "abcdef1", @@ -542,3 +543,28 @@ func TestGetFeeds2(t *testing.T) { assert.NoError(t, err) assert.Len(t, actions, 0) } + +func TestParseCloseKeywords(t *testing.T) { + // Test parsing of CloseKeywords and ReopenKeywords + assert.Len(t, parseKeywords([]string{""}), 0) + assert.Len(t, parseKeywords([]string{" aa ", " bb ", "99", "#", "", "this is", "cc"}), 3) + + for _, test := range []struct { + pattern string + match string + }{ + {"close", "Close #123"}, + {"cerró", "cerró #123"}, + {"cerró", "CERRÓ #123"}, + {"закрывается", "закрывается #123"}, + {"κλείνει", "κλείνει #123"}, + {"关闭", "关闭 #123"}, + {"閉じます", "閉じます #123"}, + } { + pat := buildKeywordsRegexp([]string{test.pattern}) + assert.NotNil(t, pat) + res := pat.FindAllStringSubmatch(test.match, -1) + assert.Len(t, res, 1) + assert.EqualValues(t, [][]string([][]string{[]string{test.match, "", "", "#123"}}), res) + } +} diff --git a/models/models.go b/models/models.go index 04acc77aa94c7..3134210efe931 100644 --- a/models/models.go +++ b/models/models.go @@ -120,6 +120,11 @@ func init() { } } +// PostConfigInit performs initialization that requires app.ini settings to work +func PostConfigInit() { + ActionPostConfigInit() +} + func getEngine() (*xorm.Engine, error) { connStr, err := setting.DBConnStr() if err != nil { diff --git a/routers/init.go b/routers/init.go index fdf90904ce759..56729b8eff26b 100644 --- a/routers/init.go +++ b/routers/init.go @@ -75,6 +75,7 @@ func GlobalInit() { log.Trace("Log path: %s", setting.LogRootPath) NewServices() + models.PostConfigInit() if setting.InstallLock { highlight.NewContext() From 8281c2578b9f5419768f96d6be50b3134e8e2ea7 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 6 Sep 2019 17:40:10 -0300 Subject: [PATCH 3/4] Use sync.Once() for initialization --- models/action.go | 13 ++++++++----- models/action_test.go | 3 +-- models/models.go | 5 ----- routers/init.go | 1 - 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/models/action.go b/models/action.go index 7da8fc44d869c..f5d0f3d88e4b7 100644 --- a/models/action.go +++ b/models/action.go @@ -13,6 +13,7 @@ import ( "regexp" "strconv" "strings" + "sync" "time" "unicode" @@ -57,6 +58,7 @@ const ( var ( issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp issueReferenceKeywordsPat *regexp.Regexp + issueKeywordsOnce sync.Once ) const issueRefRegexpStr = `(?:([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+))?(#[0-9]+)+` @@ -66,10 +68,11 @@ func init() { issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStrNoKeyword) } -// ActionPostConfigInit performs initialization that requires app.ini settings to work -func ActionPostConfigInit() { - issueCloseKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.CloseKeywords) - issueReopenKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.ReopenKeywords) +func initKeywordsRegexp() { + issueKeywordsOnce.Do(func() { + issueCloseKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.CloseKeywords) + issueReopenKeywordsPat = buildKeywordsRegexp(setting.Repository.PullRequest.ReopenKeywords) + }) } // Action represents user operation type and other information to @@ -557,6 +560,7 @@ func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[i // UpdateIssuesCommit checks if issues are manipulated by commit message. func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, branchName string) error { + initKeywordsRegexp() // Commits are appended in the reverse order. for i := len(commits) - 1; i >= 0; i-- { c := commits[i] @@ -860,4 +864,3 @@ func buildKeywordsRegexp(words []string) *regexp.Regexp { } return regexp.MustCompile(fmt.Sprintf(`(?i)(?:%s)(?::?) %s`, strings.Join(acceptedWords, "|"), issueRefRegexpStr)) } - diff --git a/models/action_test.go b/models/action_test.go index dd256bb0984e0..3e69e75415b51 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -233,7 +233,6 @@ func Test_getIssueFromRef(t *testing.T) { func TestUpdateIssuesCommit(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - ActionPostConfigInit() pushCommits := []*PushCommit{ { Sha1: "abcdef1", @@ -565,6 +564,6 @@ func TestParseCloseKeywords(t *testing.T) { assert.NotNil(t, pat) res := pat.FindAllStringSubmatch(test.match, -1) assert.Len(t, res, 1) - assert.EqualValues(t, [][]string([][]string{[]string{test.match, "", "", "#123"}}), res) + assert.EqualValues(t, [][]string([][]string{{test.match, "", "", "#123"}}), res) } } diff --git a/models/models.go b/models/models.go index 3134210efe931..04acc77aa94c7 100644 --- a/models/models.go +++ b/models/models.go @@ -120,11 +120,6 @@ func init() { } } -// PostConfigInit performs initialization that requires app.ini settings to work -func PostConfigInit() { - ActionPostConfigInit() -} - func getEngine() (*xorm.Engine, error) { connStr, err := setting.DBConnStr() if err != nil { diff --git a/routers/init.go b/routers/init.go index 56729b8eff26b..fdf90904ce759 100644 --- a/routers/init.go +++ b/routers/init.go @@ -75,7 +75,6 @@ func GlobalInit() { log.Trace("Log path: %s", setting.LogRootPath) NewServices() - models.PostConfigInit() if setting.InstallLock { highlight.NewContext() From aea1fce04e2262e961604848109647d2bda58b33 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Wed, 16 Oct 2019 16:23:26 -0300 Subject: [PATCH 4/4] Fix unintended exported function --- modules/references/references.go | 8 ++++---- modules/references/references_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/references/references.go b/modules/references/references.go index 96a68ebada1cd..58a8da28957b9 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -128,14 +128,14 @@ func parseKeywords(words []string) []string { return acceptedWords } -func NewKeywords() { +func newKeywords() { issueKeywordsOnce.Do(func() { // Delay initialization until after the settings module is initialized - newKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) + doNewKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) }) } -func newKeywords(close []string, reopen []string) { +func doNewKeywords(close []string, reopen []string) { issueCloseKeywordsPat = makeKeywordsPat(close) issueReopenKeywordsPat = makeKeywordsPat(reopen) } @@ -334,7 +334,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere } func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { - NewKeywords() + newKeywords() var m []int if issueCloseKeywordsPat != nil { m = issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) diff --git a/modules/references/references_test.go b/modules/references/references_test.go index a1dd3a233a802..52e9b4ff524e8 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -329,11 +329,11 @@ func TestCustomizeCloseKeywords(t *testing.T) { issueKeywordsOnce.Do(func() {}) - newKeywords([]string{"cierra", "cerró"}, []string{"reabre"}) + doNewKeywords([]string{"cierra", "cerró"}, []string{"reabre"}) testFixtures(t, fixtures, "spanish") // Restore default settings - newKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) + doNewKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) } func TestParseCloseKeywords(t *testing.T) {