diff options
Diffstat (limited to 'models')
| -rw-r--r-- | models/action.go | 177 | ||||
| -rw-r--r-- | models/action_test.go | 53 | ||||
| -rw-r--r-- | models/issue_comment.go | 11 | ||||
| -rw-r--r-- | models/issue_xref.go | 111 |
4 files changed, 83 insertions, 269 deletions
diff --git a/models/action.go b/models/action.go index 87088101f9..2d2999f880 100644 --- a/models/action.go +++ b/models/action.go @@ -10,15 +10,14 @@ import ( "fmt" "html" "path" - "regexp" "strconv" "strings" "time" - "unicode" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -54,29 +53,6 @@ const ( ActionMirrorSyncDelete // 20 ) -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 -) - -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() { - issueCloseKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueCloseKeywords)) - issueReopenKeywordsPat = regexp.MustCompile(assembleKeywordsPattern(issueReopenKeywords)) - issueReferenceKeywordsPat = regexp.MustCompile(issueRefRegexpStrNoKeyword) -} - // Action represents user operation type and other information to // repository. It implemented interface base.Actioner so that can be // used in template render. @@ -351,10 +327,6 @@ func RenameRepoAction(actUser *User, oldRepoName string, repo *Repository) error return renameRepoAction(x, actUser, oldRepoName, repo) } -func issueIndexTrimRight(c rune) bool { - return !unicode.IsDigit(c) -} - // PushCommit represents a commit in a push operation. type PushCommit struct { Sha1 string @@ -480,39 +452,9 @@ func (pc *PushCommits) AvatarLink(email string) string { } // getIssueFromRef returns the issue referenced by a ref. Returns a nil *Issue -// if the provided ref is misformatted or references a non-existent issue. -func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { - ref = ref[strings.IndexByte(ref, ' ')+1:] - ref = strings.TrimRightFunc(ref, issueIndexTrimRight) - - var refRepo *Repository - poundIndex := strings.IndexByte(ref, '#') - if poundIndex < 0 { - return nil, nil - } else if poundIndex == 0 { - refRepo = repo - } else { - slashIndex := strings.IndexByte(ref, '/') - if slashIndex < 0 || slashIndex >= poundIndex { - return nil, nil - } - ownerName := ref[:slashIndex] - repoName := ref[slashIndex+1 : poundIndex] - var err error - refRepo, err = GetRepositoryByOwnerAndName(ownerName, repoName) - if err != nil { - if IsErrRepoNotExist(err) { - return nil, nil - } - return nil, err - } - } - issueIndex, err := strconv.ParseInt(ref[poundIndex+1:], 10, 64) - if err != nil { - return nil, nil - } - - issue, err := GetIssueByIndex(refRepo.ID, issueIndex) +// if the provided ref references a non-existent issue. +func getIssueFromRef(repo *Repository, index int64) (*Issue, error) { + issue, err := GetIssueByIndex(repo.ID, index) if err != nil { if IsErrIssueNotExist(err) { return nil, nil @@ -522,20 +464,7 @@ func getIssueFromRef(repo *Repository, ref string) (*Issue, error) { return issue, nil } -func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[int64]bool, status bool) error { - issue, err := getIssueFromRef(repo, ref) - if err != nil { - return err - } - - if issue == nil || refMarked[issue.ID] { - return nil - } - refMarked[issue.ID] = true - - if issue.RepoID != repo.ID || issue.IsClosed == status { - return nil - } +func changeIssueStatus(repo *Repository, issue *Issue, doer *User, status bool) error { stopTimerIfAvailable := func(doer *User, issue *Issue) error { @@ -549,7 +478,7 @@ func changeIssueStatus(repo *Repository, doer *User, ref string, refMarked map[i } issue.Repo = repo - if err = issue.ChangeStatus(doer, status); err != nil { + if err := issue.ChangeStatus(doer, status); err != nil { // Don't return an error when dependencies are open as this would let the push fail if IsErrDependenciesLeft(err) { return stopTimerIfAvailable(doer, issue) @@ -566,99 +495,67 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra for i := len(commits) - 1; i >= 0; i-- { c := commits[i] - refMarked := make(map[int64]bool) + type markKey struct { + ID int64 + Action references.XRefAction + } + + refMarked := make(map[markKey]bool) var refRepo *Repository + var refIssue *Issue var err error - for _, m := range issueReferenceKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { - continue - } - ref := m[3] + for _, ref := range references.FindAllIssueReferences(c.Message) { // issue is from another repo - if len(m[1]) > 0 && len(m[2]) > 0 { - refRepo, err = GetRepositoryFromMatch(m[1], m[2]) + if len(ref.Owner) > 0 && len(ref.Name) > 0 { + refRepo, err = GetRepositoryFromMatch(ref.Owner, ref.Name) if err != nil { continue } } else { refRepo = repo } - issue, err := getIssueFromRef(refRepo, ref) - if err != nil { + if refIssue, err = getIssueFromRef(refRepo, ref.Index); err != nil { return err } - - if issue == nil || refMarked[issue.ID] { + if refIssue == nil { continue } - refMarked[issue.ID] = true - message := fmt.Sprintf(`<a href="%s/commit/%s">%s</a>`, repo.Link(), c.Sha1, html.EscapeString(c.Message)) - if err = CreateRefComment(doer, refRepo, issue, message, c.Sha1); err != nil { + perm, err := GetUserRepoPermission(refRepo, doer) + if err != nil { return err } - } - // Change issue status only if the commit has been pushed to the default branch. - // and if the repo is configured to allow only that - if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { - continue - } - refMarked = make(map[int64]bool) - for _, m := range issueCloseKeywordsPat.FindAllStringSubmatch(c.Message, -1) { - if len(m[3]) == 0 { + key := markKey{ID: refIssue.ID, Action: ref.Action} + if refMarked[key] { continue } - ref := m[3] + refMarked[key] = true - // 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 { + // only create comments for issues if user has permission for it + if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeIssues) { + message := fmt.Sprintf(`<a href="%s/commit/%s">%s</a>`, repo.Link(), c.Sha1, html.EscapeString(c.Message)) + if err = CreateRefComment(doer, refRepo, refIssue, message, c.Sha1); 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 { + // Process closing/reopening keywords + if ref.Action != references.XRefActionCloses && ref.Action != references.XRefActionReopens { 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 { - continue - } - } else { - refRepo = repo - } - - perm, err := GetUserRepoPermission(refRepo, doer) - if err != nil { - return err + // Change issue status only if the commit has been pushed to the default branch. + // and if the repo is configured to allow only that + // FIXME: we should be using Issue.ref if set instead of repo.DefaultBranch + if repo.DefaultBranch != branchName && !repo.CloseIssuesViaCommitInAnyBranch { + continue } - // 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 { + // only close issues in another repo if user has push access + if perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeCode) { + if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil { return err } } diff --git a/models/action_test.go b/models/action_test.go index c90538ebe6..df41556850 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -1,7 +1,6 @@ package models import ( - "fmt" "path" "strings" "testing" @@ -181,56 +180,6 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("nonexistent@example.com")) } -func TestRegExp_issueReferenceKeywordsPat(t *testing.T) { - trueTestCases := []string{ - "#2", - "[#2]", - "please see go-gitea/gitea#5", - "#2:", - } - falseTestCases := []string{ - "kb#2", - "#2xy", - } - - for _, testCase := range trueTestCases { - assert.True(t, issueReferenceKeywordsPat.MatchString(testCase)) - } - for _, testCase := range falseTestCases { - assert.False(t, issueReferenceKeywordsPat.MatchString(testCase)) - } -} - -func Test_getIssueFromRef(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - for _, test := range []struct { - Ref string - ExpectedIssueID int64 - }{ - {"#2", 2}, - {"reopen #2", 2}, - {"user2/repo2#1", 4}, - {"fixes user2/repo2#1", 4}, - {"fixes: user2/repo2#1", 4}, - } { - issue, err := getIssueFromRef(repo, test.Ref) - assert.NoError(t, err) - if assert.NotNil(t, issue) { - assert.EqualValues(t, test.ExpectedIssueID, issue.ID) - } - } - - for _, badRef := range []string{ - "doesnotexist/doesnotexist#1", - fmt.Sprintf("#%d", NonexistentID), - } { - issue, err := getIssueFromRef(repo, badRef) - assert.NoError(t, err) - assert.Nil(t, issue) - } -} - func TestUpdateIssuesCommit(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pushCommits := []*PushCommit{ @@ -431,7 +380,7 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) { AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, issueBean, "is_closed=1") assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch)) - AssertExistsAndLoadBean(t, commentBean) + AssertNotExistsBean(t, commentBean) AssertNotExistsBean(t, issueBean, "is_closed=1") CheckConsistencyFor(t, &Action{}) } diff --git a/models/issue_comment.go b/models/issue_comment.go index e8043c1ec7..7d38302b98 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/references" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -144,10 +145,10 @@ type Comment struct { // Reference an issue or pull from another comment, issue or PR // All information is about the origin of the reference - RefRepoID int64 `xorm:"index"` // Repo where the referencing - RefIssueID int64 `xorm:"index"` - RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's) - RefAction XRefAction `xorm:"SMALLINT"` // What hapens if RefIssueID resolves + RefRepoID int64 `xorm:"index"` // Repo where the referencing + RefIssueID int64 `xorm:"index"` + RefCommentID int64 `xorm:"index"` // 0 if origin is Issue title or content (or PR's) + RefAction references.XRefAction `xorm:"SMALLINT"` // What hapens if RefIssueID resolves RefIsPull bool RefRepo *Repository `xorm:"-"` @@ -773,7 +774,7 @@ type CreateCommentOptions struct { RefRepoID int64 RefIssueID int64 RefCommentID int64 - RefAction XRefAction + RefAction references.XRefAction RefIsPull bool } diff --git a/models/issue_xref.go b/models/issue_xref.go index 1cc0bcfe6a..141a7e0e8c 100644 --- a/models/issue_xref.go +++ b/models/issue_xref.go @@ -5,42 +5,16 @@ package models import ( - "regexp" - "strconv" - "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" "github.com/go-xorm/xorm" "github.com/unknwon/com" ) -var ( - // TODO: Unify all regexp treatment of cross references in one place - - // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 - issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(?:#)([0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) - // crossReferenceIssueNumericPattern matches string that references a numeric issue in a different repository - // e.g. gogits/gogs#12345 - crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+)/([0-9a-zA-Z-_\.]+)#([0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) -) - -// XRefAction represents the kind of effect a cross reference has once is resolved -type XRefAction int64 - -const ( - // XRefActionNone means the cross-reference is a mention (commit, etc.) - XRefActionNone XRefAction = iota // 0 - // XRefActionCloses means the cross-reference should close an issue if it is resolved - XRefActionCloses // 1 - not implemented yet - // XRefActionReopens means the cross-reference should reopen an issue if it is resolved - XRefActionReopens // 2 - Not implemented yet - // XRefActionNeutered means the cross-reference will no longer affect the source - XRefActionNeutered // 3 -) - type crossReference struct { Issue *Issue - Action XRefAction + Action references.XRefAction } // crossReferencesContext is context to pass along findCrossReference functions @@ -72,7 +46,7 @@ func newCrossReference(e *xorm.Session, ctx *crossReferencesContext, xref *cross func neuterCrossReferences(e Engine, issueID int64, commentID int64) error { active := make([]*Comment, 0, 10) - sess := e.Where("`ref_action` IN (?, ?, ?)", XRefActionNone, XRefActionCloses, XRefActionReopens) + sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens) if issueID != 0 { sess = sess.And("`ref_issue_id` = ?", issueID) } @@ -86,7 +60,7 @@ func neuterCrossReferences(e Engine, issueID int64, commentID int64) error { for i, c := range active { ids[i] = c.ID } - _, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: XRefActionNeutered}) + _, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: references.XRefActionNeutered}) return err } @@ -110,11 +84,11 @@ func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error { Doer: doer, OrigIssue: issue, } - return issue.createCrossReferences(e, ctx, issue.Title+"\n"+issue.Content) + return issue.createCrossReferences(e, ctx, issue.Title, issue.Content) } -func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesContext, content string) error { - xreflist, err := ctx.OrigIssue.getCrossReferences(e, ctx, content) +func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) error { + xreflist, err := ctx.OrigIssue.getCrossReferences(e, ctx, plaincontent, mdcontent) if err != nil { return err } @@ -126,47 +100,43 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC return nil } -func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, content string) ([]*crossReference, error) { +func (issue *Issue) getCrossReferences(e *xorm.Session, ctx *crossReferencesContext, plaincontent, mdcontent string) ([]*crossReference, error) { xreflist := make([]*crossReference, 0, 5) - var xref *crossReference - - // Issues in the same repository - // FIXME: Should we support IssueNameStyleAlphanumeric? - matches := issueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if index, err := strconv.ParseInt(match[1], 10, 64); err == nil { - if err = ctx.OrigIssue.loadRepo(e); err != nil { + var ( + refRepo *Repository + refIssue *Issue + err error + ) + + allrefs := append(references.FindAllIssueReferences(plaincontent), references.FindAllIssueReferencesMarkdown(mdcontent)...) + + for _, ref := range allrefs { + if ref.Owner == "" && ref.Name == "" { + // Issues in the same repository + if err := ctx.OrigIssue.loadRepo(e); err != nil { return nil, err } - if xref, err = ctx.OrigIssue.isValidCommentReference(e, ctx, issue.Repo, index); err != nil { - return nil, err - } - if xref != nil { - xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, xref) - } - } - } - - // Issues in other repositories - matches = crossReferenceIssueNumericPattern.FindAllStringSubmatch(content, -1) - for _, match := range matches { - if index, err := strconv.ParseInt(match[3], 10, 64); err == nil { - repo, err := getRepositoryByOwnerAndName(e, match[1], match[2]) + refRepo = ctx.OrigIssue.Repo + } else { + // Issues in other repositories + refRepo, err = getRepositoryByOwnerAndName(e, ref.Owner, ref.Name) if err != nil { if IsErrRepoNotExist(err) { continue } return nil, err } - if err = ctx.OrigIssue.loadRepo(e); err != nil { - return nil, err - } - if xref, err = issue.isValidCommentReference(e, ctx, repo, index); err != nil { - return nil, err - } - if xref != nil { - xreflist = issue.updateCrossReferenceList(xreflist, xref) - } + } + if refIssue, err = ctx.OrigIssue.findReferencedIssue(e, ctx, refRepo, ref.Index); err != nil { + return nil, err + } + if refIssue != nil { + xreflist = ctx.OrigIssue.updateCrossReferenceList(xreflist, &crossReference{ + Issue: refIssue, + // FIXME: currently ignore keywords + // Action: ref.Action, + Action: references.XRefActionNone, + }) } } @@ -179,7 +149,7 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross } for i, r := range list { if r.Issue.ID == xref.Issue.ID { - if xref.Action != XRefActionNone { + if xref.Action != references.XRefActionNone { list[i].Action = xref.Action } return list @@ -188,7 +158,7 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross return append(list, xref) } -func (issue *Issue) isValidCommentReference(e Engine, ctx *crossReferencesContext, repo *Repository, index int64) (*crossReference, error) { +func (issue *Issue) findReferencedIssue(e Engine, ctx *crossReferencesContext, repo *Repository, index int64) (*Issue, error) { refIssue := &Issue{RepoID: repo.ID, Index: index} if has, _ := e.Get(refIssue); !has { return nil, nil @@ -206,10 +176,7 @@ func (issue *Issue) isValidCommentReference(e Engine, ctx *crossReferencesContex return nil, nil } } - return &crossReference{ - Issue: refIssue, - Action: XRefActionNone, - }, nil + return refIssue, nil } func (issue *Issue) neuterCrossReferences(e Engine) error { @@ -237,7 +204,7 @@ func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error { OrigIssue: comment.Issue, OrigComment: comment, } - return comment.Issue.createCrossReferences(e, ctx, comment.Content) + return comment.Issue.createCrossReferences(e, ctx, "", comment.Content) } func (comment *Comment) neuterCrossReferences(e Engine) error { |