#1545 prevent duplicated refs of issues in single commit

diff --git a/models/action.go b/models/action.go
index d1f0be4a7835189b77e0362e74d680d25fdee28c..1abc7363d2dcb6ae18badbe3ad5372162bb23a9d 100644 (file)
--- 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(`<a href="%s">%s</a>`, 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 err = issue.GetLabels(); err != nil {
-                                       return err
-                               }
-                               for _, label := range issue.Labels {
-                                       label.NumClosedIssues++
-
-                                       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 refMarked[issue.ID] {
+                               continue
+                       }
+                       refMarked[issue.ID] = true
+
+                       if issue.RepoID != repo.ID || issue.IsClosed {
+                               continue
+                       }
+
+                       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 err = issue.GetLabels(); err != nil {
-                                       return err
-                               }
-                               for _, label := range issue.Labels {
-                                       label.NumClosedIssues--
-
-                                       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 refMarked[issue.ID] {
+                               continue
+                       }
+                       refMarked[issue.ID] = true
+
+                       if issue.RepoID != repo.ID || !issue.IsClosed {
+                               continue
+                       }
+
+                       if err = issue.ChangeStatus(u, false); err != nil {
+                               return err
                        }
                }
        }

diff --git a/models/issue.go b/models/issue.go
index 1f63007768f37cee11e18c7c13d9068c6c651c85..dbc63b9574b531cd625f83d6a8704a128fb68bda 100644 (file)
--- 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 5add81f288bef05cb47281c3a069695982067cda..706bb90fa451e4bacefd7d5a7feadc0bd6d9ee79 100644 (file)
--- 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
        }