diff --git a/activejob/lib/active_job/callbacks.rb b/activejob/lib/active_job/callbacks.rb index e393da81a2..43943d6fb3 100644 --- a/activejob/lib/active_job/callbacks.rb +++ b/activejob/lib/active_job/callbacks.rb @@ -42,15 +42,6 @@ module ActiveJob # callbacks for +perform+ and +enqueue+ methods. module ClassMethods def inherited(klass) - unless skip_after_callbacks_if_terminated - ActiveSupport::Deprecation.warn(<<~EOM) - In Rails 6.2, ActiveJob's `after_enqueue` and `after_perform` callbacks will no longer run in case the - callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback). - To enable this behaviour right now, add in your application configuration file - `config.active_job.skip_after_callbacks_if_terminated = true`. - EOM - end - 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 diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index cd588de617..02a5420e8d 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -63,6 +63,14 @@ module ActiveJob if successfully_enqueued self else + if !self.class.skip_after_callbacks_if_terminated && _enqueue_callbacks.any? { |c| c.kind == :after } + ActiveSupport::Deprecation.warn(<<~EOM) + In Rails 6.2, ActiveJob's `after_enqueue` callbacks will no longer run in case the + callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback). + To enable this behaviour right now, add in your application configuration file + `config.active_job.skip_after_callbacks_if_terminated = true`. + EOM + end if self.class.return_false_on_aborted_enqueue false else diff --git a/activejob/lib/active_job/execution.rb b/activejob/lib/active_job/execution.rb index e96dbcd4c9..5cf41a4283 100644 --- a/activejob/lib/active_job/execution.rb +++ b/activejob/lib/active_job/execution.rb @@ -35,9 +35,23 @@ module ActiveJob self.executions = (executions || 0) + 1 deserialize_arguments_if_needed + successfully_performed = false + run_callbacks :perform do perform(*arguments) + + successfully_performed = true end + + if !successfully_performed && !self.class.skip_after_callbacks_if_terminated && _perform_callbacks.any? { |c| c.kind == :after } + ActiveSupport::Deprecation.warn(<<~EOM) + In Rails 6.2, ActiveJob's `after_perform` callbacks will no longer run in case the + callback chain is halted (i.e. `throw(:abort)` is thrown in a before_perform callback). + To enable this behaviour right now, add in your application configuration file + `config.active_job.skip_after_callbacks_if_terminated = true`. + EOM + end + successfully_performed rescue => exception rescue_with_handler(exception) || raise end diff --git a/activejob/test/cases/callbacks_test.rb b/activejob/test/cases/callbacks_test.rb index 87dc7634ae..412b7618f9 100644 --- a/activejob/test/cases/callbacks_test.rb +++ b/activejob/test/cases/callbacks_test.rb @@ -27,7 +27,10 @@ class CallbacksTest < ActiveSupport::TestCase test "#enqueue returns false when before_enqueue aborts callback chain and return_false_on_aborted_enqueue = true" do prev = ActiveJob::Base.return_false_on_aborted_enqueue ActiveJob::Base.return_false_on_aborted_enqueue = true - assert_equal false, AbortBeforeEnqueueJob.new.enqueue + + ActiveSupport::Deprecation.silence do + assert_equal false, AbortBeforeEnqueueJob.new.enqueue + end ensure ActiveJob::Base.return_false_on_aborted_enqueue = prev end @@ -48,7 +51,9 @@ class CallbacksTest < ActiveSupport::TestCase ActiveJob::Base.skip_after_callbacks_if_terminated = true reload_job job = AbortBeforeEnqueueJob.new - job.enqueue + ActiveSupport::Deprecation.silence do + job.enqueue + end assert_nil(job.flag) ensure @@ -60,13 +65,31 @@ class CallbacksTest < ActiveSupport::TestCase ActiveJob::Base.skip_after_callbacks_if_terminated = false reload_job job = AbortBeforeEnqueueJob.new - job.enqueue + assert_deprecated(/`after_enqueue` callbacks will 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) } + self.return_false_on_aborted_enqueue = true + end.new + + assert_not_deprecated do + job.enqueue + 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 @@ -84,13 +107,30 @@ class CallbacksTest < ActiveSupport::TestCase ActiveJob::Base.skip_after_callbacks_if_terminated = false reload_job job = AbortBeforeEnqueueJob.new - job.perform_now + assert_deprecated(/`after_perform` callbacks will 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 "#enqueue returns self when the job was enqueued" do job = CallbackJob.new assert_equal job, job.enqueue diff --git a/activejob/test/helper.rb b/activejob/test/helper.rb index 2b1aac8875..d7925aacce 100644 --- a/activejob/test/helper.rb +++ b/activejob/test/helper.rb @@ -12,6 +12,7 @@ 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