From 08ae6bb7edb9582c38edb8a0dba1b1be10fb00fc Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Mon, 18 Nov 2019 05:08:20 -0300 Subject: [PATCH] Rewrite delivery of issue and comment mails (#9009) * Mail issue subscribers, rework the function * Simplify a little more * Fix unused variable * Refactor mail delivery to avoid heavy load on server * Avoid splitting into too many goroutines * Fix comments and optimize GetMaileableUsersByIDs() * Fix return on errors --- models/issue.go | 13 +++ models/issue_assignees.go | 12 +++ models/issue_watch.go | 12 +++ models/repo_watch.go | 12 +++ models/user.go | 14 +++ services/mailer/mail.go | 94 ++++++++--------- services/mailer/mail_comment.go | 13 ++- services/mailer/mail_issue.go | 172 ++++++++++++++++++-------------- services/mailer/mail_test.go | 49 ++++++--- services/mailer/mailer.go | 11 +- 10 files changed, 254 insertions(+), 148 deletions(-) diff --git a/models/issue.go b/models/issue.go index 1fe9140da..0113f0a40 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1219,6 +1219,19 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) { return issues, nil } +// GetParticipantsIDsByIssueID returns the IDs of all users who participated in comments of an issue, +// but skips joining with `user` for performance reasons. +// User permissions must be verified elsewhere if required. +func GetParticipantsIDsByIssueID(issueID int64) ([]int64, error) { + userIDs := make([]int64, 0, 5) + return userIDs, x.Table("comment"). + Cols("poster_id"). + Where("issue_id = ?", issueID). + And("type in (?,?,?)", CommentTypeComment, CommentTypeCode, CommentTypeReview). + Distinct("poster_id"). + Find(&userIDs) +} + // GetParticipantsByIssueID returns all users who are participated in comments of an issue. func GetParticipantsByIssueID(issueID int64) ([]*User, error) { return getParticipantsByIssueID(x, issueID) diff --git a/models/issue_assignees.go b/models/issue_assignees.go index 4d71cc6e5..08a567c4e 100644 --- a/models/issue_assignees.go +++ b/models/issue_assignees.go @@ -41,6 +41,18 @@ func (issue *Issue) loadAssignees(e Engine) (err error) { return } +// GetAssigneeIDsByIssue returns the IDs of users assigned to an issue +// but skips joining with `user` for performance reasons. +// User permissions must be verified elsewhere if required. +func GetAssigneeIDsByIssue(issueID int64) ([]int64, error) { + userIDs := make([]int64, 0, 5) + return userIDs, x.Table("issue_assignees"). + Cols("assignee_id"). + Where("issue_id = ?", issueID). + Distinct("assignee_id"). + Find(&userIDs) +} + // GetAssigneesByIssue returns everyone assigned to that issue func GetAssigneesByIssue(issue *Issue) (assignees []*User, err error) { return getAssigneesByIssue(x, issue) diff --git a/models/issue_watch.go b/models/issue_watch.go index 1ae0c9d47..3d7d48a12 100644 --- a/models/issue_watch.go +++ b/models/issue_watch.go @@ -60,6 +60,18 @@ func getIssueWatch(e Engine, userID, issueID int64) (iw *IssueWatch, exists bool return } +// GetIssueWatchersIDs returns IDs of subscribers to a given issue id +// but avoids joining with `user` for performance reasons +// User permissions must be verified elsewhere if required +func GetIssueWatchersIDs(issueID int64) ([]int64, error) { + ids := make([]int64, 0, 64) + return ids, x.Table("issue_watch"). + Where("issue_id=?", issueID). + And("is_watching = ?", true). + Select("user_id"). + Find(&ids) +} + // GetIssueWatchers returns watchers/unwatchers of a given issue func GetIssueWatchers(issueID int64) (IssueWatchList, error) { return getIssueWatchers(x, issueID) diff --git a/models/repo_watch.go b/models/repo_watch.go index 2de4f8b32..2279dcb11 100644 --- a/models/repo_watch.go +++ b/models/repo_watch.go @@ -140,6 +140,18 @@ func GetWatchers(repoID int64) ([]*Watch, error) { return getWatchers(x, repoID) } +// GetRepoWatchersIDs returns IDs of watchers for a given repo ID +// but avoids joining with `user` for performance reasons +// User permissions must be verified elsewhere if required +func GetRepoWatchersIDs(repoID int64) ([]int64, error) { + ids := make([]int64, 0, 64) + return ids, x.Table("watch"). + Where("watch.repo_id=?", repoID). + And("watch.mode<>?", RepoWatchModeDont). + Select("user_id"). + Find(&ids) +} + // GetWatchers returns range of users watching given repository. func (repo *Repository) GetWatchers(page int) ([]*User, error) { users := make([]*User, 0, ItemsPerPage) diff --git a/models/user.go b/models/user.go index 4a8c644cc..ce78e5bfc 100644 --- a/models/user.go +++ b/models/user.go @@ -1307,6 +1307,20 @@ func getUserEmailsByNames(e Engine, names []string) []string { return mails } +// GetMaileableUsersByIDs gets users from ids, but only if they can receive mails +func GetMaileableUsersByIDs(ids []int64) ([]*User, error) { + if len(ids) == 0 { + return nil, nil + } + ous := make([]*User, 0, len(ids)) + return ous, x.In("id", ids). + Where("`type` = ?", UserTypeIndividual). + And("`prohibit_login` = ?", false). + And("`is_active` = ?", true). + And("`email_notifications_preference` = ?", EmailNotificationsEnabled). + Find(&ous) +} + // GetUsersByIDs returns all resolved users from a list of Ids. func GetUsersByIDs(ids []int64) ([]*User, error) { ous := make([]*User, 0, len(ids)) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 27767be68..7d26487a0 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -164,13 +164,7 @@ func SendCollaboratorMail(u, doer *models.User, repo *models.Repository) { SendAsync(msg) } -func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionType models.ActionType, fromMention bool, - content string, comment *models.Comment, tos []string, info string) *Message { - - if err := issue.LoadPullRequest(); err != nil { - log.Error("LoadPullRequest: %v", err) - return nil - } +func composeIssueCommentMessages(ctx *mailCommentContext, tos []string, fromMention bool, info string) []*Message { var ( subject string @@ -182,29 +176,29 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy ) commentType := models.CommentTypeComment - if comment != nil { + if ctx.Comment != nil { prefix = "Re: " - commentType = comment.Type - link = issue.HTMLURL() + "#" + comment.HashTag() + commentType = ctx.Comment.Type + link = ctx.Issue.HTMLURL() + "#" + ctx.Comment.HashTag() } else { - link = issue.HTMLURL() + link = ctx.Issue.HTMLURL() } reviewType := models.ReviewTypeComment - if comment != nil && comment.Review != nil { - reviewType = comment.Review.Type + if ctx.Comment != nil && ctx.Comment.Review != nil { + reviewType = ctx.Comment.Review.Type } - fallback = prefix + fallbackMailSubject(issue) + fallback = prefix + fallbackMailSubject(ctx.Issue) // This is the body of the new issue or comment, not the mail body - body := string(markup.RenderByType(markdown.MarkupName, []byte(content), issue.Repo.HTMLURL(), issue.Repo.ComposeMetas())) + body := string(markup.RenderByType(markdown.MarkupName, []byte(ctx.Content), ctx.Issue.Repo.HTMLURL(), ctx.Issue.Repo.ComposeMetas())) - actType, actName, tplName := actionToTemplate(issue, actionType, commentType, reviewType) + actType, actName, tplName := actionToTemplate(ctx.Issue, ctx.ActionType, commentType, reviewType) - if comment != nil && comment.Review != nil { + if ctx.Comment != nil && ctx.Comment.Review != nil { reviewComments = make([]*models.Comment, 0, 10) - for _, lines := range comment.Review.CodeComments { + for _, lines := range ctx.Comment.Review.CodeComments { for _, comments := range lines { reviewComments = append(reviewComments, comments...) } @@ -215,12 +209,12 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy "FallbackSubject": fallback, "Body": body, "Link": link, - "Issue": issue, - "Comment": comment, - "IsPull": issue.IsPull, - "User": issue.Repo.MustOwner(), - "Repo": issue.Repo.FullName(), - "Doer": doer, + "Issue": ctx.Issue, + "Comment": ctx.Comment, + "IsPull": ctx.Issue.IsPull, + "User": ctx.Issue.Repo.MustOwner(), + "Repo": ctx.Issue.Repo.FullName(), + "Doer": ctx.Doer, "IsMention": fromMention, "SubjectPrefix": prefix, "ActionType": actType, @@ -246,18 +240,23 @@ func composeIssueCommentMessage(issue *models.Issue, doer *models.User, actionTy log.Error("ExecuteTemplate [%s]: %v", string(tplName)+"/body", err) } - msg := NewMessageFrom(tos, doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) - msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) + // Make sure to compose independent messages to avoid leaking user emails + msgs := make([]*Message, 0, len(tos)) + for _, to := range tos { + msg := NewMessageFrom([]string{to}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) + msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) - // Set Message-ID on first message so replies know what to reference - if comment == nil { - msg.SetHeader("Message-ID", "<"+issue.ReplyReference()+">") - } else { - msg.SetHeader("In-Reply-To", "<"+issue.ReplyReference()+">") - msg.SetHeader("References", "<"+issue.ReplyReference()+">") + // Set Message-ID on first message so replies know what to reference + if ctx.Comment == nil { + msg.SetHeader("Message-ID", "<"+ctx.Issue.ReplyReference()+">") + } else { + msg.SetHeader("In-Reply-To", "<"+ctx.Issue.ReplyReference()+">") + msg.SetHeader("References", "<"+ctx.Issue.ReplyReference()+">") + } + msgs = append(msgs, msg) } - return msg + return msgs } func sanitizeSubject(subject string) string { @@ -269,21 +268,15 @@ func sanitizeSubject(subject string) string { return mime.QEncoding.Encode("utf-8", string(runes)) } -// SendIssueCommentMail composes and sends issue comment emails to target receivers. -func SendIssueCommentMail(issue *models.Issue, doer *models.User, actionType models.ActionType, content string, comment *models.Comment, tos []string) { - if len(tos) == 0 { - return - } - - SendAsync(composeIssueCommentMessage(issue, doer, actionType, false, content, comment, tos, "issue comment")) -} - -// SendIssueMentionMail composes and sends issue mention emails to target receivers. -func SendIssueMentionMail(issue *models.Issue, doer *models.User, actionType models.ActionType, content string, comment *models.Comment, tos []string) { - if len(tos) == 0 { - return - } - SendAsync(composeIssueCommentMessage(issue, doer, actionType, true, content, comment, tos, "issue mention")) +// SendIssueAssignedMail composes and sends issue assigned email +func SendIssueAssignedMail(issue *models.Issue, doer *models.User, content string, comment *models.Comment, tos []string) { + SendAsyncs(composeIssueCommentMessages(&mailCommentContext{ + Issue: issue, + Doer: doer, + ActionType: models.ActionType(0), + Content: content, + Comment: comment, + }, tos, false, "issue assigned")) } // actionToTemplate returns the type and name of the action facing the user @@ -341,8 +334,3 @@ func actionToTemplate(issue *models.Issue, actionType models.ActionType, } return } - -// SendIssueAssignedMail composes and sends issue assigned email -func SendIssueAssignedMail(issue *models.Issue, doer *models.User, content string, comment *models.Comment, tos []string) { - SendAsync(composeIssueCommentMessage(issue, doer, models.ActionType(0), false, content, comment, tos, "issue assigned")) -} diff --git a/services/mailer/mail_comment.go b/services/mailer/mail_comment.go index 6469eb1fa..9edbfabd4 100644 --- a/services/mailer/mail_comment.go +++ b/services/mailer/mail_comment.go @@ -27,11 +27,18 @@ func mailParticipantsComment(ctx models.DBContext, c *models.Comment, opType mod if err = models.UpdateIssueMentions(ctx, c.IssueID, userMentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", c.IssueID, err) } - mentions := make([]string, len(userMentions)) + mentions := make([]int64, len(userMentions)) for i, u := range userMentions { - mentions[i] = u.LowerName + mentions[i] = u.ID } - if err = mailIssueCommentToParticipants(issue, c.Poster, opType, c.Content, c, mentions); err != nil { + if err = mailIssueCommentToParticipants( + &mailCommentContext{ + Issue: issue, + Doer: c.Poster, + ActionType: opType, + Content: c.Content, + Comment: c, + }, mentions); err != nil { log.Error("mailIssueCommentToParticipants: %v", err) } return nil diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index 32b21b132..696adfadd 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -10,105 +10,118 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/references" - - "github.com/unknwon/com" ) func fallbackMailSubject(issue *models.Issue) string { return fmt.Sprintf("[%s] %s (#%d)", issue.Repo.FullName(), issue.Title, issue.Index) } +type mailCommentContext struct { + Issue *models.Issue + Doer *models.User + ActionType models.ActionType + Content string + Comment *models.Comment +} + // mailIssueCommentToParticipants can be used for both new issue creation and comment. // This function sends two list of emails: // 1. Repository watchers and users who are participated in comments. // 2. Users who are not in 1. but get mentioned in current issue/comment. -func mailIssueCommentToParticipants(issue *models.Issue, doer *models.User, actionType models.ActionType, content string, comment *models.Comment, mentions []string) error { +func mailIssueCommentToParticipants(ctx *mailCommentContext, mentions []int64) error { - watchers, err := models.GetWatchers(issue.RepoID) - if err != nil { - return fmt.Errorf("getWatchers [repo_id: %d]: %v", issue.RepoID, err) + // Required by the mail composer; make sure to load these before calling the async function + if err := ctx.Issue.LoadRepo(); err != nil { + return fmt.Errorf("LoadRepo(): %v", err) } - participants, err := models.GetParticipantsByIssueID(issue.ID) - if err != nil { - return fmt.Errorf("getParticipantsByIssueID [issue_id: %d]: %v", issue.ID, err) + if err := ctx.Issue.LoadPoster(); err != nil { + return fmt.Errorf("LoadPoster(): %v", err) + } + if err := ctx.Issue.LoadPullRequest(); err != nil { + return fmt.Errorf("LoadPullRequest(): %v", err) } - // In case the issue poster is not watching the repository and is active, - // even if we have duplicated in watchers, can be safely filtered out. - err = issue.LoadPoster() + // Enough room to avoid reallocations + unfiltered := make([]int64, 1, 64) + + // =========== Original poster =========== + unfiltered[0] = ctx.Issue.PosterID + + // =========== Assignees =========== + ids, err := models.GetAssigneeIDsByIssue(ctx.Issue.ID) if err != nil { - return fmt.Errorf("GetUserByID [%d]: %v", issue.PosterID, err) + return fmt.Errorf("GetAssigneeIDsByIssue(%d): %v", ctx.Issue.ID, err) } - if issue.PosterID != doer.ID && issue.Poster.IsActive && !issue.Poster.ProhibitLogin { - participants = append(participants, issue.Poster) + unfiltered = append(unfiltered, ids...) + + // =========== Participants (i.e. commenters, reviewers) =========== + ids, err = models.GetParticipantsIDsByIssueID(ctx.Issue.ID) + if err != nil { + return fmt.Errorf("GetParticipantsIDsByIssueID(%d): %v", ctx.Issue.ID, err) + } + unfiltered = append(unfiltered, ids...) + + // =========== Issue watchers =========== + ids, err = models.GetIssueWatchersIDs(ctx.Issue.ID) + if err != nil { + return fmt.Errorf("GetIssueWatchersIDs(%d): %v", ctx.Issue.ID, err) + } + unfiltered = append(unfiltered, ids...) + + // =========== Repo watchers =========== + // Make repo watchers last, since it's likely the list with the most users + ids, err = models.GetRepoWatchersIDs(ctx.Issue.RepoID) + if err != nil { + return fmt.Errorf("GetRepoWatchersIDs(%d): %v", ctx.Issue.RepoID, err) + } + unfiltered = append(ids, unfiltered...) + + visited := make(map[int64]bool, len(unfiltered)+len(mentions)+1) + + // Avoid mailing the doer + visited[ctx.Doer.ID] = true + + if err = mailIssueCommentBatch(ctx, unfiltered, visited, false); err != nil { + return fmt.Errorf("mailIssueCommentBatch(): %v", err) } - // Assignees must receive any communications - assignees, err := models.GetAssigneesByIssue(issue) - if err != nil { - return err + // =========== Mentions =========== + if err = mailIssueCommentBatch(ctx, mentions, visited, true); err != nil { + return fmt.Errorf("mailIssueCommentBatch() mentions: %v", err) } - for _, assignee := range assignees { - if assignee.ID != doer.ID { - participants = append(participants, assignee) + return nil +} + +func mailIssueCommentBatch(ctx *mailCommentContext, ids []int64, visited map[int64]bool, fromMention bool) error { + const batchSize = 100 + for i := 0; i < len(ids); i += batchSize { + var last int + if i+batchSize < len(ids) { + last = i + batchSize + } else { + last = len(ids) } - } - - tos := make([]string, 0, len(watchers)) // List of email addresses. - names := make([]string, 0, len(watchers)) - for i := range watchers { - if watchers[i].UserID == doer.ID { - continue + unique := make([]int64, 0, last-i) + for j := i; j < last; j++ { + id := ids[j] + if _, ok := visited[id]; !ok { + unique = append(unique, id) + visited[id] = true + } } - - to, err := models.GetUserByID(watchers[i].UserID) + recipients, err := models.GetMaileableUsersByIDs(unique) if err != nil { - return fmt.Errorf("GetUserByID [%d]: %v", watchers[i].UserID, err) + return err } - if to.IsOrganization() || to.EmailNotifications() != models.EmailNotificationsEnabled { - continue + // TODO: Check issue visibility for each user + // TODO: Separate recipients by language for i18n mail templates + tos := make([]string, len(recipients)) + for i := range recipients { + tos[i] = recipients[i].Email } - - tos = append(tos, to.Email) - names = append(names, to.Name) + SendAsyncs(composeIssueCommentMessages(ctx, tos, fromMention, "issue comments")) } - for i := range participants { - if participants[i].ID == doer.ID || - com.IsSliceContainsStr(names, participants[i].Name) || - participants[i].EmailNotifications() != models.EmailNotificationsEnabled { - continue - } - - tos = append(tos, participants[i].Email) - names = append(names, participants[i].Name) - } - - if err := issue.LoadRepo(); err != nil { - return err - } - - for _, to := range tos { - SendIssueCommentMail(issue, doer, actionType, content, comment, []string{to}) - } - - // Mail mentioned people and exclude watchers. - names = append(names, doer.Name) - tos = make([]string, 0, len(mentions)) // list of user names. - for i := range mentions { - if com.IsSliceContainsStr(names, mentions[i]) { - continue - } - - tos = append(tos, mentions[i]) - } - - emails := models.GetUserEmailsByNames(tos) - - for _, to := range emails { - SendIssueMentionMail(issue, doer, actionType, content, comment, []string{to}) - } - return nil } @@ -127,11 +140,18 @@ func mailParticipants(ctx models.DBContext, issue *models.Issue, doer *models.Us if err = models.UpdateIssueMentions(ctx, issue.ID, userMentions); err != nil { return fmt.Errorf("UpdateIssueMentions [%d]: %v", issue.ID, err) } - mentions := make([]string, len(userMentions)) + mentions := make([]int64, len(userMentions)) for i, u := range userMentions { - mentions[i] = u.LowerName + mentions[i] = u.ID } - if err = mailIssueCommentToParticipants(issue, doer, opType, issue.Content, nil, mentions); err != nil { + if err = mailIssueCommentToParticipants( + &mailCommentContext{ + Issue: issue, + Doer: doer, + ActionType: opType, + Content: issue.Content, + Comment: nil, + }, mentions); err != nil { log.Error("mailIssueCommentToParticipants: %v", err) } return nil diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index a10507e0e..fd87a157b 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -58,12 +58,16 @@ func TestComposeIssueCommentMessage(t *testing.T) { InitMailRender(stpl, btpl) tos := []string{"test@gitea.com", "test2@gitea.com"} - msg := composeIssueCommentMessage(issue, doer, models.ActionCommentIssue, false, "test body", comment, tos, "issue comment") + msgs := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue, + Content: "test body", Comment: comment}, tos, false, "issue comment") + assert.Len(t, msgs, 2) - subject := msg.GetHeader("Subject") - inreplyTo := msg.GetHeader("In-Reply-To") - references := msg.GetHeader("References") + mailto := msgs[0].GetHeader("To") + subject := msgs[0].GetHeader("Subject") + inreplyTo := msgs[0].GetHeader("In-Reply-To") + references := msgs[0].GetHeader("References") + assert.Len(t, mailto, 1, "exactly one recipient is expected in the To field") assert.Equal(t, "Re: ", subject[0][:4], "Comment reply subject should contain Re:") assert.Equal(t, "Re: [user2/repo1] @user2 #1 - issue1", subject[0]) assert.Equal(t, inreplyTo[0], "", "In-Reply-To header doesn't match") @@ -88,14 +92,18 @@ func TestComposeIssueMessage(t *testing.T) { InitMailRender(stpl, btpl) tos := []string{"test@gitea.com", "test2@gitea.com"} - msg := composeIssueCommentMessage(issue, doer, models.ActionCreateIssue, false, "test body", nil, tos, "issue create") + msgs := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue, + Content: "test body"}, tos, false, "issue create") + assert.Len(t, msgs, 2) - subject := msg.GetHeader("Subject") - messageID := msg.GetHeader("Message-ID") + mailto := msgs[0].GetHeader("To") + subject := msgs[0].GetHeader("Subject") + messageID := msgs[0].GetHeader("Message-ID") + assert.Len(t, mailto, 1, "exactly one recipient is expected in the To field") assert.Equal(t, "[user2/repo1] @user2 #1 - issue1", subject[0]) - assert.Nil(t, msg.GetHeader("In-Reply-To")) - assert.Nil(t, msg.GetHeader("References")) + assert.Nil(t, msgs[0].GetHeader("In-Reply-To")) + assert.Nil(t, msgs[0].GetHeader("References")) assert.Equal(t, messageID[0], "", "Message-ID header doesn't match") } @@ -134,20 +142,24 @@ func TestTemplateSelection(t *testing.T) { assert.Contains(t, wholemsg, expBody) } - msg := composeIssueCommentMessage(issue, doer, models.ActionCreateIssue, false, "test body", nil, tos, "TestTemplateSelection") + msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue, + Content: "test body"}, tos, false, "TestTemplateSelection") expect(t, msg, "issue/new/subject", "issue/new/body") comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment) - msg = composeIssueCommentMessage(issue, doer, models.ActionCommentIssue, false, "test body", comment, tos, "TestTemplateSelection") + msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue, + Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") expect(t, msg, "issue/default/subject", "issue/default/body") pull := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 2, Repo: repo, Poster: doer}).(*models.Issue) comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 4, Issue: pull}).(*models.Comment) - msg = composeIssueCommentMessage(pull, doer, models.ActionCommentIssue, false, "test body", comment, tos, "TestTemplateSelection") + msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: pull, Doer: doer, ActionType: models.ActionCommentIssue, + Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") expect(t, msg, "pull/comment/subject", "pull/comment/body") - msg = composeIssueCommentMessage(issue, doer, models.ActionCloseIssue, false, "test body", nil, tos, "TestTemplateSelection") - expect(t, msg, "[user2/repo1] issue1 (#1)", "issue/close/body") + msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCloseIssue, + Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") + expect(t, msg, "Re: [user2/repo1] issue1 (#1)", "issue/close/body") } func TestTemplateServices(t *testing.T) { @@ -173,7 +185,8 @@ func TestTemplateServices(t *testing.T) { InitMailRender(stpl, btpl) tos := []string{"test@gitea.com"} - msg := composeIssueCommentMessage(issue, doer, actionType, fromMention, "test body", comment, tos, "TestTemplateServices") + msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: actionType, + Content: "test body", Comment: comment}, tos, fromMention, "TestTemplateServices") subject := msg.GetHeader("Subject") msgbuf := new(bytes.Buffer) @@ -202,3 +215,9 @@ func TestTemplateServices(t *testing.T) { "Re: [user2/repo1] issue1 (#1)", "//Re: //") } + +func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, tos []string, fromMention bool, info string) *Message { + msgs := composeIssueCommentMessages(ctx, tos, fromMention, info) + assert.Len(t, msgs, 1) + return msgs[0] +} diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index d19ae7b2f..2e4aa8d71 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -295,9 +295,18 @@ func NewContext() { go processMailQueue() } -// SendAsync send mail asynchronous +// SendAsync send mail asynchronously func SendAsync(msg *Message) { go func() { mailQueue <- msg }() } + +// SendAsyncs send mails asynchronously +func SendAsyncs(msgs []*Message) { + go func() { + for _, msg := range msgs { + mailQueue <- msg + } + }() +}