Skip to content

Commit 2c2b971

Browse files
guillep2klafriks
guillep2k
authored and
lafriks
committed
Avoid re-issuing redundant cross-references. (#8734)
* Avoid re-issuing redundant cross-references. * Remove unused func; fix lint * Simplify code as suggested by @laftriks * Update test
1 parent f128e06 commit 2c2b971

File tree

4 files changed

+56
-31
lines changed

4 files changed

+56
-31
lines changed

‎models/issue.go‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -722,11 +722,7 @@ func (issue *Issue) ChangeTitle(doer *User, oldTitle string) (err error) {
722722
return fmt.Errorf("createComment: %v", err)
723723
}
724724

725-
if err = issue.neuterCrossReferences(sess); err != nil {
726-
return err
727-
}
728-
729-
if err = issue.addCrossReferences(sess, doer); err != nil {
725+
if err = issue.addCrossReferences(sess, doer, true); err != nil {
730726
return err
731727
}
732728

@@ -790,10 +786,8 @@ func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
790786
if err = updateIssueCols(sess, issue, "content"); err != nil {
791787
return fmt.Errorf("UpdateIssueCols: %v", err)
792788
}
793-
if err = issue.neuterCrossReferences(sess); err != nil {
794-
return err
795-
}
796-
if err = issue.addCrossReferences(sess, doer); err != nil {
789+
790+
if err = issue.addCrossReferences(sess, doer, true); err != nil {
797791
return err
798792
}
799793

@@ -944,7 +938,7 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) {
944938
if err = opts.Issue.loadAttributes(e); err != nil {
945939
return err
946940
}
947-
return opts.Issue.addCrossReferences(e, doer)
941+
return opts.Issue.addCrossReferences(e, doer, false)
948942
}
949943

950944
// NewIssue creates new issue with labels for repository.
@@ -1578,13 +1572,10 @@ func UpdateIssue(issue *Issue) error {
15781572
if err := updateIssue(sess, issue); err != nil {
15791573
return err
15801574
}
1581-
if err := issue.neuterCrossReferences(sess); err != nil {
1582-
return err
1583-
}
15841575
if err := issue.loadPoster(sess); err != nil {
15851576
return err
15861577
}
1587-
if err := issue.addCrossReferences(sess, issue.Poster); err != nil {
1578+
if err := issue.addCrossReferences(sess, issue.Poster, true); err != nil {
15881579
return err
15891580
}
15901581
return sess.Commit()

‎models/issue_comment.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,7 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
545545
}
546546
}
547547

548-
if err = comment.addCrossReferences(e, opts.Doer); err != nil {
548+
if err = comment.addCrossReferences(e, opts.Doer, false); err != nil {
549549
return nil, err
550550
}
551551

@@ -882,10 +882,7 @@ func UpdateComment(c *Comment, doer *User) error {
882882
if err := c.loadIssue(sess); err != nil {
883883
return err
884884
}
885-
if err := c.neuterCrossReferences(sess); err != nil {
886-
return err
887-
}
888-
if err := c.addCrossReferences(sess, doer); err != nil {
885+
if err := c.addCrossReferences(sess, doer, true); err != nil {
889886
return err
890887
}
891888
if err := sess.Commit(); err != nil {

‎models/issue_xref.go‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,30 @@ type crossReferencesContext struct {
2525
Doer *User
2626
OrigIssue *Issue
2727
OrigComment *Comment
28+
RemoveOld bool
2829
}
2930

30-
func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
31+
func findOldCrossReferences(e Engine, issueID int64, commentID int64) ([]*Comment, error) {
3132
active := make([]*Comment, 0, 10)
32-
sess := e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens).
33+
return active, e.Where("`ref_action` IN (?, ?, ?)", references.XRefActionNone, references.XRefActionCloses, references.XRefActionReopens).
3334
And("`ref_issue_id` = ?", issueID).
34-
And("`ref_comment_id` = ?", commentID)
35-
if err := sess.Find(&active); err != nil || len(active) == 0 {
35+
And("`ref_comment_id` = ?", commentID).
36+
Find(&active)
37+
}
38+
39+
func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
40+
active, err := findOldCrossReferences(e, issueID, commentID)
41+
if err != nil {
3642
return err
3743
}
3844
ids := make([]int64, len(active))
3945
for i, c := range active {
4046
ids[i] = c.ID
4147
}
48+
return neuterCrossReferencesIds(e, ids)
49+
}
50+
51+
func neuterCrossReferencesIds(e Engine, ids []int64) error {
4252
_, err := e.In("id", ids).Cols("`ref_action`").Update(&Comment{RefAction: references.XRefActionNeutered})
4353
return err
4454
}
@@ -51,7 +61,7 @@ func neuterCrossReferences(e Engine, issueID int64, commentID int64) error {
5161
// \/ \/ \/
5262
//
5363

54-
func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error {
64+
func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User, removeOld bool) error {
5565
var commentType CommentType
5666
if issue.IsPull {
5767
commentType = CommentTypePullRef
@@ -62,6 +72,7 @@ func (issue *Issue) addCrossReferences(e *xorm.Session, doer *User) error {
6272
Type: commentType,
6373
Doer: doer,
6474
OrigIssue: issue,
75+
RemoveOld: removeOld,
6576
}
6677
return issue.createCrossReferences(e, ctx, issue.Title, issue.Content)
6778
}
@@ -71,6 +82,35 @@ func (issue *Issue) createCrossReferences(e *xorm.Session, ctx *crossReferencesC
7182
if err != nil {
7283
return err
7384
}
85+
if ctx.RemoveOld {
86+
var commentID int64
87+
if ctx.OrigComment != nil {
88+
commentID = ctx.OrigComment.ID
89+
}
90+
active, err := findOldCrossReferences(e, ctx.OrigIssue.ID, commentID)
91+
if err != nil {
92+
return err
93+
}
94+
ids := make([]int64, 0, len(active))
95+
for _, c := range active {
96+
found := false
97+
for i, x := range xreflist {
98+
if x.Issue.ID == c.IssueID && x.Action == c.RefAction {
99+
found = true
100+
xreflist = append(xreflist[:i], xreflist[i+1:]...)
101+
break
102+
}
103+
}
104+
if !found {
105+
ids = append(ids, c.ID)
106+
}
107+
}
108+
if len(ids) > 0 {
109+
if err = neuterCrossReferencesIds(e, ids); err != nil {
110+
return err
111+
}
112+
}
113+
}
74114
for _, xref := range xreflist {
75115
var refCommentID int64
76116
if ctx.OrigComment != nil {
@@ -193,10 +233,6 @@ func (issue *Issue) verifyReferencedIssue(e Engine, ctx *crossReferencesContext,
193233
return refIssue, refAction, nil
194234
}
195235

196-
func (issue *Issue) neuterCrossReferences(e Engine) error {
197-
return neuterCrossReferences(e, issue.ID, 0)
198-
}
199-
200236
// _________ __
201237
// \_ ___ \ ____ _____ _____ ____ _____/ |_
202238
// / \ \/ / _ \ / \ / \_/ __ \ / \ __\
@@ -205,7 +241,7 @@ func (issue *Issue) neuterCrossReferences(e Engine) error {
205241
// \/ \/ \/ \/ \/
206242
//
207243

208-
func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error {
244+
func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User, removeOld bool) error {
209245
if comment.Type != CommentTypeCode && comment.Type != CommentTypeComment {
210246
return nil
211247
}
@@ -217,6 +253,7 @@ func (comment *Comment) addCrossReferences(e *xorm.Session, doer *User) error {
217253
Doer: doer,
218254
OrigIssue: comment.Issue,
219255
OrigComment: comment,
256+
RemoveOld: removeOld,
220257
}
221258
return comment.Issue.createCrossReferences(e, ctx, "", comment.Content)
222259
}

‎models/issue_xref_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func testCreateIssue(t *testing.T, repo, doer int64, title, content string, ispu
133133
assert.NoError(t, err)
134134
i, err = getIssueByID(sess, i.ID)
135135
assert.NoError(t, err)
136-
assert.NoError(t, i.addCrossReferences(sess, d))
136+
assert.NoError(t, i.addCrossReferences(sess, d, false))
137137
assert.NoError(t, sess.Commit())
138138
return i
139139
}
@@ -158,7 +158,7 @@ func testCreateComment(t *testing.T, repo, doer, issue int64, content string) *C
158158
assert.NoError(t, sess.Begin())
159159
_, err := sess.Insert(c)
160160
assert.NoError(t, err)
161-
assert.NoError(t, c.addCrossReferences(sess, d))
161+
assert.NoError(t, c.addCrossReferences(sess, d, false))
162162
assert.NoError(t, sess.Commit())
163163
return c
164164
}

0 commit comments

Comments
 (0)