Merge branch 'master' into always-inherit-from-application-record

15 files changed, 226 insertions, 161 deletions

diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md
index 76d99f31c7..3b9f503a0b 100644
--- a/actionmailer/CHANGELOG.md
+++ b/actionmailer/CHANGELOG.md
@@ -1,134 +1,6 @@
-*   Disallow calling `#deliver_later` after making local modifications to
-    the message which would be lost when the delivery job is enqueued.
-
-    Prevents a common, hard-to-find bug like:
-
-        message = Notifier.welcome(user, foo)
-        message.message_id = my_generated_message_id
-        message.deliver_later
-
-    The message_id is silently lost! *Only the mailer arguments are passed
-    to the delivery job.*
-
-    This raises an exception now. Make modifications to the message within
-    the mailer method instead, or use a custom Active Job to manage delivery
-    instead of using #deliver_later.
+*   Exception handling: use `rescue_from` to handle exceptions raised by
+    mailer actions, by message delivery, and by deferred delivery jobs.
 
     *Jeremy Daer*
 
-*   Removes `-t` from default Sendmail arguments to match the underlying
-    `Mail::Sendmail` setting.
-
-    *Clayton Liggitt*
-
-
-## Rails 5.0.0.beta3 (February 24, 2016) ##
-
-*   Add support for fragment caching in Action Mailer views.
-
-    *Stan Lo*
-
-*   Reset `ActionMailer::Base.deliveries` after every test in
-    `ActionDispatch::IntegrationTest`.
-
-    *Yves Senn*
-
-
-## Rails 5.0.0.beta2 (February 01, 2016) ##
-
-*   No changes.
-
-
-## Rails 5.0.0.beta1 (December 18, 2015) ##
-
-*   `config.action_mailer.default_url_options[:protocol]` is now set to `https` if `config.force_ssl` is set to `true`.
-
-    *Andrew Kampjes*
-
-*   Add `config.action_mailer.deliver_later_queue_name` configuration to set the
-    mailer queue name.
-
-    *Chris McGrath*
-
-*   `assert_emails` in block form, uses the given number as expected value.
-    This makes the error message much easier to understand.
-
-    *Yuji Yaginuma*
-
-*   Add support for inline images in mailer previews by using an interceptor
-    class to convert cid: urls in image src attributes to data urls.
-
-    *Andrew White*
-
-*   Mailer preview now uses `url_for` to fix links to emails for apps running on
-    a subdirectory.
-
-    *Remo Mueller*
-
-*   Mailer previews no longer crash when the `mail` method wasn't called
-    (`NullMail`).
-
-    Fixes #19849.
-
-    *Yves Senn*
-
-*   Make sure labels and values line up in mailer previews.
-
-    *Yves Senn*
-
-*   Add `assert_enqueued_emails` and `assert_no_enqueued_emails`.
-
-    Example:
-
-        def test_emails
-          assert_enqueued_emails 2 do
-            ContactMailer.welcome.deliver_later
-            ContactMailer.welcome.deliver_later
-          end
-        end
-
-        def test_no_emails
-          assert_no_enqueued_emails do
-            # No emails enqueued here
-          end
-        end
-
-    *George Claghorn*
-
-*   Add `_mailer` suffix to mailers created via generator, following the same
-    naming convention used in controllers and jobs.
-
-    *Carlos Souza*
-
-*   Remove deprecated `*_path` helpers in email views.
-
-    *Rafael Mendonça França*
-
-*   Remove deprecated `deliver` and `deliver!` methods.
-
-    *claudiob*
-
-*   Template lookup now respects default locale and I18n fallbacks.
-
-    Given the following templates:
-
-        mailer/demo.html.erb
-        mailer/demo.en.html.erb
-        mailer/demo.pt.html.erb
-
-    Before this change, for a locale that doesn't have its associated file, the
-    `mailer/demo.html.erb` would be rendered even if `en` was the default locale.
-
-    Now `mailer/demo.en.html.erb` has precedence over the file without locale.
-
-    Also, it is possible to give a fallback.
-
-        mailer/demo.pt.html.erb
-        mailer/demo.pt-BR.html.erb
-
-    So if the locale is `pt-PT`, `mailer/demo.pt.html.erb` will be rendered given
-    the right I18n fallback configuration.
-
-    *Rafael Mendonça França*
-
-Please check [4-2-stable](https://github.com/rails/rails/blob/4-2-stable/actionmailer/CHANGELOG.md) for previous changes.
+Please check [5-0-stable](https://github.com/rails/rails/blob/5-0-stable/actionmailer/CHANGELOG.md) for previous changes.
diff --git a/actionmailer/Rakefile b/actionmailer/Rakefile
index 54e6cff48d..416c9ee33a 100644
--- a/actionmailer/Rakefile
+++ b/actionmailer/Rakefile
@@ -4,7 +4,6 @@ desc "Default Task"
 task default: [ :test ]
 
 task :package
-task "package:clean"
 
 # Run the unit tests
 Rake::TestTask.new { |t|
diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec
index fa6043fdd7..25e3bcb2e9 100644
--- a/actionmailer/actionmailer.gemspec
+++ b/actionmailer/actionmailer.gemspec
@@ -24,5 +24,5 @@ Gem::Specification.new do |s|
   s.add_dependency 'activejob', version
 
   s.add_dependency 'mail', ['~> 2.5', '>= 2.5.4']
-  s.add_dependency 'rails-dom-testing', '~> 1.0', '>= 1.0.5'
+  s.add_dependency 'rails-dom-testing', '~> 2.0'
 end
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 4bb7842297..ea5af9e4f2 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -5,6 +5,7 @@ require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/module/anonymous'
 
 require 'action_mailer/log_subscriber'
+require 'action_mailer/rescuable'
 
 module ActionMailer
   # Action Mailer allows you to send email from your application using a mailer model and views.
@@ -392,6 +393,7 @@ module ActionMailer
   #     of an OpenSSL verify constant (<tt>'none'</tt>, <tt>'peer'</tt>, <tt>'client_once'</tt>,
   #     <tt>'fail_if_no_peer_cert'</tt>) or directly the constant (<tt>OpenSSL::SSL::VERIFY_NONE</tt>,
   #     <tt>OpenSSL::SSL::VERIFY_PEER</tt>, ...).
+  #     <tt>:ssl/:tls</tt> Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection)
   #
   # * <tt>sendmail_settings</tt> - Allows you to override options for the <tt>:sendmail</tt> delivery method.
   #   * <tt>:location</tt> - The location of the sendmail executable. Defaults to <tt>/usr/sbin/sendmail</tt>.
@@ -419,6 +421,7 @@ module ActionMailer
   # * <tt>deliver_later_queue_name</tt> - The name of the queue used with <tt>deliver_later</tt>.
   class Base < AbstractController::Base
     include DeliveryMethods
+    include Rescuable
     include Previews
 
     abstract!
diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb
index 52772af2d3..d371c1b61a 100644
--- a/actionmailer/lib/action_mailer/delivery_job.rb
+++ b/actionmailer/lib/action_mailer/delivery_job.rb
@@ -3,11 +3,32 @@ require 'active_job'
 module ActionMailer
   # The <tt>ActionMailer::DeliveryJob</tt> class is used when you
   # want to send emails outside of the request-response cycle.
+  #
+  # Exceptions are rescued and handled by the mailer class.
   class DeliveryJob < ActiveJob::Base # :nodoc:
     queue_as { ActionMailer::Base.deliver_later_queue_name }
 
+    rescue_from StandardError, with: :handle_exception_with_mailer_class
+
     def perform(mailer, mail_method, delivery_method, *args) #:nodoc:
       mailer.constantize.public_send(mail_method, *args).send(delivery_method)
     end
+
+    private
+      # "Deserialize" the mailer class name by hand in case another argument
+      # (like a Global ID reference) raised DeserializationError.
+      def mailer_class
+        if mailer = Array(@serialized_arguments).first || Array(arguments).first
+          mailer.constantize
+        end
+      end
+
+      def handle_exception_with_mailer_class(exception)
+        if klass = mailer_class
+          klass.handle_exception exception
+        else
+          raise exception
+        end
+      end
   end
 end
diff --git a/actionmailer/lib/action_mailer/gem_version.rb b/actionmailer/lib/action_mailer/gem_version.rb
index cbe5fc3e64..7dafceef2b 100644
--- a/actionmailer/lib/action_mailer/gem_version.rb
+++ b/actionmailer/lib/action_mailer/gem_version.rb
@@ -6,9 +6,9 @@ module ActionMailer
 
   module VERSION
     MAJOR = 5
-    MINOR = 0
+    MINOR = 1
     TINY  = 0
-    PRE   = "beta3"
+    PRE   = "alpha"
 
     STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
   end
diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb
index d638057d72..c5ba5f9f1d 100644
--- a/actionmailer/lib/action_mailer/message_delivery.rb
+++ b/actionmailer/lib/action_mailer/message_delivery.rb
@@ -1,7 +1,6 @@
 require 'delegate'
 
 module ActionMailer
-
   # The <tt>ActionMailer::MessageDelivery</tt> class is used by
   # <tt>ActionMailer::Base</tt> when creating a new mailer.
   # <tt>MessageDelivery</tt> is a wrapper (+Delegator+ subclass) around a lazy
@@ -14,30 +13,35 @@ module ActionMailer
   #   Notifier.welcome(User.first).deliver_later # enqueue email delivery as a job through Active Job
   #   Notifier.welcome(User.first).message       # a Mail::Message object
   class MessageDelivery < Delegator
-    def initialize(mailer, mail_method, *args) #:nodoc:
-      @mailer = mailer
-      @mail_method = mail_method
-      @args = args
-      @obj = nil
+    def initialize(mailer_class, action, *args) #:nodoc:
+      @mailer_class, @action, @args = mailer_class, action, args
+
+      # The mail is only processed if we try to call any methods on it.
+      # Typical usage will leave it unloaded and call deliver_later.
+      @processed_mailer = nil
+      @mail_message = nil
     end
 
+    # Method calls are delegated to the Mail::Message that's ready to deliver.
     def __getobj__ #:nodoc:
-      @obj ||= begin
-                 mailer = @mailer.new
-                 mailer.process @mail_method, *@args
-                 mailer.message
-               end
+      @mail_message ||= processed_mailer.message
     end
 
-    def __setobj__(obj) #:nodoc:
-      @obj = obj
+    # Unused except for delegator internals (dup, marshaling).
+    def __setobj__(mail_message) #:nodoc:
+      @mail_message = mail_message
     end
 
-    # Returns the Mail::Message object
+    # Returns the resulting Mail::Message
     def message
       __getobj__
     end
 
+    # Was the delegate loaded, causing the mailer action to be processed?
+    def processed?
+      @processed_mailer || @mail_message
+    end
+
     # Enqueues the email to be delivered through Active Job. When the
     # job runs it will send the email using +deliver_now!+. That means
     # that the message will be sent bypassing checking +perform_deliveries+
@@ -78,7 +82,9 @@ module ActionMailer
     #   Notifier.welcome(User.first).deliver_now!
     #
     def deliver_now!
-      message.deliver!
+      processed_mailer.handle_exceptions do
+        message.deliver!
+      end
     end
 
     # Delivers an email:
@@ -86,24 +92,33 @@ module ActionMailer
     #   Notifier.welcome(User.first).deliver_now
     #
     def deliver_now
-      message.deliver
+      processed_mailer.handle_exceptions do
+        message.deliver
+      end
     end
 
     private
+      # Returns the processed Mailer instance. We keep this instance
+      # on hand so we can delegate exception handling to it.
+      def processed_mailer
+        @processed_mailer ||= @mailer_class.new.tap do |mailer|
+          mailer.process @action, *@args
+        end
+      end
 
       def enqueue_delivery(delivery_method, options={})
-        if @obj
-          raise "You've accessed the message before asking to deliver it " \
-            "later, so you may have made local changes that would be " \
-            "silently lost if we enqueued a job to deliver it. Why? Only " \
+        if processed?
+          ::Kernel.raise "You've accessed the message before asking to " \
+            "deliver it later, so you may have made local changes that would " \
+            "be silently lost if we enqueued a job to deliver it. Why? Only " \
             "the mailer method *arguments* are passed with the delivery job! " \
             "Do not access the message in any way if you mean to deliver it " \
             "later. Workarounds: 1. don't touch the message before calling " \
             "#deliver_later, 2. only touch the message *within your mailer " \
             "method*, or 3. use a custom Active Job instead of #deliver_later."
         else
-          args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args
-          ActionMailer::DeliveryJob.set(options).perform_later(*args)
+          args = @mailer_class.name, @action.to_s, delivery_method.to_s, *@args
+          ::ActionMailer::DeliveryJob.set(options).perform_later(*args)
         end
       end
   end
diff --git a/actionmailer/lib/action_mailer/rescuable.rb b/actionmailer/lib/action_mailer/rescuable.rb
new file mode 100644
index 0000000000..f2eabfa057
--- /dev/null
+++ b/actionmailer/lib/action_mailer/rescuable.rb
@@ -0,0 +1,27 @@
+module ActionMailer #:nodoc:
+  # Provides `rescue_from` for mailers. Wraps mailer action processing,
+  # mail job processing, and mail delivery.
+  module Rescuable
+    extend ActiveSupport::Concern
+    include ActiveSupport::Rescuable
+
+    class_methods do
+      def handle_exception(exception) #:nodoc:
+        rescue_with_handler(exception) || raise(exception)
+      end
+    end
+
+    def handle_exceptions #:nodoc:
+      yield
+    rescue => exception
+      rescue_with_handler(exception) || raise
+    end
+
+    private
+      def process(*)
+        handle_exceptions do
+          super
+        end
+      end
+  end
+end
diff --git a/actionmailer/test/caching_test.rb b/actionmailer/test/caching_test.rb
index b4344eb167..22e1bdb5f1 100644
--- a/actionmailer/test/caching_test.rb
+++ b/actionmailer/test/caching_test.rb
@@ -125,15 +125,16 @@ class FunctionalFragmentCachingTest < BaseCachingTest
     expected_body = "\"Welcome\""
 
     assert_match expected_body, email.body.encoded
-    assert_match "\"Welcome\"",
+    assert_match expected_body,
       @store.read("views/caching/#{template_digest("caching_mailer/fragment_cache")}")
   end
 
   def test_fragment_caching_in_partials
     email = @mailer.fragment_cache_in_partials
-    assert_match(/Old fragment caching in a partial/, email.body.encoded)
+    expected_body = 'Old fragment caching in a partial'
+    assert_match(expected_body, email.body.encoded)
 
-    assert_match("Old fragment caching in a partial",
+    assert_match(expected_body,
       @store.read("views/caching/#{template_digest("caching_mailer/_partial")}"))
   end
 
@@ -145,6 +146,47 @@ class FunctionalFragmentCachingTest < BaseCachingTest
     assert_match expected_body, @store.read("views/no_digest")
   end
 
+  def test_fragment_caching_options
+    time = Time.now
+    email = @mailer.fragment_caching_options
+    expected_body = "No Digest"
+
+    assert_match expected_body, email.body.encoded
+    Time.stub(:now, time + 11) do
+      assert_nil @store.read("views/no_digest")
+    end
+  end
+
+  def test_multipart_fragment_caching
+    email = @mailer.multipart_cache
+
+    expected_text_body = "\"Welcome text\""
+    expected_html_body = "\"Welcome html\""
+    encoded_body = email.body.encoded
+    assert_match expected_text_body, encoded_body
+    assert_match expected_html_body, encoded_body
+    assert_match expected_text_body,
+                 @store.read("views/text_caching")
+    assert_match expected_html_body,
+                 @store.read("views/html_caching")
+  end
+
+  def test_fragment_cache_instrumentation
+    payload = nil
+
+    subscriber = proc do |*args|
+      event = ActiveSupport::Notifications::Event.new(*args)
+      payload = event.payload
+    end
+
+    ActiveSupport::Notifications.subscribed(subscriber, "read_fragment.action_mailer") do
+      @mailer.fragment_cache
+    end
+
+    assert_equal "caching_mailer", payload[:mailer]
+    assert_equal "views/caching/#{template_digest("caching_mailer/fragment_cache")}", payload[:key]
+  end
+
   private
 
     def template_digest(name)
diff --git a/actionmailer/test/fixtures/caching_mailer/fragment_caching_options.html.erb b/actionmailer/test/fixtures/caching_mailer/fragment_caching_options.html.erb
new file mode 100644
index 0000000000..0541ac321b
--- /dev/null
+++ b/actionmailer/test/fixtures/caching_mailer/fragment_caching_options.html.erb
@@ -0,0 +1,3 @@
+<%= cache :no_digest, skip_digest: true, expires_in: 0 do %>
+    No Digest
+<% end %>
diff --git a/actionmailer/test/fixtures/caching_mailer/multipart_cache.html.erb b/actionmailer/test/fixtures/caching_mailer/multipart_cache.html.erb
new file mode 100644
index 0000000000..0d26baa2d7
--- /dev/null
+++ b/actionmailer/test/fixtures/caching_mailer/multipart_cache.html.erb
@@ -0,0 +1,3 @@
+<% cache :html_caching, skip_digest: true do %>
+    "Welcome html"
+<% end %>
diff --git a/actionmailer/test/fixtures/caching_mailer/multipart_cache.text.erb b/actionmailer/test/fixtures/caching_mailer/multipart_cache.text.erb
new file mode 100644
index 0000000000..ef97326e03
--- /dev/null
+++ b/actionmailer/test/fixtures/caching_mailer/multipart_cache.text.erb
@@ -0,0 +1,3 @@
+<% cache :text_caching, skip_digest: true do %>
+    "Welcome text"
+<% end %>
diff --git a/actionmailer/test/mailers/caching_mailer.rb b/actionmailer/test/mailers/caching_mailer.rb
index 345d267a36..92d3cff7c9 100644
--- a/actionmailer/test/mailers/caching_mailer.rb
+++ b/actionmailer/test/mailers/caching_mailer.rb
@@ -12,4 +12,12 @@ class CachingMailer < ActionMailer::Base
   def skip_fragment_cache_digesting
     mail(subject: "welcome", template_name: "skip_fragment_cache_digesting")
   end
+
+  def fragment_caching_options
+    mail(subject: "welcome", template_name: "fragment_caching_options")
+  end
+
+  def multipart_cache
+    mail(subject: "welcome", template_name: "multipart_cache")
+  end
 end
diff --git a/actionmailer/test/mailers/delayed_mailer.rb b/actionmailer/test/mailers/delayed_mailer.rb
index 62d4baa434..e6211ef028 100644
--- a/actionmailer/test/mailers/delayed_mailer.rb
+++ b/actionmailer/test/mailers/delayed_mailer.rb
@@ -1,6 +1,26 @@
+require 'active_job/arguments'
+
+class DelayedMailerError < StandardError; end
+
 class DelayedMailer < ActionMailer::Base
+  cattr_accessor :last_error
+  cattr_accessor :last_rescue_from_instance
+
+  rescue_from DelayedMailerError do |error|
+    @@last_error = error
+    @@last_rescue_from_instance = self
+  end
+
+  rescue_from ActiveJob::DeserializationError do |error|
+    @@last_error = error
+    @@last_rescue_from_instance = self
+  end
 
   def test_message(*)
     mail(from: 'test-sender@test.com', to: 'test-receiver@test.com', subject: 'Test Subject', body: 'Test Body')
   end
+
+  def test_raise(klass_name)
+    raise klass_name.constantize, 'boom'
+  end
 end
diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb
index f06d69369f..aaed94d519 100644
--- a/actionmailer/test/message_delivery_test.rb
+++ b/actionmailer/test/message_delivery_test.rb
@@ -12,16 +12,23 @@ class MessageDeliveryTest < ActiveSupport::TestCase
     ActionMailer::Base.deliver_later_queue_name = :test_queue
     ActionMailer::Base.delivery_method = :test
     ActiveJob::Base.logger = Logger.new(nil)
-    @mail = DelayedMailer.test_message(1, 2, 3)
     ActionMailer::Base.deliveries.clear
     ActiveJob::Base.queue_adapter.perform_enqueued_at_jobs = true
     ActiveJob::Base.queue_adapter.perform_enqueued_jobs = true
+
+    DelayedMailer.last_error = nil
+    DelayedMailer.last_rescue_from_instance = nil
+
+    @mail = DelayedMailer.test_message(1, 2, 3)
   end
 
   teardown do
     ActiveJob::Base.logger = @previous_logger
     ActionMailer::Base.delivery_method = @previous_delivery_method
     ActionMailer::Base.deliver_later_queue_name = @previous_deliver_later_queue_name
+
+    DelayedMailer.last_error = nil
+    DelayedMailer.last_rescue_from_instance = nil
   end
 
   test 'should have a message' do
@@ -101,4 +108,46 @@ class MessageDeliveryTest < ActiveSupport::TestCase
       @mail.deliver_later
     end
   end
+
+  test 'job delegates error handling to mailer' do
+    # Superclass not rescued by mailer's rescue_from RuntimeError
+    message = DelayedMailer.test_raise('StandardError')
+    assert_raise(StandardError) { message.deliver_later }
+    assert_nil DelayedMailer.last_error
+    assert_nil DelayedMailer.last_rescue_from_instance
+
+    # Rescued by mailer's rescue_from RuntimeError
+    message = DelayedMailer.test_raise('DelayedMailerError')
+    assert_nothing_raised { message.deliver_later }
+    assert_equal 'boom', DelayedMailer.last_error.message
+    assert_kind_of DelayedMailer, DelayedMailer.last_rescue_from_instance
+  end
+
+  class DeserializationErrorFixture
+    include GlobalID::Identification
+
+    def self.find(id)
+      raise 'boom, missing find'
+    end
+
+    attr_reader :id
+    def initialize(id = 1)
+      @id = id
+    end
+
+    def to_global_id(options = {})
+      super app: 'foo'
+    end
+  end
+
+  test 'job delegates deserialization errors to mailer class' do
+    # Inject an argument that can't be deserialized.
+    message = DelayedMailer.test_message(DeserializationErrorFixture.new)
+
+    # DeserializationError is raised, rescued, and delegated to the handler
+    # on the mailer class.
+    assert_nothing_raised { message.deliver_later }
+    assert_equal DelayedMailer, DelayedMailer.last_rescue_from_instance
+    assert_equal 'Error while trying to deserialize arguments: boom, missing find', DelayedMailer.last_error.message
+  end
 end