Fix deprecation being thrown at boot time:

-
  ### Problem

  In rails/rails@bbfab0b33a I introduced a change which outputs
  a deprecation whenever a class inherits from ActiveJob::Base.

  This has the negative effect to trigger a massive amount of
  deprecation at boot time especially if your app is eagerloaded
  (like it's usually the case on CI).

  Another issue with this approach was that the deprecation will
  be output no matter if a job define a `after_perform` callbacks
  i.e.
  ```ruby
    class MyJob < AJ::Base
      before_enqueue { throw(:abort) }
    end

    # This shouldn't trigger a deprecation since no after callbacks are defined
    # The change in 6.2 will be already safe for the app.
  ```

  ### Solution

  Trigger the deprecation only when a job is abort
  (during enqueuing or performing) AND a `after_perform`
  callback is defined on the job.
This commit is contained in:
Edouard CHIN 2019-12-13 03:25:03 +01:00
parent ff299f1741
commit 9eb4b4ed01
5 changed files with 67 additions and 13 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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