From 1f5d55907a05fefbda17f7a4f45f58f2b522aee2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 24 May 2016 01:19:20 +0800 Subject: 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, "") } -- cgit v1.2.1