Browse Source

Fix database write context interleaving bug (#3822)

* UpdateIssueUsersByMentions was calling database write operations while
a transaction session was in progress. MailParticipants was failing
silently because of the SQLITE_LOCKED error. Make sure failures in
MailParticipants enter the log, and pass on the transaction context.

issue: let caller pass in database context, and use it
issue_comment: obtain database context to pass to UpdateIssueMentions
issue_comment: log any error from call to MailParticipants
issue_mail: pass on database context to UpdateIssueMentions

* issue: forgot debug statement
pull/3781/merge
stroucki 8 years ago committed by 无闻
parent
commit
e9f6a43073
  1. 14
      models/issue.go
  2. 8
      models/issue_comment.go
  3. 2
      models/issue_mail.go

14
models/issue.go

@ -1026,7 +1026,7 @@ func GetIssueUserPairsByMode(uid, rid int64, isClosed bool, page, filterMode int
// UpdateIssueMentions extracts mentioned people from content and // UpdateIssueMentions extracts mentioned people from content and
// updates issue-user relations for them. // updates issue-user relations for them.
func UpdateIssueMentions(issueID int64, mentions []string) error { func UpdateIssueMentions(e Engine, issueID int64, mentions []string) error {
if len(mentions) == 0 { if len(mentions) == 0 {
return nil return nil
} }
@ -1036,7 +1036,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
} }
users := make([]*User, 0, len(mentions)) users := make([]*User, 0, len(mentions))
if err := x.In("lower_name", mentions).Asc("lower_name").Find(&users); err != nil { if err := e.In("lower_name", mentions).Asc("lower_name").Find(&users); err != nil {
return fmt.Errorf("find mentioned users: %v", err) return fmt.Errorf("find mentioned users: %v", err)
} }
@ -1060,7 +1060,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
ids = append(ids, memberIDs...) ids = append(ids, memberIDs...)
} }
if err := UpdateIssueUsersByMentions(issueID, ids); err != nil { if err := UpdateIssueUsersByMentions(e, issueID, ids); err != nil {
return fmt.Errorf("UpdateIssueUsersByMentions: %v", err) return fmt.Errorf("UpdateIssueUsersByMentions: %v", err)
} }
@ -1293,22 +1293,22 @@ func UpdateIssueUserByRead(uid, issueID int64) error {
} }
// UpdateIssueUsersByMentions updates issue-user pairs by mentioning. // UpdateIssueUsersByMentions updates issue-user pairs by mentioning.
func UpdateIssueUsersByMentions(issueID int64, uids []int64) error { func UpdateIssueUsersByMentions(e Engine, issueID int64, uids []int64) error {
for _, uid := range uids { for _, uid := range uids {
iu := &IssueUser{ iu := &IssueUser{
UID: uid, UID: uid,
IssueID: issueID, IssueID: issueID,
} }
has, err := x.Get(iu) has, err := e.Get(iu)
if err != nil { if err != nil {
return err return err
} }
iu.IsMentioned = true iu.IsMentioned = true
if has { if has {
_, err = x.Id(iu.ID).AllCols().Update(iu) _, err = e.Id(iu.ID).AllCols().Update(iu)
} else { } else {
_, err = x.Insert(iu) _, err = e.Insert(iu)
} }
if err != nil { if err != nil {
return err return err

8
models/issue_comment.go

@ -162,9 +162,9 @@ func (c *Comment) EventTag() string {
// MailParticipants sends new comment emails to repository watchers // MailParticipants sends new comment emails to repository watchers
// and mentioned people. // and mentioned people.
func (cmt *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) { func (cmt *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) {
mentions := markdown.FindAllMentions(cmt.Content) mentions := markdown.FindAllMentions(cmt.Content)
if err = UpdateIssueMentions(cmt.IssueID, mentions); err != nil { if err = UpdateIssueMentions(e, cmt.IssueID, mentions); err != nil {
return fmt.Errorf("UpdateIssueMentions [%d]: %v", cmt.IssueID, err) return fmt.Errorf("UpdateIssueMentions [%d]: %v", cmt.IssueID, err)
} }
@ -278,7 +278,9 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
if err = notifyWatchers(e, act); err != nil { if err = notifyWatchers(e, act); err != nil {
log.Error(4, "notifyWatchers: %v", err) log.Error(4, "notifyWatchers: %v", err)
} }
comment.MailParticipants(act.OpType, opts.Issue) if err = comment.MailParticipants(e, act.OpType, opts.Issue); err != nil {
log.Error(4, "MailParticipants: %v", err)
}
} }
return comment, comment.loadAttributes(e) return comment, comment.loadAttributes(e)

2
models/issue_mail.go

@ -69,7 +69,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
// and mentioned people. // and mentioned people.
func (issue *Issue) MailParticipants() (err error) { func (issue *Issue) MailParticipants() (err error) {
mentions := markdown.FindAllMentions(issue.Content) mentions := markdown.FindAllMentions(issue.Content)
if err = UpdateIssueMentions(issue.ID, mentions); err != nil { if err = UpdateIssueMentions(x, issue.ID, mentions); err != nil {
return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err)
} }

Loading…
Cancel
Save