From f68aab1945679f76a0e9a51069cdc4f41e11821d Mon Sep 17 00:00:00 2001
From: Riccardo Padovani <riccardo@rpadovani.com>
Date: Mon, 9 Apr 2018 09:39:03 +0000
Subject: [PATCH] Make email handler clearer

---
 doc/development/emails.md                     | 18 ++++++++++++++
 .../email/handler/create_issue_handler.rb     |  2 +-
 spec/lib/gitlab/email/handler_spec.rb         | 24 +++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/doc/development/emails.md b/doc/development/emails.md
index 4dbf064fd75..73cac82caf0 100644
--- a/doc/development/emails.md
+++ b/doc/development/emails.md
@@ -74,6 +74,24 @@ See the [Rails guides] for more info.
 
 1. Reply by email should now be working.
 
+## Email namespace
+
+If you need to implement a new feature which requires a new email handler, follow these rules:
+
+ - You must choose a namespace. The namespace cannot contain `/` or `+`, and cannot match `\h{16}`.
+ - If your feature is related to a project, you will append the namespace **after** the project path, separated by a `+`
+ - If you have different actions in the namespace, you add the actions **after** the namespace separated by a `+`. The action name cannot contain `/` or `+`, , and cannot match `\h{16}`.
+ - You will register your handlers in `lib/gitlab/email/handler.rb`
+
+Therefore, these are the only valid formats for an email handler:
+
+ - `path/to/project+namespace`
+ - `path/to/project+namespace+action`
+ - `namespace`
+ - `namespace+action`
+
+Please note that `path/to/project` is used in GitLab Premium as handler for the Service Desk feature.
+
 ---
 
 [Return to Development documentation](README.md)
diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb
index a616a80e8f5..05a60deb7d3 100644
--- a/lib/gitlab/email/handler/create_issue_handler.rb
+++ b/lib/gitlab/email/handler/create_issue_handler.rb
@@ -14,7 +14,7 @@ module Gitlab
         end
 
         def can_handle?
-          !incoming_email_token.nil?
+          !incoming_email_token.nil? && !incoming_email_token.include?("+") && !mail_key.include?(Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX)
         end
 
         def execute
diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb
index 650b01c4df4..386d73e6115 100644
--- a/spec/lib/gitlab/email/handler_spec.rb
+++ b/spec/lib/gitlab/email/handler_spec.rb
@@ -14,4 +14,28 @@ describe Gitlab::Email::Handler do
       expect(described_class.for('email', '')).to be_nil
     end
   end
+
+  describe 'regexps are set properly' do
+    let(:addresses) do
+      %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key path/to/project+merge-request+user_email_token path/to/project+user_email_token)
+    end
+
+    it 'picks each handler at least once' do
+      matched_handlers = addresses.map do |address|
+        described_class.for('email', address).class
+      end
+
+      expect(matched_handlers.uniq).to match_array(Gitlab::Email::Handler::HANDLERS)
+    end
+
+    it 'can pick exactly one handler for each address' do
+      addresses.each do |address|
+        matched_handlers = Gitlab::Email::Handler::HANDLERS.select do |handler|
+          handler.new('email', address).can_handle?
+        end
+
+        expect(matched_handlers.count).to eq(1), "#{address} matches #{matched_handlers.count} handlers: #{matched_handlers}"
+      end
+    end
+  end
 end