From 55f1fcf0ad20d69fcd62c764f6da74ae56bd5f73 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 17 Sep 2024 22:56:26 +0200 Subject: [PATCH] Add missing comment reply handling (#32050) Fixes #31937 - Add missing comment reply handling - Use `onGiteaRun` in the test because the fixtures are not present otherwise (did this behaviour change?) Compare without whitespaces. --- services/mailer/incoming/incoming_handler.go | 62 ++-- tests/integration/incoming_email_test.go | 304 ++++++++++--------- 2 files changed, 186 insertions(+), 180 deletions(-) diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index dc0b539822..38a234eac1 100644 --- a/services/mailer/incoming/incoming_handler.go +++ b/services/mailer/incoming/incoming_handler.go @@ -82,43 +82,40 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u return nil } + attachmentIDs := make([]string, 0, len(content.Attachments)) + if setting.Attachment.Enabled { + for _, attachment := range content.Attachments { + a, err := attachment_service.UploadAttachment(ctx, bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, int64(len(attachment.Content)), &repo_model.Attachment{ + Name: attachment.Name, + UploaderID: doer.ID, + RepoID: issue.Repo.ID, + }) + if err != nil { + if upload.IsErrFileTypeForbidden(err) { + log.Info("Skipping disallowed attachment type: %s", attachment.Name) + continue + } + return err + } + attachmentIDs = append(attachmentIDs, a.UUID) + } + } + + if content.Content == "" && len(attachmentIDs) == 0 { + return nil + } + switch r := ref.(type) { case *issues_model.Issue: - attachmentIDs := make([]string, 0, len(content.Attachments)) - if setting.Attachment.Enabled { - for _, attachment := range content.Attachments { - a, err := attachment_service.UploadAttachment(ctx, bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, int64(len(attachment.Content)), &repo_model.Attachment{ - Name: attachment.Name, - UploaderID: doer.ID, - RepoID: issue.Repo.ID, - }) - if err != nil { - if upload.IsErrFileTypeForbidden(err) { - log.Info("Skipping disallowed attachment type: %s", attachment.Name) - continue - } - return err - } - attachmentIDs = append(attachmentIDs, a.UUID) - } - } - - if content.Content == "" && len(attachmentIDs) == 0 { - return nil - } - - _, err = issue_service.CreateIssueComment(ctx, doer, issue.Repo, issue, content.Content, attachmentIDs) + _, err := issue_service.CreateIssueComment(ctx, doer, issue.Repo, issue, content.Content, attachmentIDs) if err != nil { return fmt.Errorf("CreateIssueComment failed: %w", err) } case *issues_model.Comment: comment := r - if content.Content == "" { - return nil - } - - if comment.Type == issues_model.CommentTypeCode { + switch comment.Type { + case issues_model.CommentTypeCode: _, err := pull_service.CreateCodeComment( ctx, doer, @@ -130,11 +127,16 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u false, // not pending review but a single review comment.ReviewID, "", - nil, + attachmentIDs, ) if err != nil { return fmt.Errorf("CreateCodeComment failed: %w", err) } + default: + _, err := issue_service.CreateIssueComment(ctx, doer, issue.Repo, issue, content.Content, attachmentIDs) + if err != nil { + return fmt.Errorf("CreateIssueComment failed: %w", err) + } } } return nil diff --git a/tests/integration/incoming_email_test.go b/tests/integration/incoming_email_test.go index 1284833864..88571303ac 100644 --- a/tests/integration/incoming_email_test.go +++ b/tests/integration/incoming_email_test.go @@ -7,6 +7,7 @@ import ( "io" "net" "net/smtp" + "net/url" "strings" "testing" "time" @@ -26,187 +27,190 @@ import ( ) func TestIncomingEmail(t *testing.T) { - defer tests.PrepareTestEnv(t)() + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + t.Run("Payload", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - t.Run("Payload", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 1}) - comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 1}) + _, err := incoming_payload.CreateReferencePayload(user) + assert.Error(t, err) - _, err := incoming_payload.CreateReferencePayload(user) - assert.Error(t, err) + issuePayload, err := incoming_payload.CreateReferencePayload(issue) + assert.NoError(t, err) + commentPayload, err := incoming_payload.CreateReferencePayload(comment) + assert.NoError(t, err) - issuePayload, err := incoming_payload.CreateReferencePayload(issue) - assert.NoError(t, err) - commentPayload, err := incoming_payload.CreateReferencePayload(comment) - assert.NoError(t, err) + _, err = incoming_payload.GetReferenceFromPayload(db.DefaultContext, []byte{1, 2, 3}) + assert.Error(t, err) - _, err = incoming_payload.GetReferenceFromPayload(db.DefaultContext, []byte{1, 2, 3}) - assert.Error(t, err) + ref, err := incoming_payload.GetReferenceFromPayload(db.DefaultContext, issuePayload) + assert.NoError(t, err) + assert.IsType(t, ref, new(issues_model.Issue)) + assert.EqualValues(t, issue.ID, ref.(*issues_model.Issue).ID) - ref, err := incoming_payload.GetReferenceFromPayload(db.DefaultContext, issuePayload) - assert.NoError(t, err) - assert.IsType(t, ref, new(issues_model.Issue)) - assert.EqualValues(t, issue.ID, ref.(*issues_model.Issue).ID) + ref, err = incoming_payload.GetReferenceFromPayload(db.DefaultContext, commentPayload) + assert.NoError(t, err) + assert.IsType(t, ref, new(issues_model.Comment)) + assert.EqualValues(t, comment.ID, ref.(*issues_model.Comment).ID) + }) - ref, err = incoming_payload.GetReferenceFromPayload(db.DefaultContext, commentPayload) - assert.NoError(t, err) - assert.IsType(t, ref, new(issues_model.Comment)) - assert.EqualValues(t, comment.ID, ref.(*issues_model.Comment).ID) - }) + t.Run("Token", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - t.Run("Token", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + payload := []byte{1, 2, 3, 4, 5} - payload := []byte{1, 2, 3, 4, 5} + token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) + assert.NoError(t, err) + assert.NotEmpty(t, token) - token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) - assert.NoError(t, err) - assert.NotEmpty(t, token) + ht, u, p, err := token_service.ExtractToken(db.DefaultContext, token) + assert.NoError(t, err) + assert.Equal(t, token_service.ReplyHandlerType, ht) + assert.Equal(t, user.ID, u.ID) + assert.Equal(t, payload, p) + }) - ht, u, p, err := token_service.ExtractToken(db.DefaultContext, token) - assert.NoError(t, err) - assert.Equal(t, token_service.ReplyHandlerType, ht) - assert.Equal(t, user.ID, u.ID) - assert.Equal(t, payload, p) - }) + t.Run("Handler", func(t *testing.T) { + t.Run("Reply", func(t *testing.T) { + t.Run("Comment", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - t.Run("Handler", func(t *testing.T) { - t.Run("Reply", func(t *testing.T) { - t.Run("Comment", func(t *testing.T) { + handler := &incoming.ReplyHandler{} + + payload, err := incoming_payload.CreateReferencePayload(issue) + assert.NoError(t, err) + + assert.Error(t, handler.Handle(db.DefaultContext, &incoming.MailContent{}, nil, payload)) + assert.NoError(t, handler.Handle(db.DefaultContext, &incoming.MailContent{}, user, payload)) + + content := &incoming.MailContent{ + Content: "reply by mail", + Attachments: []*incoming.Attachment{ + { + Name: "attachment.txt", + Content: []byte("test"), + }, + }, + } + + assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) + + comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ + IssueID: issue.ID, + Type: issues_model.CommentTypeComment, + }) + assert.NoError(t, err) + assert.NotEmpty(t, comments) + comment := comments[len(comments)-1] + assert.Equal(t, user.ID, comment.PosterID) + assert.Equal(t, content.Content, comment.Content) + assert.NoError(t, comment.LoadAttachments(db.DefaultContext)) + assert.Len(t, comment.Attachments, 1) + attachment := comment.Attachments[0] + assert.Equal(t, content.Attachments[0].Name, attachment.Name) + assert.EqualValues(t, 4, attachment.Size) + }) + + t.Run("CodeComment", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 6}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID}) + + handler := &incoming.ReplyHandler{} + content := &incoming.MailContent{ + Content: "code reply by mail", + Attachments: []*incoming.Attachment{ + { + Name: "attachment.txt", + Content: []byte("test"), + }, + }, + } + + payload, err := incoming_payload.CreateReferencePayload(comment) + assert.NoError(t, err) + + assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) + + comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCode, + }) + assert.NoError(t, err) + assert.NotEmpty(t, comments) + comment = comments[len(comments)-1] + assert.Equal(t, user.ID, comment.PosterID) + assert.Equal(t, content.Content, comment.Content) + assert.NoError(t, comment.LoadAttachments(db.DefaultContext)) + assert.Len(t, comment.Attachments, 1) + attachment := comment.Attachments[0] + assert.Equal(t, content.Attachments[0].Name, attachment.Name) + assert.EqualValues(t, 4, attachment.Size) + }) + }) + + t.Run("Unsubscribe", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - handler := &incoming.ReplyHandler{} + watching, err := issues_model.CheckIssueWatch(db.DefaultContext, user, issue) + assert.NoError(t, err) + assert.True(t, watching) + + handler := &incoming.UnsubscribeHandler{} + + content := &incoming.MailContent{ + Content: "unsub me", + } payload, err := incoming_payload.CreateReferencePayload(issue) assert.NoError(t, err) - assert.Error(t, handler.Handle(db.DefaultContext, &incoming.MailContent{}, nil, payload)) - assert.NoError(t, handler.Handle(db.DefaultContext, &incoming.MailContent{}, user, payload)) - - content := &incoming.MailContent{ - Content: "reply by mail", - Attachments: []*incoming.Attachment{ - { - Name: "attachment.txt", - Content: []byte("test"), - }, - }, - } - assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) - comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ - IssueID: issue.ID, - Type: issues_model.CommentTypeComment, - }) + watching, err = issues_model.CheckIssueWatch(db.DefaultContext, user, issue) assert.NoError(t, err) - assert.NotEmpty(t, comments) - comment := comments[len(comments)-1] - assert.Equal(t, user.ID, comment.PosterID) - assert.Equal(t, content.Content, comment.Content) - assert.NoError(t, comment.LoadAttachments(db.DefaultContext)) - assert.Len(t, comment.Attachments, 1) - attachment := comment.Attachments[0] - assert.Equal(t, content.Attachments[0].Name, attachment.Name) - assert.EqualValues(t, 4, attachment.Size) + assert.False(t, watching) }) + }) - t.Run("CodeComment", func(t *testing.T) { + if setting.IncomingEmail.Enabled { + // This test connects to the configured email server and is currently only enabled for MySql integration tests. + // It sends a reply to create a comment. If the comment is not detected after 10 seconds the test fails. + t.Run("IMAP", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 6}) - issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID}) - - handler := &incoming.ReplyHandler{} - content := &incoming.MailContent{ - Content: "code reply by mail", - Attachments: []*incoming.Attachment{ - { - Name: "attachment.txt", - Content: []byte("test"), - }, - }, - } - - payload, err := incoming_payload.CreateReferencePayload(comment) + payload, err := incoming_payload.CreateReferencePayload(issue) + assert.NoError(t, err) + token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) assert.NoError(t, err) - assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) - - comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ - IssueID: issue.ID, - Type: issues_model.CommentTypeCode, - }) + msg := gomail.NewMessage() + msg.SetHeader("To", strings.Replace(setting.IncomingEmail.ReplyToAddress, setting.IncomingEmail.TokenPlaceholder, token, 1)) + msg.SetHeader("From", user.Email) + msg.SetBody("text/plain", token) + err = gomail.Send(&smtpTestSender{}, msg) assert.NoError(t, err) - assert.NotEmpty(t, comments) - comment = comments[len(comments)-1] - assert.Equal(t, user.ID, comment.PosterID) - assert.Equal(t, content.Content, comment.Content) - assert.NoError(t, comment.LoadAttachments(db.DefaultContext)) - assert.Empty(t, comment.Attachments) + + assert.Eventually(t, func() bool { + comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ + IssueID: issue.ID, + Type: issues_model.CommentTypeComment, + }) + assert.NoError(t, err) + assert.NotEmpty(t, comments) + + comment := comments[len(comments)-1] + + return comment.PosterID == user.ID && comment.Content == token + }, 10*time.Second, 1*time.Second) }) - }) - - t.Run("Unsubscribe", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - - watching, err := issues_model.CheckIssueWatch(db.DefaultContext, user, issue) - assert.NoError(t, err) - assert.True(t, watching) - - handler := &incoming.UnsubscribeHandler{} - - content := &incoming.MailContent{ - Content: "unsub me", - } - - payload, err := incoming_payload.CreateReferencePayload(issue) - assert.NoError(t, err) - - assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) - - watching, err = issues_model.CheckIssueWatch(db.DefaultContext, user, issue) - assert.NoError(t, err) - assert.False(t, watching) - }) + } }) - - if setting.IncomingEmail.Enabled { - // This test connects to the configured email server and is currently only enabled for MySql integration tests. - // It sends a reply to create a comment. If the comment is not detected after 10 seconds the test fails. - t.Run("IMAP", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - - payload, err := incoming_payload.CreateReferencePayload(issue) - assert.NoError(t, err) - token, err := token_service.CreateToken(token_service.ReplyHandlerType, user, payload) - assert.NoError(t, err) - - msg := gomail.NewMessage() - msg.SetHeader("To", strings.Replace(setting.IncomingEmail.ReplyToAddress, setting.IncomingEmail.TokenPlaceholder, token, 1)) - msg.SetHeader("From", user.Email) - msg.SetBody("text/plain", token) - err = gomail.Send(&smtpTestSender{}, msg) - assert.NoError(t, err) - - assert.Eventually(t, func() bool { - comments, err := issues_model.FindComments(db.DefaultContext, &issues_model.FindCommentsOptions{ - IssueID: issue.ID, - Type: issues_model.CommentTypeComment, - }) - assert.NoError(t, err) - assert.NotEmpty(t, comments) - - comment := comments[len(comments)-1] - - return comment.PosterID == user.ID && comment.Content == token - }, 10*time.Second, 1*time.Second) - }) - } } // A simple SMTP mail sender used for integration tests.