From ddc7fb6e6ee957aae35f1bb11d35c2d3e3b0cdda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Tue, 12 Oct 2021 21:50:40 +0000 Subject: [PATCH] Remove deprecated ActionMailer::DeliveryJob and `ActionMailer::Parameterized::DeliveryJob` --- actionmailer/CHANGELOG.md | 5 + actionmailer/lib/action_mailer.rb | 1 - actionmailer/lib/action_mailer/base.rb | 2 +- .../lib/action_mailer/delivery_job.rb | 45 ------- .../lib/action_mailer/message_delivery.rb | 38 +----- .../lib/action_mailer/parameterized.rb | 26 +--- actionmailer/lib/action_mailer/test_helper.rb | 3 +- actionmailer/test/legacy_delivery_job_test.rb | 127 ------------------ actionmailer/test/test_helper_test.rb | 28 ---- guides/source/7_0_release_notes.md | 3 + guides/source/configuring.md | 2 +- .../test/application/configuration_test.rb | 23 +--- 12 files changed, 18 insertions(+), 285 deletions(-) delete mode 100644 actionmailer/lib/action_mailer/delivery_job.rb delete mode 100644 actionmailer/test/legacy_delivery_job_test.rb diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 15d50888ad..57cdf0765e 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,8 @@ +* Remove deprecated `ActionMailer::DeliveryJob` and `ActionMailer::Parameterized::DeliveryJob` + in favor of `ActionMailer::MailDeliveryJob`. + + *Rafael Mendonça França* + * `email_address_with_name` returns just the address if name is blank. *Thomas Hutterer* diff --git a/actionmailer/lib/action_mailer.rb b/actionmailer/lib/action_mailer.rb index 2a9fceefe3..09d0353bd9 100644 --- a/actionmailer/lib/action_mailer.rb +++ b/actionmailer/lib/action_mailer.rb @@ -51,7 +51,6 @@ module ActionMailer autoload :TestCase autoload :TestHelper autoload :MessageDelivery - autoload :DeliveryJob autoload :MailDeliveryJob def self.eager_load! diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 6e6885d636..edd45196b5 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -462,7 +462,7 @@ module ActionMailer helper ActionMailer::MailHelper - class_attribute :delivery_job, default: ::ActionMailer::DeliveryJob + class_attribute :delivery_job, default: ::ActionMailer::MailDeliveryJob class_attribute :default_params, default: { mime_version: "1.0", charset: "UTF-8", diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb deleted file mode 100644 index 8edf34b519..0000000000 --- a/actionmailer/lib/action_mailer/delivery_job.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require "active_job" - -module ActionMailer - # The ActionMailer::DeliveryJob 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 7.0. - 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) - - 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/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb index ce968276e1..1fd669af3e 100644 --- a/actionmailer/lib/action_mailer/message_delivery.rb +++ b/actionmailer/lib/action_mailer/message_delivery.rb @@ -62,7 +62,7 @@ module ActionMailer # * :queue - Enqueue the email on the specified queue # * :priority - Enqueues the email with the specified priority # - # By default, the email will be enqueued using ActionMailer::DeliveryJob. Each + # By default, the email will be enqueued using ActionMailer::MailDeliveryJob. Each # ActionMailer::Base class can specify the job to use by setting the class variable # +delivery_job+. # @@ -88,7 +88,7 @@ module ActionMailer # * :queue - Enqueue the email on the specified queue. # * :priority - Enqueues the email with the specified priority # - # By default, the email will be enqueued using ActionMailer::DeliveryJob. Each + # By default, the email will be enqueued using ActionMailer::MailDeliveryJob. Each # ActionMailer::Base class can specify the job to use by setting the class variable # +delivery_job+. # @@ -140,38 +140,8 @@ 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 - job = @mailer_class.delivery_job - - if use_new_args?(job) - job.set(options).perform_later( - @mailer_class.name, @action.to_s, delivery_method.to_s, args: @args) - elsif job <= DeliveryJob - job.set(options).perform_later( - @mailer_class.name, @action.to_s, delivery_method.to_s, *@args) - else - ActiveSupport::Deprecation.warn(<<~EOM) - In Rails 7.0, Action Mailer will pass the mail arguments inside the `:args` keyword argument. - The `perform` method of the #{job} needs to change and forward the mail arguments - from the `args` keyword argument. - - The `perform` method should now look like: - - `def perform(mailer, mail_method, delivery, args:)` - EOM - - job.set(options).perform_later( - @mailer_class.name, @action.to_s, delivery_method.to_s, *@args) - end - end - end - - def use_new_args?(job) - parameters = job.public_instance_method(:perform).parameters - - parameters.find do |key, name| - return true if key == :keyreq && name == :args - - key == :keyrest and name != :** + @mailer_class.delivery_job.set(options).perform_later( + @mailer_class.name, @action.to_s, delivery_method.to_s, args: @args) end end end diff --git a/actionmailer/lib/action_mailer/parameterized.rb b/actionmailer/lib/action_mailer/parameterized.rb index 0fe26ed6dd..f8b2d74738 100644 --- a/actionmailer/lib/action_mailer/parameterized.rb +++ b/actionmailer/lib/action_mailer/parameterized.rb @@ -122,13 +122,6 @@ 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) - end - class MessageDelivery < ActionMailer::MessageDelivery # :nodoc: def initialize(mailer_class, action, params, *args) super(mailer_class, action, *args) @@ -148,23 +141,8 @@ module ActionMailer if processed? super else - 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 + @mailer_class.delivery_job.set(options).perform_later( + @mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args) end end end diff --git a/actionmailer/lib/action_mailer/test_helper.rb b/actionmailer/lib/action_mailer/test_helper.rb index ee1bce1897..a5ed233ee9 100644 --- a/actionmailer/lib/action_mailer/test_helper.rb +++ b/actionmailer/lib/action_mailer/test_helper.rb @@ -155,8 +155,7 @@ 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) || - ActionMailer::Parameterized::DeliveryJob == job_class + Base.descendants.map(&:delivery_job).include?(job_class) end end end diff --git a/actionmailer/test/legacy_delivery_job_test.rb b/actionmailer/test/legacy_delivery_job_test.rb deleted file mode 100644 index 92b2645e3a..0000000000 --- a/actionmailer/test/legacy_delivery_job_test.rb +++ /dev/null @@ -1,127 +0,0 @@ -# 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 - - class LegacyArgumentDeliveryJob < ActiveJob::Base - def perform(mailer, mail_method, delivery_method, *args) - end - end - - class NewArgumentDeliveryJob < ActiveJob::Base - def perform(mailer, mail_method, delivery_method, args:) - end - end - - class KeyRestArgumentJob < ActiveJob::Base - def perform(mailer, mail_method, delivery_method, **kwargs) - end - 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 - - test "triggers a deprecation warning when a delivery job use legacy arguments" do - with_delivery_job(LegacyArgumentDeliveryJob) do - assert_deprecated("Action Mailer will pass the mail arguments inside the `:args` keyword argument") do - perform_enqueued_jobs do - DelayedMailer.test_message(1, 2, 3).deliver_later - end - end - end - end - - test "does not trigger a deprecation warning when a delivery job use a required `args` kwargs" do - with_delivery_job(NewArgumentDeliveryJob) do - assert_not_deprecated do - perform_enqueued_jobs do - DelayedMailer.test_message(1, 2, 3).deliver_later - end - end - end - end - - test "does not trigger a deprecation warning when a delivery job use a keyrest argument" do - with_delivery_job(KeyRestArgumentJob) do - assert_not_deprecated do - perform_enqueued_jobs do - DelayedMailer.test_message(1, 2, 3).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 diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb index 582afd7af0..e261629ab4 100644 --- a/actionmailer/test/test_helper_test.rb +++ b/actionmailer/test/test_helper_test.rb @@ -214,20 +214,6 @@ 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 @@ -238,20 +224,6 @@ 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 diff --git a/guides/source/7_0_release_notes.md b/guides/source/7_0_release_notes.md index a62694a458..901ecc096c 100644 --- a/guides/source/7_0_release_notes.md +++ b/guides/source/7_0_release_notes.md @@ -74,6 +74,9 @@ Please refer to the [Changelog][action-mailer] for detailed changes. ### Removals +* Remove deprecated `ActionMailer::DeliveryJob` and `ActionMailer::Parameterized::DeliveryJob` + in favor of `ActionMailer::MailDeliveryJob`. + ### Deprecations ### Notable changes diff --git a/guides/source/configuring.md b/guides/source/configuring.md index bc127c22bd..ae0132c385 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1797,7 +1797,7 @@ Accepts a string for the HTML tag used to wrap attachments. Defaults to `"action - `config.action_controller.raise_on_open_redirects`: `false` - `config.action_controller.urlsafe_csrf_tokens`: `false` - `config.action_dispatch.cookies_same_site_protection`: `nil` -- `config.action_mailer.delivery_job`: `ActionMailer::DeliveryJob` +- `config.action_mailer.delivery_job`: `ActionMailer::MailDeliveryJob` - `config.action_view.form_with_generates_ids`: `false` - `config.action_view.preload_links_header`: `nil` - `config.action_view.button_to_generates_button_tag`: `false` diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 7916f0ccb5..fa1cf41b12 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1170,7 +1170,7 @@ module ApplicationTests assert_equal [::MyMailObserver, ::MyOtherMailObserver], ::Mail.class_variable_get(:@@delivery_notification_observers) end - test "allows setting the queue name for the ActionMailer::DeliveryJob" do + test "allows setting the queue name for the ActionMailer::MailDeliveryJob" do add_to_config <<-RUBY config.action_mailer.deliver_later_queue_name = 'test_default' RUBY @@ -3038,27 +3038,6 @@ 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 ]