From b6131793dacc3e49411d6bf1ade6220221739dc1 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Thu, 3 Sep 2015 04:34:08 -0400 Subject: [PATCH] #1545 prevent duplicated refs of issues in single commit --- models/action.go | 111 ++++++++++++++++------------------------------- models/issue.go | 25 ++++++----- models/repo.go | 3 -- 3 files changed, 51 insertions(+), 88 deletions(-) diff --git a/models/action.go b/models/action.go index d1f0be4a7..1abc7363d 100644 --- a/models/action.go +++ b/models/action.go @@ -183,14 +183,17 @@ func RenameRepoAction(actUser *User, oldRepoName string, repo *Repository) error return renameRepoAction(x, actUser, oldRepoName, repo) } +func issueIndexTrimRight(c rune) bool { + return !unicode.IsDigit(c) +} + // updateIssuesCommit checks if issues are manipulated by commit message. func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string, commits []*base.PushCommit) error { for _, c := range commits { + refMarked := make(map[int64]bool) for _, ref := range IssueReferenceKeywordsPat.FindAllString(c.Message, -1) { - ref := ref[strings.IndexByte(ref, byte(' '))+1:] - ref = strings.TrimRightFunc(ref, func(c rune) bool { - return !unicode.IsDigit(c) - }) + ref = ref[strings.IndexByte(ref, byte(' '))+1:] + ref = strings.TrimRightFunc(ref, issueIndexTrimRight) if len(ref) == 0 { continue @@ -199,10 +202,9 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string // Add repo name if missing if ref[0] == '#' { ref = fmt.Sprintf("%s/%s%s", repoUserName, repoName, ref) - } else if strings.Contains(ref, "/") == false { + } else if !strings.Contains(ref, "/") { // FIXME: We don't support User#ID syntax yet // return ErrNotImplemented - continue } @@ -211,6 +213,11 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string return err } + if refMarked[issue.ID] { + continue + } + refMarked[issue.ID] = true + url := fmt.Sprintf("%s/%s/%s/commit/%s", setting.AppSubUrl, repoUserName, repoName, c.Sha1) message := fmt.Sprintf(`%s`, url, c.Message) if _, err = CreateComment(u, repo, issue, 0, 0, COMMENT_TYPE_COMMIT_REF, message, nil); err != nil { @@ -218,11 +225,10 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string } } + refMarked = make(map[int64]bool) for _, ref := range IssueCloseKeywordsPat.FindAllString(c.Message, -1) { - ref := ref[strings.IndexByte(ref, byte(' '))+1:] - ref = strings.TrimRightFunc(ref, func(c rune) bool { - return !unicode.IsDigit(c) - }) + ref = ref[strings.IndexByte(ref, byte(' '))+1:] + ref = strings.TrimRightFunc(ref, issueIndexTrimRight) if len(ref) == 0 { continue @@ -234,7 +240,6 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string } else if strings.Contains(ref, "/") == false { // We don't support User#ID syntax yet // return ErrNotImplemented - continue } @@ -243,45 +248,24 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string return err } - if issue.RepoID == repo.ID { - if issue.IsClosed { - continue - } - issue.IsClosed = true + if refMarked[issue.ID] { + continue + } + refMarked[issue.ID] = true - if err = issue.GetLabels(); err != nil { - return err - } - for _, label := range issue.Labels { - label.NumClosedIssues++ + if issue.RepoID != repo.ID || issue.IsClosed { + continue + } - if err = UpdateLabel(label); err != nil { - return err - } - } - - if err = UpdateIssue(issue); err != nil { - return err - } else if err = UpdateIssueUsersByStatus(issue.ID, issue.IsClosed); err != nil { - return err - } - - if err = ChangeMilestoneIssueStats(issue); err != nil { - return err - } - - // If commit happened in the referenced repository, it means the issue can be closed. - if _, err = CreateComment(u, repo, issue, 0, 0, COMMENT_TYPE_CLOSE, "", nil); err != nil { - return err - } + if err = issue.ChangeStatus(u, true); err != nil { + return err } } + // It is conflict to have close and reopen at same time, so refsMarkd doesn't need to reinit here. for _, ref := range IssueReopenKeywordsPat.FindAllString(c.Message, -1) { - ref := ref[strings.IndexByte(ref, byte(' '))+1:] - ref = strings.TrimRightFunc(ref, func(c rune) bool { - return !unicode.IsDigit(c) - }) + ref = ref[strings.IndexByte(ref, byte(' '))+1:] + ref = strings.TrimRightFunc(ref, issueIndexTrimRight) if len(ref) == 0 { continue @@ -293,7 +277,6 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string } else if strings.Contains(ref, "/") == false { // We don't support User#ID syntax yet // return ErrNotImplemented - continue } @@ -302,37 +285,17 @@ func updateIssuesCommit(u *User, repo *Repository, repoUserName, repoName string return err } - if issue.RepoID == repo.ID { - if !issue.IsClosed { - continue - } - issue.IsClosed = false + if refMarked[issue.ID] { + continue + } + refMarked[issue.ID] = true - if err = issue.GetLabels(); err != nil { - return err - } - for _, label := range issue.Labels { - label.NumClosedIssues-- + if issue.RepoID != repo.ID || !issue.IsClosed { + continue + } - if err = UpdateLabel(label); err != nil { - return err - } - } - - if err = UpdateIssue(issue); err != nil { - return err - } else if err = UpdateIssueUsersByStatus(issue.ID, issue.IsClosed); err != nil { - return err - } - - if err = ChangeMilestoneIssueStats(issue); err != nil { - return err - } - - // If commit happened in the referenced repository, it means the issue can be closed. - if _, err = CreateComment(u, repo, issue, 0, 0, COMMENT_TYPE_REOPEN, "", nil); err != nil { - return err - } + if err = issue.ChangeStatus(u, false); err != nil { + return err } } } diff --git a/models/issue.go b/models/issue.go index 1f6300776..dbc63b957 100644 --- a/models/issue.go +++ b/models/issue.go @@ -13,7 +13,6 @@ import ( "mime/multipart" "os" "path" - "strconv" "strings" "time" @@ -384,25 +383,29 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, uuids []string) // 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. -func GetIssueByRef(ref string) (issue *Issue, err error) { - var issueNumber int64 - var repo *Repository - +func GetIssueByRef(ref string) (*Issue, error) { n := strings.IndexByte(ref, byte('#')) - if n == -1 { return nil, ErrMissingIssueNumber } - if issueNumber, err = strconv.ParseInt(ref[n+1:], 10, 64); err != nil { - return + index, err := com.StrTo(ref[n+1:]).Int64() + if err != nil { + return nil, err } - if repo, err = GetRepositoryByRef(ref[:n]); err != nil { - return + repo, err := GetRepositoryByRef(ref[:n]) + if err != nil { + return nil, err } - return GetIssueByIndex(repo.ID, issueNumber) + issue, err := GetIssueByIndex(repo.ID, index) + if err != nil { + return nil, err + } + + issue.Repo = repo + return issue, nil } // GetIssueByIndex returns issue by given index in repository. diff --git a/models/repo.go b/models/repo.go index 5add81f28..706bb90fa 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1105,15 +1105,12 @@ func DeleteRepository(uid, repoID int64) error { // See https://help.github.com/articles/writing-on-github#references for more information on the syntax. func GetRepositoryByRef(ref string) (*Repository, error) { n := strings.IndexByte(ref, byte('/')) - if n < 2 { return nil, ErrInvalidReference } userName, repoName := ref[:n], ref[n+1:] - user, err := GetUserByName(userName) - if err != nil { return nil, err }