Fix index produces problem when issues/pulls deleted (#6973)

* fix index produces problem when issues/pulls deleted

* fix tests

* fix tests

* fix tests

diff --git a/models/issue.go b/models/issue.go
index f9394594be75ea6673c90ee18645f2d44f2c0f87..999bd2f7a9b4d2de8ce670da01b687cd82f02359 100644 (file)
--- a/models/issue.go
+++ b/models/issue.go
@@ -5,6 +5,7 @@
 package models
 
 import (
+       "errors"
        "fmt"
        "path"
        "regexp"
@@ -1015,9 +1016,35 @@ type NewIssueOptions struct {
        IsPull      bool
 }
 
+// GetMaxIndexOfIssue returns the max index on issue
+func GetMaxIndexOfIssue(repoID int64) (int64, error) {
+       return getMaxIndexOfIssue(x, repoID)
+}
+
+func getMaxIndexOfIssue(e Engine, repoID int64) (int64, error) {
+       var (
+               maxIndex int64
+               has      bool
+               err      error
+       )
+
+       has, err = e.SQL("SELECT COALESCE((SELECT MAX(`index`) FROM issue WHERE repo_id = ?),0)", repoID).Get(&maxIndex)
+       if err != nil {
+               return 0, err
+       } else if !has {
+               return 0, errors.New("Retrieve Max index from issue failed")
+       }
+       return maxIndex, nil
+}
+
 func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
        opts.Issue.Title = strings.TrimSpace(opts.Issue.Title)
-       opts.Issue.Index = opts.Repo.NextIssueIndex()
+
+       maxIndex, err := getMaxIndexOfIssue(e, opts.Issue.RepoID)
+       if err != nil {
+               return err
+       }
+       opts.Issue.Index = maxIndex + 1
 
        if opts.Issue.MilestoneID > 0 {
                milestone, err := getMilestoneByRepoID(e, opts.Issue.RepoID, opts.Issue.MilestoneID)

diff --git a/models/repo.go b/models/repo.go
index b37120236f75fce01459847cc9c7c9ceb7391de6..3283223d5bbd46813f9165af28e06f0d0e36de29 100644 (file)
--- a/models/repo.go
+++ b/models/repo.go
@@ -687,13 +687,6 @@ func (repo *Repository) getUsersWithAccessMode(e Engine, mode AccessMode) (_ []*
        return users, nil
 }
 
-// NextIssueIndex returns the next issue index
-// FIXME: should have a mutex to prevent producing same index for two issues that are created
-// closely enough.
-func (repo *Repository) NextIssueIndex() int64 {
-       return int64(repo.NumIssues+repo.NumPulls) + 1
-}
-
 var (
        descPattern = regexp.MustCompile(`https?://\S+`)
 )

diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index dda8387913730c4b15af291f6b17fe98367af15d..f53ab4b8f3e547a46c5dcbd630e2494ddb7d1f8b 100644 (file)
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -251,9 +251,15 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
                deadlineUnix = util.TimeStamp(form.Deadline.Unix())
        }
 
+       maxIndex, err := models.GetMaxIndexOfIssue(repo.ID)
+       if err != nil {
+               ctx.ServerError("GetPatch", err)
+               return
+       }
+
        prIssue := &models.Issue{
                RepoID:       repo.ID,
-               Index:        repo.NextIssueIndex(),
+               Index:        maxIndex + 1,
                Title:        form.Title,
                PosterID:     ctx.User.ID,
                Poster:       ctx.User,

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index d1e2f0b0b32a289a46ee4ba0cc7b01e2c02dc15f..412750dd55d2892f8928a66f4c46ff4de79cedc7 100644 (file)
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -946,9 +946,15 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
                return
        }
 
+       maxIndex, err := models.GetMaxIndexOfIssue(repo.ID)
+       if err != nil {
+               ctx.ServerError("GetPatch", err)
+               return
+       }
+
        pullIssue := &models.Issue{
                RepoID:      repo.ID,
-               Index:       repo.NextIssueIndex(),
+               Index:       maxIndex + 1,
                Title:       form.Title,
                PosterID:    ctx.User.ID,
                Poster:      ctx.User,