Revert "Remove deprecated `ActionMailer::DeliveryJob` and `ActionMailer::Parameterized::DeliveryJob`"

This reverts commit 0f9249c93f.

Reverted because this wasn't warning in custom jobs and therefore
applications may have not seen the deprecation. We'll need to fix the
deprecation to warn for custom jobs so that applications can migrate.

actionmailer/CHANGELOG.md

@@ -1,8 +1,3 @@
-*   Remove deprecated `ActionMailer::DeliveryJob` and `ActionMailer::Parameterized::DeliveryJob`
-    in favor of `ActionMailer::MailDeliveryJob`.
-
-    *Rafael Mendonça França*
-
 *   Remove deprecated `ActionMailer::Base.receive` in favor of [Action Mailbox](https://github.com/rails/rails/tree/master/actionmailbox).
 
     *Rafael Mendonça França*

actionmailer/lib/action_mailer.rb

@@ -51,6 +51,7 @@ module ActionMailer
   autoload :TestCase
   autoload :TestHelper
   autoload :MessageDelivery
+  autoload :DeliveryJob
   autoload :MailDeliveryJob
 
   def self.eager_load!

actionmailer/lib/action_mailer/base.rb

@@ -457,7 +457,7 @@ module ActionMailer
 
     helper ActionMailer::MailHelper
 
-    class_attribute :delivery_job, default: ::ActionMailer::MailDeliveryJob
+    class_attribute :delivery_job, default: ::ActionMailer::DeliveryJob
     class_attribute :default_params, default: {
       mime_version: "1.0",
       charset:      "UTF-8",

actionmailer/lib/action_mailer/delivery_job.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+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
+
+    before_perform do
+      ActiveSupport::Deprecation.warn <<~MSG.squish
+        Sending mail with DeliveryJob and Parameterized::DeliveryJob
+        is deprecated and will be removed in Rails 6.1.
+        Please use MailDeliveryJob instead.
+      MSG
+    end
+
+    def perform(mailer, mail_method, delivery_method, *args) #:nodoc:
+      mailer.constantize.public_send(mail_method, *args).send(delivery_method)
+    end
+    ruby2_keywords(:perform) if respond_to?(:ruby2_keywords, true)
+
+    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

actionmailer/lib/action_mailer/message_delivery.rb

@@ -60,7 +60,7 @@ module ActionMailer
     # * <tt>:wait_until</tt> - Enqueue the email to be delivered at (after) a specific date / time
     # * <tt>:queue</tt> - Enqueue the email on the specified queue
     #
-    # By default, the email will be enqueued using <tt>ActionMailer::MailDeliveryJob</tt>. Each
+    # By default, the email will be enqueued using <tt>ActionMailer::DeliveryJob</tt>. Each
     # <tt>ActionMailer::Base</tt> class can specify the job to use by setting the class variable
     # +delivery_job+.
     #
@@ -84,7 +84,7 @@ module ActionMailer
     # * <tt>:wait_until</tt> - Enqueue the email to be delivered at (after) a specific date / time.
     # * <tt>:queue</tt> - Enqueue the email on the specified queue.
     #
-    # By default, the email will be enqueued using <tt>ActionMailer::MailDeliveryJob</tt>. Each
+    # By default, the email will be enqueued using <tt>ActionMailer::DeliveryJob</tt>. Each
     # <tt>ActionMailer::Base</tt> class can specify the job to use by setting the class variable
     # +delivery_job+.
     #
@@ -136,8 +136,15 @@ module ActionMailer
             "#deliver_later, 2. only touch the message *within your mailer " \
             "method*, or 3. use a custom Active Job instead of #deliver_later."
         else
-          @mailer_class.delivery_job.set(options).perform_later(
-            @mailer_class.name, @action.to_s, delivery_method.to_s, args: @args)
+          job = @mailer_class.delivery_job
+
+          if job <= MailDeliveryJob
+            job.set(options).perform_later(
+              @mailer_class.name, @action.to_s, delivery_method.to_s, args: @args)
+          else
+            job.set(options).perform_later(
+              @mailer_class.name, @action.to_s, delivery_method.to_s, *@args)
+          end
         end
       end
   end

actionmailer/lib/action_mailer/parameterized.rb

@@ -122,6 +122,13 @@ module ActionMailer
         end
     end
 
+    class DeliveryJob < ActionMailer::DeliveryJob # :nodoc:
+      def perform(mailer, mail_method, delivery_method, params, *args)
+        mailer.constantize.with(params).public_send(mail_method, *args).send(delivery_method)
+      end
+      ruby2_keywords(:perform) if respond_to?(:ruby2_keywords, true)
+    end
+
     class MessageDelivery < ActionMailer::MessageDelivery # :nodoc:
       def initialize(mailer_class, action, params, *args)
         super(mailer_class, action, *args)
@@ -141,8 +148,23 @@ module ActionMailer
           if processed?
             super
           else
-            @mailer_class.delivery_job.set(options).perform_later(
-              @mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args)
+            job = delivery_job_class
+
+            if job <= MailDeliveryJob
+              job.set(options).perform_later(
+                @mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args)
+            else
+              job.set(options).perform_later(
+                @mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args)
+            end
+          end
+        end
+
+        def delivery_job_class
+          if @mailer_class.delivery_job <= MailDeliveryJob
+            @mailer_class.delivery_job
+          else
+            Parameterized::DeliveryJob
           end
         end
     end

actionmailer/lib/action_mailer/test_helper.rb

@@ -155,7 +155,8 @@ module ActionMailer
       def delivery_job_filter(job)
         job_class = job.is_a?(Hash) ? job.fetch(:job) : job.class
 
-        Base.descendants.map(&:delivery_job).include?(job_class)
+        Base.descendants.map(&:delivery_job).include?(job_class) ||
+          ActionMailer::Parameterized::DeliveryJob == job_class
       end
   end
 end

actionmailer/test/legacy_delivery_job_test.rb

@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require "abstract_unit"
+require "active_job"
+require "mailers/params_mailer"
+require "mailers/delayed_mailer"
+
+class LegacyDeliveryJobTest < ActiveSupport::TestCase
+  include ActiveJob::TestHelper
+
+  class LegacyDeliveryJob < ActionMailer::DeliveryJob
+  end
+
+  setup do
+    @previous_logger = ActiveJob::Base.logger
+    ActiveJob::Base.logger = Logger.new(nil)
+
+    @previous_delivery_method = ActionMailer::Base.delivery_method
+    ActionMailer::Base.delivery_method = :test
+
+    @previous_deliver_later_queue_name = ActionMailer::Base.deliver_later_queue_name
+    ActionMailer::Base.deliver_later_queue_name = :test_queue
+  end
+
+  teardown do
+    ActiveJob::Base.logger = @previous_logger
+    ParamsMailer.deliveries.clear
+
+    ActionMailer::Base.delivery_method = @previous_delivery_method
+    ActionMailer::Base.deliver_later_queue_name = @previous_deliver_later_queue_name
+  end
+
+  test "should send parameterized mail correctly" do
+    mail = ParamsMailer.with(inviter: "david@basecamp.com", invitee: "jason@basecamp.com").invitation
+    args = [
+      "ParamsMailer",
+      "invitation",
+      "deliver_now",
+      { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
+    ]
+
+    with_delivery_job(LegacyDeliveryJob) do
+      assert_deprecated do
+        assert_performed_with(job: ActionMailer::Parameterized::DeliveryJob, args: args) do
+          mail.deliver_later
+        end
+      end
+    end
+  end
+
+  test "should send mail correctly" do
+    mail = DelayedMailer.test_message(1, 2, 3)
+    args = [
+      "DelayedMailer",
+      "test_message",
+      "deliver_now",
+      1,
+      2,
+      3,
+    ]
+
+    with_delivery_job(LegacyDeliveryJob) do
+      assert_deprecated do
+        assert_performed_with(job: LegacyDeliveryJob, args: args) do
+          mail.deliver_later
+        end
+      end
+    end
+  end
+
+  private
+    def with_delivery_job(job)
+      old_params_delivery_job = ParamsMailer.delivery_job
+      old_regular_delivery_job = DelayedMailer.delivery_job
+      ParamsMailer.delivery_job = job
+      DelayedMailer.delivery_job = job
+      yield
+    ensure
+      ParamsMailer.delivery_job = old_params_delivery_job
+      DelayedMailer.delivery_job = old_regular_delivery_job
+    end
+end

actionmailer/test/test_helper_test.rb

@@ -206,6 +206,20 @@ class TestHelperMailerTest < ActionMailer::TestCase
     end
   end
 
+  def test_assert_enqueued_emails_with_legacy_delivery_job
+    previous_delivery_job = TestHelperMailer.delivery_job
+    TestHelperMailer.delivery_job = ActionMailer::DeliveryJob
+    assert_nothing_raised do
+      assert_enqueued_emails 1 do
+        silence_stream($stdout) do
+          TestHelperMailer.test.deliver_later
+        end
+      end
+    end
+  ensure
+    TestHelperMailer.delivery_job = previous_delivery_job
+  end
+
   def test_assert_enqueued_parameterized_emails
     assert_nothing_raised do
       assert_enqueued_emails 1 do
@@ -216,6 +230,20 @@ class TestHelperMailerTest < ActionMailer::TestCase
     end
   end
 
+  def test_assert_enqueued_parameterized_emails_with_legacy_delivery_job
+    previous_delivery_job = TestHelperMailer.delivery_job
+    TestHelperMailer.delivery_job = ActionMailer::DeliveryJob
+    assert_nothing_raised do
+      assert_enqueued_emails 1 do
+        silence_stream($stdout) do
+          TestHelperMailer.with(a: 1).test.deliver_later
+        end
+      end
+    end
+  ensure
+    TestHelperMailer.delivery_job = previous_delivery_job
+  end
+
   def test_assert_enqueued_emails_too_few_sent
     error = assert_raise ActiveSupport::TestCase::Assertion do
       assert_enqueued_emails 2 do

guides/source/6_1_release_notes.md

@@ -79,9 +79,6 @@ Please refer to the [Changelog][action-mailer] for detailed changes.
 
 ### Removals
 
-*   Remove deprecated `ActionMailer::DeliveryJob` and `ActionMailer::Parameterized::DeliveryJob`
-    in favor of `ActionMailer::MailDeliveryJob`.
-
 *   Remove deprecated `ActionMailer::Base.receive` in favor of [Action Mailbox](https://github.com/rails/rails/tree/master/actionmailbox).
 
 ### Deprecations

guides/source/configuring.md

@@ -770,7 +770,7 @@ There are a number of settings available on `config.action_mailer`:
 
 * `config.action_mailer.perform_caching` specifies whether the mailer templates should perform fragment caching or not. If it's not specified, the default will be `true`.
 
-* `config.action_mailer.delivery_job` specifies delivery job for mail. Defaults to `ActionMailer::MailDeliveryJob`.
+* `config.action_mailer.delivery_job` specifies delivery job for mail. Defaults to `ActionMailer::DeliveryJob`.
 
 
 ### Configuring Active Support

railties/test/application/configuration_test.rb

@@ -1123,7 +1123,7 @@ module ApplicationTests
       assert_equal [::MyMailObserver, ::MyOtherMailObserver], ::Mail.class_variable_get(:@@delivery_notification_observers)
     end
 
-    test "allows setting the queue name for the ActionMailer::MailDeliveryJob" do
+    test "allows setting the queue name for the ActionMailer::DeliveryJob" do
       add_to_config <<-RUBY
         config.action_mailer.deliver_later_queue_name = 'test_default'
       RUBY
@@ -2503,6 +2503,27 @@ module ApplicationTests
       assert_equal ActionMailer::MailDeliveryJob, ActionMailer::Base.delivery_job
     end
 
+    test "ActionMailer::Base.delivery_job is ActionMailer::DeliveryJob in the 5.x defaults" do
+      remove_from_config '.*config\.load_defaults.*\n'
+      add_to_config 'config.load_defaults "5.2"'
+
+      app "development"
+
+      assert_equal ActionMailer::DeliveryJob, ActionMailer::Base.delivery_job
+    end
+
+    test "ActionMailer::Base.delivery_job can be configured in the new framework defaults" do
+      remove_from_config '.*config\.load_defaults.*\n'
+
+      app_file "config/initializers/new_framework_defaults_6_0.rb", <<-RUBY
+        Rails.application.config.action_mailer.delivery_job = "ActionMailer::MailDeliveryJob"
+      RUBY
+
+      app "development"
+
+      assert_equal ActionMailer::MailDeliveryJob, ActionMailer::Base.delivery_job
+    end
+
     test "ActiveRecord::Base.filter_attributes should equal to filter_parameters" do
       app_file "config/initializers/filter_parameters_logging.rb", <<-RUBY
         Rails.application.config.filter_parameters += [ :password, :credit_card_number ]