From 7b95ebc9e44b7dba1e4aa32b773a124f75de9474 Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Sat, 6 Oct 2018 22:15:36 -0400
Subject: [PATCH] Terminate processing if inbound email is marked as delivered
 in callback

---
 lib/action_mailbox/base.rb          |  7 ++++++-
 lib/action_mailbox/callbacks.rb     | 12 +++---------
 test/unit/mailbox/callbacks_test.rb | 25 +++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/lib/action_mailbox/base.rb b/lib/action_mailbox/base.rb
index 66a3b7fd32..3b12493662 100644
--- a/lib/action_mailbox/base.rb
+++ b/lib/action_mailbox/base.rb
@@ -8,7 +8,7 @@ class ActionMailbox::Base
   include ActionMailbox::Callbacks, ActionMailbox::Routing
 
   attr_reader :inbound_email
-  delegate :mail, :bounced!, to: :inbound_email
+  delegate :mail, :delivered!, :bounced!, to: :inbound_email
 
   delegate :logger, to: ActionMailbox
 
@@ -35,6 +35,11 @@ class ActionMailbox::Base
     # Overwrite in subclasses
   end
 
+  def finished_processing?
+    inbound_email.delivered? || inbound_email.bounced?
+  end
+
+
   def bounce_with(message)
     inbound_email.bounced!
     message.deliver_later
diff --git a/lib/action_mailbox/callbacks.rb b/lib/action_mailbox/callbacks.rb
index a5f7aa7e6f..5f39faa01e 100644
--- a/lib/action_mailbox/callbacks.rb
+++ b/lib/action_mailbox/callbacks.rb
@@ -5,15 +5,9 @@ module ActionMailbox
     extend  ActiveSupport::Concern
     include ActiveSupport::Callbacks
 
-    TERMINATOR = ->(target, chain) do
-      terminate = true
-
-      catch(:abort) do
-        chain.call
-        terminate = target.inbound_email.bounced?
-      end
-
-      terminate
+    TERMINATOR = ->(mailbox, chain) do
+      chain.call
+      mailbox.finished_processing?
     end
 
     included do
diff --git a/test/unit/mailbox/callbacks_test.rb b/test/unit/mailbox/callbacks_test.rb
index 617e8a89b1..279bf7e574 100644
--- a/test/unit/mailbox/callbacks_test.rb
+++ b/test/unit/mailbox/callbacks_test.rb
@@ -27,6 +27,23 @@ class BouncingCallbackMailbox < ActionMailbox::Base
   end
 end
 
+class DiscardingCallbackMailbox < ActionMailbox::Base
+  before_processing { $before_processing = [ "Pre-discard" ] }
+
+  before_processing do
+    delivered!
+    $before_processing << "Discard"
+  end
+
+  before_processing { $before_processing << "Post-discard" }
+
+  after_processing { $after_processing = true }
+
+  def process
+    $processed = true
+  end
+end
+
 class ActionMailbox::Base::CallbacksTest < ActiveSupport::TestCase
   setup do
     $before_processing = $after_processing = $around_processing = $processed = false
@@ -47,4 +64,12 @@ class ActionMailbox::Base::CallbacksTest < ActiveSupport::TestCase
     assert_not $processed
     assert_not $after_processing
   end
+
+  test "marking the inbound email as delivered in a callback terminates processing" do
+    DiscardingCallbackMailbox.receive @inbound_email
+    assert @inbound_email.delivered?
+    assert_equal [ "Pre-discard", "Discard" ], $before_processing
+    assert_not $processed
+    assert_not $after_processing
+  end
 end