From 1f5d55907a05fefbda17f7a4f45f58f2b522aee2 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 24 May 2016 01:19:20 +0800
Subject: [PATCH] Merge the places where exceptions could be raised
---
app/workers/email_receiver_worker.rb | 2 ++
lib/gitlab/email/handler/create_issue_handler.rb | 3 ++-
lib/gitlab/email/handler/create_note_handler.rb | 3 ++-
lib/gitlab/email/receiver.rb | 6 ++----
spec/lib/gitlab/email/receiver_spec.rb | 8 ++++++++
5 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb
index af9006d8dde..68081643113 100644
--- a/app/workers/email_receiver_worker.rb
+++ b/app/workers/email_receiver_worker.rb
@@ -24,6 +24,8 @@ class EmailReceiverWorker
reason = nil
case e
+ when Gitlab::Email::UnknownIncomingEmail
+ reason = "We couldn't figure out what the email is for."
when Gitlab::Email::SentNotificationNotFoundError
reason = "We couldn't figure out what the email is in reply to. Please create your comment through the web interface."
when Gitlab::Email::ProjectNotFound
diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb
index 7e3506bbd3f..bfc56cb17ff 100644
--- a/lib/gitlab/email/handler/create_issue_handler.rb
+++ b/lib/gitlab/email/handler/create_issue_handler.rb
@@ -14,10 +14,11 @@ module Gitlab
end
def can_handle?
- !!(project_namespace && project)
+ !!authentication_token
end
def execute
+ raise ProjectNotFound unless project
validate_permission!(:create_issue)
verify_record!(
diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb
index b91dd623161..2ae3bf23a74 100644
--- a/lib/gitlab/email/handler/create_note_handler.rb
+++ b/lib/gitlab/email/handler/create_note_handler.rb
@@ -6,7 +6,8 @@ module Gitlab
module Handler
class CreateNoteHandler < BaseHandler
def can_handle?
- !!(mail_key && sent_notification)
+ # We want to raise SentNotificationNotFoundError for missing key
+ !!(mail_key.nil? || mail_key =~ /\A\w+\z/)
end
def execute
diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index a497d09b0a8..fef9ee8402b 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -16,6 +16,7 @@ module Gitlab
class NoteableNotFoundError < ProcessingError; end
class InvalidNoteError < ProcessingError; end
class InvalidIssueError < ProcessingError; end
+ class UnknownIncomingEmail < ProcessingError; end
class Receiver
def initialize(raw)
@@ -30,11 +31,8 @@ module Gitlab
if handler = Handler.for(mail, mail_key)
handler.execute
- elsif mail_key =~ %r{/|\+}
- # Sent Notification mail_key would not have / or +
- raise ProjectNotFound
else
- raise SentNotificationNotFoundError
+ raise UnknownIncomingEmail
end
end
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index a9e2be0ad47..7291e478d90 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -30,6 +30,14 @@ describe Gitlab::Email::Receiver, lib: true do
)
end
+ context "when we cannot find a capable handler" do
+ let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") }
+
+ it "raises a UnknownIncomingEmail" do
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
+ end
+ end
+
context "when the recipient address doesn't include a mail key" do
let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") }
--
GitLab