Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid re-issuing redundant cross-references. #8734

Merged
merged 12 commits into from
Nov 18, 2019
19 changes: 5 additions & 14 deletions models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,11 +699,7 @@ func (issue *Issue) ChangeTitle(doer *User, oldTitle string) (err error) {
return fmt.Errorf("createChangeTitleComment: %v", err)
}

if err = issue.neuterCrossReferences(sess); err != nil {
return err
}

if err = issue.addCrossReferences(sess, doer); err != nil {
if err = issue.addCrossReferences(sess, doer, true); err != nil {
return err
}

Expand Down Expand Up @@ -762,10 +758,8 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
if err = updateIssueCols(sess, issue, "content"); err != nil {
return fmt.Errorf("UpdateIssueCols: %v", err)
}
if err = issue.neuterCrossReferences(sess); err != nil {
return err
}
if err = issue.addCrossReferences(sess, doer); err != nil {

if err = issue.addCrossReferences(sess, doer, true); err != nil {
return err
}

Expand Down Expand Up @@ -956,7 +950,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
if err = opts.Issue.loadAttributes(e); err != nil {
return err
}
return opts.Issue.addCrossReferences(e, doer)
return opts.Issue.addCrossReferences(e, doer, false)
}

// NewIssue creates new issue with labels for repository.
Expand Down Expand Up @@ -1568,13 +1562,10 @@ func UpdateIssue(issue *Issue) error {
if err := updateIssue(sess, issue); err != nil {
return err
}
if err := issue.neuterCrossReferences(sess); err != nil {
return err
}
if err := issue.loadPoster(sess); err != nil {
return err
}
if err := issue.addCrossReferences(sess, issue.Poster); err != nil {
if err := issue.addCrossReferences(sess, issue.Poster, true); err != nil {
return err
}
return sess.Commit()
Expand Down
7 changes: 2 additions & 5 deletions models/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
return nil, err
}

if err = comment.addCrossReferences(e, opts.Doer); err != nil {
if err = comment.addCrossReferences(e, opts.Doer, false); err != nil {
return nil, err
}

Expand Down Expand Up @@ -921,10 +921,7 @@ func UpdateComment(c *Comment, doer *User) error {
if err := c.loadIssue(sess); err != nil {
return err
}
if err := c.neuterCrossReferences(sess); err != nil {
return err
}
if err := c.addCrossReferences(sess, doer); err != nil {
if err := c.addCrossReferences(sess, doer, true); err != nil {
return err
}
if err := sess.Commit(); err != nil {
Expand Down
53 changes: 45 additions & 8 deletions models/issue_xref.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type crossReferencesContext struct {
Doer *User
OrigIssue *Issue
OrigComment *Comment
RemoveOld bool
}

func newCrossReference(e *xorm.Session, ctx *crossReferencesContext, xref *crossReference) error {
Expand All @@ -44,7 +45,7 @@ func newCrossReference(e *xorm.Session, ctx *crossReferencesContext, xref *cross
return err
}

func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
func findOldCrossReferences(e Engine, issueID int64, commentID int64) ([]*Comment, error) {
active := make([]*Comment, 0, 10)
sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens)
if issueID != 0 {
Expand All @@ -53,13 +54,22 @@ func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
if commentID != 0 {
sess = sess.And("`ref_comment_id` = ?", commentID)
}
if err := sess.Find(&active); err != nil || len(active) == 0 {
return active, sess.Find(&active)
}

func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
active, err := findOldCrossReferences(e, issueID, commentID)
if err != nil {
return err
}
ids := make([]int64, len(active))
for i, c := range active {
ids[i] = c.ID
}
return neuterCrossReferencesIds(e, ids)
}

func neuterCrossReferencesIds(e Engine, ids []int64) error {
_, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: references.XRefActionNeutered})
return err
}
Expand All @@ -72,7 +82,7 @@ func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
// \/ \/ \/
//

func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error {
func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User, removeOld bool) error {
var commentType CommentType
if issue.IsPull {
commentType = CommentTypePullRef
Expand All @@ -83,6 +93,7 @@ func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error {
Type: commentType,
Doer: doer,
OrigIssue: issue,
RemoveOld: removeOld,
}
return issue.createCrossReferences(e, ctx, issue.Title, issue.Content)
}
Expand All @@ -92,6 +103,35 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC
if err != nil {
return err
}
if ctx.RemoveOld {
var commentID int64
if ctx.OrigComment != nil {
commentID = ctx.OrigComment.ID
}
active, err := findOldCrossReferences(e, ctx.OrigIssue.ID, commentID)
if err != nil {
return err
}
ids := make([]int64, 0, len(active))
for _, c := range active {
found := false
for i, x := range xreflist {
if x.Issue.ID == c.IssueID && x.Action == c.RefAction {
found = true
xreflist = append(xreflist[:i], xreflist[i+1:]...)
break
}
}
if !found {
ids = append(ids, c.ID)
}
}
if len(ids) > 0 {
if err = neuterCrossReferencesIds(e, ids); err != nil {
return err
}
}
}
for _, xref := range xreflist {
if err = newCrossReference(e, ctx, xref); err != nil {
return err
Expand Down Expand Up @@ -179,10 +219,6 @@ func (issue *Issue) findReferencedIssue(e Engine, ctx *crossReferencesContext, r
return refIssue, nil
}

func (issue *Issue) neuterCrossReferences(e Engine) error {
return neuterCrossReferences(e, issue.ID, 0)
}

// _________ __
// \_ ___ \ ____ _____ _____ ____ _____/ |_
// / \ \/ / _ \ / \ / \_/ __ \ / \ __\
Expand All @@ -191,7 +227,7 @@ func (issue *Issue) neuterCrossReferences(e Engine) error {
// \/ \/ \/ \/ \/
//

func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error {
func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User, removeOld bool) error {
if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment {
return nil
}
Expand All @@ -203,6 +239,7 @@ func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error {
Doer: doer,
OrigIssue: comment.Issue,
OrigComment: comment,
RemoveOld: removeOld,
}
return comment.Issue.createCrossReferences(e, ctx, "", comment.Content)
}
Expand Down