Added delivery_handler method to mail and implemented in ActionMailer to deliver inside of instrumentation

4 files changed, 43 insertions, 9 deletions

diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 552fd7ccb8..4e5e1bbb29 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -326,10 +326,19 @@ module ActionMailer #:nodoc:
         end
       end
 
-      # TODO The delivery should happen inside the instrument block
-      def delivered_email(mail) #:nodoc:
+      def deliver_mail(mail) #:nodoc:
         ActiveSupport::Notifications.instrument("action_mailer.deliver") do |payload|
           self.set_payload_for_mail(payload, mail)
+
+          if mail.perform_deliveries
+            begin
+              mail.deliver!
+            rescue Exception => e
+              raise e if mail.raise_delivery_errors
+            end
+            Mail.deliveries << mail
+          end
+
         end
       end
 
diff --git a/actionmailer/lib/action_mailer/delivery_methods.rb b/actionmailer/lib/action_mailer/delivery_methods.rb
index 21909d5d57..7a6b410932 100644
--- a/actionmailer/lib/action_mailer/delivery_methods.rb
+++ b/actionmailer/lib/action_mailer/delivery_methods.rb
@@ -63,7 +63,7 @@ module ActionMailer
 
       def wrap_delivery_behavior(mail, method=nil) #:nodoc:
         method ||= self.delivery_method
-        mail.register_for_delivery_notification(self)
+        mail.delivery_handler = self
 
         if method.is_a?(Symbol)
           if klass = delivery_methods[method.to_sym]
diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb
index d8616ee3be..81e1df3911 100644
--- a/actionmailer/test/base_test.rb
+++ b/actionmailer/test/base_test.rb
@@ -354,7 +354,12 @@ class BaseTest < ActiveSupport::TestCase
 
   test "calling deliver on the action should deliver the mail object" do
     BaseMailer.deliveries.clear
-    BaseMailer.expects(:delivered_email).once
+    BaseMailer.expects(:deliver_mail).once
+    BaseMailer.welcome.deliver
+  end
+
+  test "calling deliver on the action should increment the deliveries collection" do
+    BaseMailer.deliveries.clear
     BaseMailer.welcome.deliver
     assert_equal(1, BaseMailer.deliveries.length)
   end
diff --git a/actionmailer/test/delivery_methods_test.rb b/actionmailer/test/delivery_methods_test.rb
index 145f8ba30d..e70e8acd9b 100644
--- a/actionmailer/test/delivery_methods_test.rb
+++ b/actionmailer/test/delivery_methods_test.rb
@@ -94,9 +94,8 @@ class MailDeliveryTest < ActiveSupport::TestCase
 
   test "ActionMailer should be told when Mail gets delivered" do
     DeliveryMailer.deliveries.clear
-    DeliveryMailer.expects(:delivered_email).once
+    DeliveryMailer.expects(:deliver_mail).once
     DeliveryMailer.welcome.deliver
-    assert_equal(1, DeliveryMailer.deliveries.length)
   end
 
   test "delivery method can be customized per instance" do
@@ -124,7 +123,13 @@ class MailDeliveryTest < ActiveSupport::TestCase
   test "does not perform deliveries if requested" do
     DeliveryMailer.perform_deliveries = false
     DeliveryMailer.deliveries.clear
-    DeliveryMailer.expects(:delivered_email).never
+    Mail::Message.any_instance.expects(:deliver!).never
+    DeliveryMailer.welcome.deliver
+  end
+  
+  test "does not append the deliveries collection if told not to perform the delivery" do
+    DeliveryMailer.perform_deliveries = false
+    DeliveryMailer.deliveries.clear
     DeliveryMailer.welcome.deliver
     assert_equal(0, DeliveryMailer.deliveries.length)
   end
@@ -132,7 +137,14 @@ class MailDeliveryTest < ActiveSupport::TestCase
   test "raise errors on bogus deliveries" do
     DeliveryMailer.delivery_method = BogusDelivery
     DeliveryMailer.deliveries.clear
-    DeliveryMailer.expects(:delivered_email).never
+    assert_raise RuntimeError do
+      DeliveryMailer.welcome.deliver
+    end
+  end
+
+  test "does not increment the deliveries collection on error" do
+    DeliveryMailer.delivery_method = BogusDelivery
+    DeliveryMailer.deliveries.clear
     assert_raise RuntimeError do
       DeliveryMailer.welcome.deliver
     end
@@ -142,9 +154,17 @@ class MailDeliveryTest < ActiveSupport::TestCase
   test "does not raise errors on bogus deliveries if set" do
     DeliveryMailer.delivery_method = BogusDelivery
     DeliveryMailer.raise_delivery_errors = false
+    assert_nothing_raised do
+      DeliveryMailer.welcome.deliver
+    end
+  end
+  
+  test "increments the deliveries collection on bogus deliveries if set to ignore" do
+    DeliveryMailer.delivery_method = BogusDelivery
+    DeliveryMailer.raise_delivery_errors = false
     DeliveryMailer.deliveries.clear
-    DeliveryMailer.expects(:delivered_email).once
     DeliveryMailer.welcome.deliver
     assert_equal(1, DeliveryMailer.deliveries.length)
   end
+  
 end
\ No newline at end of file