action: fix issue reference regexp and error handling (#6352)

4 files changed, 49 insertions, 29 deletions

diff --git a/internal/db/action.go b/internal/db/action.go
index d744162f..b44af9c7 100644
--- a/internal/db/action.go
+++ b/internal/db/action.go
@@ -57,9 +57,9 @@ var (
 	IssueCloseKeywords  = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"}
 	IssueReopenKeywords = []string{"reopen", "reopens", "reopened"}
 
-	IssueCloseKeywordsPat     = lazyregexp.New(assembleKeywordsPattern(IssueCloseKeywords))
-	IssueReopenKeywordsPat    = lazyregexp.New(assembleKeywordsPattern(IssueReopenKeywords))
-	IssueReferenceKeywordsPat = lazyregexp.New(`(?i)(?:)(^| )\S+`)
+	IssueCloseKeywordsPat  = lazyregexp.New(assembleKeywordsPattern(IssueCloseKeywords))
+	IssueReopenKeywordsPat = lazyregexp.New(assembleKeywordsPattern(IssueReopenKeywords))
+	issueReferencePattern  = lazyregexp.New(`(?i)(?:)(^| )\S*#\d+`)
 )
 
 func assembleKeywordsPattern(words []string) string {
@@ -321,8 +321,8 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err
 		c := commits[i]
 
 		refMarked := make(map[int64]bool)
-		for _, ref := range IssueReferenceKeywordsPat.FindAllString(c.Message, -1) {
-			ref = ref[strings.IndexByte(ref, byte(' '))+1:]
+		for _, ref := range issueReferencePattern.FindAllString(c.Message, -1) {
+			ref = strings.TrimSpace(ref)
 			ref = strings.TrimRightFunc(ref, issueIndexTrimRight)
 
 			if len(ref) == 0 {
@@ -455,7 +455,7 @@ type CommitRepoActionOptions struct {
 	Commits     *PushCommits
 }
 
-// CommitRepoAction adds new commit actio to the repository, and prepare corresponding webhooks.
+// CommitRepoAction adds new commit action to the repository, and prepare corresponding webhooks.
 func CommitRepoAction(opts CommitRepoActionOptions) error {
 	pusher, err := GetUserByName(opts.PusherName)
 	if err != nil {
diff --git a/internal/db/action_test.go b/internal/db/action_test.go
new file mode 100644
index 00000000..afd750bb
--- /dev/null
+++ b/internal/db/action_test.go
@@ -0,0 +1,41 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package db
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_issueReferencePattern(t *testing.T) {
+	tests := []struct {
+		name       string
+		message    string
+		expStrings []string
+	}{
+		{
+			name:       "no match",
+			message:    "Hello world!",
+			expStrings: nil,
+		},
+		{
+			name:       "contains issue numbers",
+			message:    "#123 is fixed, and #456 is WIP",
+			expStrings: []string{"#123", " #456"},
+		},
+		{
+			name:       "contains full issue references",
+			message:    "#123 is fixed, and user/repo#456 is WIP",
+			expStrings: []string{"#123", " user/repo#456"},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			strs := issueReferencePattern.FindAllString(test.message, -1)
+			assert.Equal(t, test.expStrings, strs)
+		})
+	}
+}
diff --git a/internal/db/errors/issue.go b/internal/db/errors/issue.go
deleted file mode 100644
index 4ea898fb..00000000
--- a/internal/db/errors/issue.go
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright 2017 The Gogs Authors. All rights reserved.
-// Use of this source code is governed by a MIT-style
-// license that can be found in the LICENSE file.
-
-package errors
-
-import "fmt"
-
-type InvalidIssueReference struct {
-	Ref string
-}
-
-func IsInvalidIssueReference(err error) bool {
-	_, ok := err.(InvalidIssueReference)
-	return ok
-}
-
-func (err InvalidIssueReference) Error() string {
-	return fmt.Sprintf("invalid issue reference [ref: %s]", err.Ref)
-}
diff --git a/internal/db/issue.go b/internal/db/issue.go
index bb935dd9..79d4a7b9 100644
--- a/internal/db/issue.go
+++ b/internal/db/issue.go
@@ -817,12 +817,11 @@ func (ErrIssueNotExist) NotFound() bool {
 	return true
 }
 
-// GetIssueByRef returns an Issue specified by a GFM reference.
-// See https://help.github.com/articles/writing-on-github#references for more information on the syntax.
+// GetIssueByRef returns an Issue specified by a GFM reference, e.g. owner/repo#123.
 func GetIssueByRef(ref string) (*Issue, error) {
 	n := strings.IndexByte(ref, byte('#'))
 	if n == -1 {
-		return nil, errors.InvalidIssueReference{Ref: ref}
+		return nil, ErrIssueNotExist{args: map[string]interface{}{"ref": ref}}
 	}
 
 	index := com.StrTo(ref[n+1:]).MustInt64()