diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index c469f5c85c..3536e0d8e5 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,5 +1,14 @@ +* Deprecated `Rails.config.active_job.skip_after_callbacks_if_terminated`. + + *Rafael Mendonça França* + +* Removed deprecated behavior that was not halting `after_enqueue`/`after_perform` callbacks when a + previous callback was halted with `throw :abort`. + + *Rafael Mendonça França* + * Raise an `SerializationError` in `Serializer::ModuleSerializer` - if the module name is not present. + if the module name is not present. *Veerpal Brar* diff --git a/activejob/lib/active_job/callbacks.rb b/activejob/lib/active_job/callbacks.rb index a5f82bd470..cabda57b0d 100644 --- a/activejob/lib/active_job/callbacks.rb +++ b/activejob/lib/active_job/callbacks.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/callbacks" -require "active_support/core_ext/object/with_options" require "active_support/core_ext/module/attribute_accessors" module ActiveJob @@ -32,22 +31,15 @@ module ActiveJob class_attribute :return_false_on_aborted_enqueue, instance_accessor: false, instance_predicate: false, default: false singleton_class.deprecate :return_false_on_aborted_enqueue, :return_false_on_aborted_enqueue= cattr_accessor :skip_after_callbacks_if_terminated, instance_accessor: false, default: false + singleton_class.deprecate :skip_after_callbacks_if_terminated, :skip_after_callbacks_if_terminated= - with_options(skip_after_callbacks_if_terminated: skip_after_callbacks_if_terminated) do - define_callbacks :perform - define_callbacks :enqueue - end + define_callbacks :perform, skip_after_callbacks_if_terminated: true + define_callbacks :enqueue, skip_after_callbacks_if_terminated: true end # These methods will be included into any Active Job object, adding # callbacks for +perform+ and +enqueue+ methods. module ClassMethods - def inherited(klass) - klass.get_callbacks(:enqueue).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated - klass.get_callbacks(:perform).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated - super - end - # Defines a callback that will get called right before the # job's perform method is executed. # @@ -178,21 +170,5 @@ module ActiveJob set_callback(:enqueue, :around, *filters, &blk) end end - - private - def halted_callback_hook(_filter, name) # :nodoc: - return super unless %i(enqueue perform).include?(name.to_sym) - callbacks = public_send("_#{name}_callbacks") - - if !self.class.skip_after_callbacks_if_terminated && callbacks.any? { |c| c.kind == :after } - ActiveSupport::Deprecation.warn(<<~EOM) - In Rails 7.0, `after_enqueue`/`after_perform` callbacks no longer run if `before_enqueue`/`before_perform` respectively halts with `throw :abort`. - To enable this behavior, uncomment the `config.active_job.skip_after_callbacks_if_terminated` config - in the new 6.1 framework defaults initializer. - EOM - end - - super - end end end diff --git a/activejob/test/cases/callbacks_test.rb b/activejob/test/cases/callbacks_test.rb index 655638db8d..97fde7434a 100644 --- a/activejob/test/cases/callbacks_test.rb +++ b/activejob/test/cases/callbacks_test.rb @@ -54,145 +54,24 @@ class CallbacksTest < ActiveSupport::TestCase assert_equal false, AbortBeforeEnqueueJob.new.enqueue end - test "#enqueue does not run after_enqueue callbacks when skip_after_callbacks_if_terminated is true" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = true - reload_job + test "#enqueue does not run after_enqueue callbacks when previous callbacks aborted" do job = AbortBeforeEnqueueJob.new ActiveSupport::Deprecation.silence do job.enqueue end assert_nil(job.flag) - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev end - test "#enqueue does run after_enqueue callbacks when skip_after_callbacks_if_terminated is false" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - reload_job - job = AbortBeforeEnqueueJob.new - assert_deprecated(/`after_enqueue`\/`after_perform` callbacks no longer run/) do - job.enqueue - end - - assert_equal("after_enqueue", job.flag) - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#enqueue does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - - job = Class.new(ActiveJob::Base) do - before_enqueue { throw(:abort) } - end.new - - assert_not_deprecated do - job.enqueue - end - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#enqueue does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false and job did not throw an abort" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - - job = Class.new(ActiveJob::Base) do - after_enqueue { nil } - - around_enqueue do |_, block| - block.call - rescue ArgumentError - nil - end - - before_enqueue { raise ArgumentError } - end - - assert_not_deprecated do - job.perform_later - end - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#perform does not run after_perform callbacks when skip_after_callbacks_if_terminated is true" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = true - reload_job + test "#perform does not run after_perform callbacks when swhen previous callbacks aborted" do job = AbortBeforeEnqueueJob.new job.perform_now assert_nil(job.flag) - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#perform does run after_perform callbacks when skip_after_callbacks_if_terminated is false" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - reload_job - job = AbortBeforeEnqueueJob.new - assert_deprecated(/`after_enqueue`\/`after_perform` callbacks no longer run/) do - job.perform_now - end - - assert_equal("after_perform", job.flag) - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#perform does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - - job = Class.new(ActiveJob::Base) do - before_perform { throw(:abort) } - end - - assert_not_deprecated do - job.perform_now - end - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev - end - - test "#perform does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false and job did not throw an abort" do - prev = ActiveJob::Base.skip_after_callbacks_if_terminated - ActiveJob::Base.skip_after_callbacks_if_terminated = false - - job = Class.new(ActiveJob::Base) do - after_perform { nil } - - around_perform do |_, block| - block.call - rescue ArgumentError - nil - end - - before_perform { raise ArgumentError } - end - - assert_not_deprecated do - job.perform_now - end - ensure - ActiveJob::Base.skip_after_callbacks_if_terminated = prev end test "#enqueue returns self when the job was enqueued" do job = CallbackJob.new assert_equal job, job.enqueue end - - private - def reload_job - Object.send(:remove_const, :AbortBeforeEnqueueJob) - $LOADED_FEATURES.delete($LOADED_FEATURES.grep(%r{jobs/abort_before_enqueue_job}).first) - require "jobs/abort_before_enqueue_job" - end end diff --git a/activejob/test/helper.rb b/activejob/test/helper.rb index d7925aacce..2b1aac8875 100644 --- a/activejob/test/helper.rb +++ b/activejob/test/helper.rb @@ -12,7 +12,6 @@ if ENV["AJ_INTEGRATION_TESTS"] require "support/integration/helper" else ActiveJob::Base.logger = Logger.new(nil) - ActiveJob::Base.skip_after_callbacks_if_terminated = true require "adapters/#{@adapter}" end diff --git a/guides/source/7_0_release_notes.md b/guides/source/7_0_release_notes.md index 2787528a65..46458d5ada 100644 --- a/guides/source/7_0_release_notes.md +++ b/guides/source/7_0_release_notes.md @@ -144,8 +144,13 @@ Please refer to the [Changelog][active-job] for detailed changes. ### Removals +* Removed deprecated behavior that was not halting `after_enqueue`/`after_perform` callbacks when a + previous callback was halted with `throw :abort`. + ### Deprecations +* Deprecated `Rails.config.active_job.skip_after_callbacks_if_terminated`. + ### Notable changes Action Text diff --git a/guides/source/configuring.md b/guides/source/configuring.md index ae0132c385..4a040fc5f1 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -1499,11 +1499,6 @@ Controls if the arguments of a job are logged. Defaults to `true`. Controls the amount of "jitter" (random variation) applied to the delay time calculated when retrying failed jobs. -#### `config.active_job.skip_after_callbacks_if_terminated` - -Controls whether `after_enqueue` / `after_perform` callbacks run when a -`before_enqueue` / `before_perform` callback halts with `throw :abort`. - #### `config.active_job.log_query_tags_around_perform` Determines whether job context for query tags will be automatically updated via @@ -1750,7 +1745,6 @@ Accepts a string for the HTML tag used to wrap attachments. Defaults to `"action - `config.action_mailbox.queues.routing`: `nil` - `config.action_mailer.deliver_later_queue_name`: `nil` - `config.active_job.retry_jitter`: `0.15` -- `config.active_job.skip_after_callbacks_if_terminated`: `true` - `config.action_dispatch.cookies_same_site_protection`: `:lax` - `config.action_dispatch.ssl_default_redirect_status` = `308` - `ActiveSupport.utc_to_local_returns_utc_offset_times`: `true` @@ -1803,7 +1797,6 @@ Accepts a string for the HTML tag used to wrap attachments. Defaults to `"action - `config.action_view.button_to_generates_button_tag`: `false` - `config.action_view.apply_stylesheet_media_default`: `true` - `config.active_job.retry_jitter`: `0.0` -- `config.active_job.skip_after_callbacks_if_terminated`: `false` - `config.action_mailbox.queues.incineration`: `:action_mailbox_incineration` - `config.action_mailbox.queues.routing`: `:action_mailbox_routing` - `config.action_mailer.deliver_later_queue_name`: `:mailers` diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index a832ba2530..122472231c 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -161,7 +161,6 @@ module Rails if respond_to?(:active_job) active_job.retry_jitter = 0.15 - active_job.skip_after_callbacks_if_terminated = true end if respond_to?(:action_dispatch) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index fa1cf41b12..feaf2a47e5 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2706,21 +2706,6 @@ module ApplicationTests assert_equal 0.22, ActiveJob::Base.retry_jitter end - test "ActiveJob::Base.skip_after_callbacks_if_terminated is true by default" do - app "development" - - assert_equal true, ActiveJob::Base.skip_after_callbacks_if_terminated - end - - test "ActiveJob::Base.skip_after_callbacks_if_terminated is false in the 6.0 defaults" do - remove_from_config '.*config\.load_defaults.*\n' - add_to_config 'config.load_defaults "6.0"' - - app "development" - - assert_equal false, ActiveJob::Base.skip_after_callbacks_if_terminated - end - test "Rails.application.config.action_dispatch.cookies_same_site_protection is :lax by default" do app "production"