From 0c5c34d7ddaf31a6d8123dac36b221de61f5ff96 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Thu, 22 Dec 2016 17:00:39 +0800
Subject: [PATCH] UpdateIssueUsersByMentions was calling database write
 operations while (#443)

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
---
 models/issue.go         | 14 +++++++-------
 models/issue_comment.go |  8 +++++---
 models/issue_mail.go    |  2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/models/issue.go b/models/issue.go
index 0e6c794055d08..13d27b22c6255 100644
--- a/models/issue.go
+++ b/models/issue.go
@@ -1085,7 +1085,7 @@ func GetIssueUserPairsByMode(uid, rid int64, isClosed bool, page, filterMode int
 
 // UpdateIssueMentions extracts mentioned people from content and
 // 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 {
 		return nil
 	}
@@ -1095,7 +1095,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
 	}
 	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)
 	}
 
@@ -1119,7 +1119,7 @@ func UpdateIssueMentions(issueID int64, mentions []string) error {
 		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)
 	}
 
@@ -1361,22 +1361,22 @@ func UpdateIssueUserByRead(uid, issueID int64) error {
 }
 
 // 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 {
 		iu := &IssueUser{
 			UID:     uid,
 			IssueID: issueID,
 		}
-		has, err := x.Get(iu)
+		has, err := e.Get(iu)
 		if err != nil {
 			return err
 		}
 
 		iu.IsMentioned = true
 		if has {
-			_, err = x.Id(iu.ID).AllCols().Update(iu)
+			_, err = e.Id(iu.ID).AllCols().Update(iu)
 		} else {
-			_, err = x.Insert(iu)
+			_, err = e.Insert(iu)
 		}
 		if err != nil {
 			return err
diff --git a/models/issue_comment.go b/models/issue_comment.go
index fc615d5346a44..22c1e8cf912cf 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -187,9 +187,9 @@ func (c *Comment) EventTag() string {
 
 // MailParticipants sends new comment emails to repository watchers
 // and mentioned people.
-func (c *Comment) MailParticipants(opType ActionType, issue *Issue) (err error) {
+func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) {
 	mentions := markdown.FindAllMentions(c.Content)
-	if err = UpdateIssueMentions(c.IssueID, mentions); err != nil {
+	if err = UpdateIssueMentions(e, c.IssueID, mentions); err != nil {
 		return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err)
 	}
 
@@ -303,7 +303,9 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err
 		if err = notifyWatchers(e, act); err != nil {
 			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, nil
diff --git a/models/issue_mail.go b/models/issue_mail.go
index 56c1e0c278e70..88adda5619838 100644
--- a/models/issue_mail.go
+++ b/models/issue_mail.go
@@ -69,7 +69,7 @@ func mailIssueCommentToParticipants(issue *Issue, doer *User, mentions []string)
 // and mentioned people.
 func (issue *Issue) MailParticipants() (err error) {
 	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)
 	}