Skip to content

Commit 9eb4b4e

Browse files
committed
Fix deprecation being thrown at boot time:
- ### Problem In rails/rails@bbfab0b33ab 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.
1 parent ff299f1 commit 9eb4b4e

File tree

5 files changed

+67
-13
lines changed

5 files changed

+67
-13
lines changed

activejob/lib/active_job/callbacks.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,6 @@ class << self
4242
# callbacks for +perform+ and +enqueue+ methods.
4343
module ClassMethods
4444
def inherited(klass)
45-
unless skip_after_callbacks_if_terminated
46-
ActiveSupport::Deprecation.warn(<<~EOM)
47-
In Rails 6.2, ActiveJob's `after_enqueue` and `after_perform` callbacks will no longer run in case the
48-
callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback).
49-
To enable this behaviour right now, add in your application configuration file
50-
`config.active_job.skip_after_callbacks_if_terminated = true`.
51-
EOM
52-
end
53-
5445
klass.get_callbacks(:enqueue).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated
5546
klass.get_callbacks(:perform).config[:skip_after_callbacks_if_terminated] = skip_after_callbacks_if_terminated
5647
super

activejob/lib/active_job/enqueuing.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ def enqueue(options = {})
6363
if successfully_enqueued
6464
self
6565
else
66+
if !self.class.skip_after_callbacks_if_terminated && _enqueue_callbacks.any? { |c| c.kind == :after }
67+
ActiveSupport::Deprecation.warn(<<~EOM)
68+
In Rails 6.2, ActiveJob's `after_enqueue` callbacks will no longer run in case the
69+
callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback).
70+
To enable this behaviour right now, add in your application configuration file
71+
`config.active_job.skip_after_callbacks_if_terminated = true`.
72+
EOM
73+
end
6674
if self.class.return_false_on_aborted_enqueue
6775
false
6876
else

activejob/lib/active_job/execution.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,23 @@ def perform_now
3535
self.executions = (executions || 0) + 1
3636

3737
deserialize_arguments_if_needed
38+
successfully_performed = false
39+
3840
run_callbacks :perform do
3941
perform(*arguments)
42+
43+
successfully_performed = true
44+
end
45+
46+
if !successfully_performed && !self.class.skip_after_callbacks_if_terminated && _perform_callbacks.any? { |c| c.kind == :after }
47+
ActiveSupport::Deprecation.warn(<<~EOM)
48+
In Rails 6.2, ActiveJob's `after_perform` callbacks will no longer run in case the
49+
callback chain is halted (i.e. `throw(:abort)` is thrown in a before_perform callback).
50+
To enable this behaviour right now, add in your application configuration file
51+
`config.active_job.skip_after_callbacks_if_terminated = true`.
52+
EOM
4053
end
54+
successfully_performed
4155
rescue => exception
4256
rescue_with_handler(exception) || raise
4357
end

activejob/test/cases/callbacks_test.rb

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ class CallbacksTest < ActiveSupport::TestCase
2727
test "#enqueue returns false when before_enqueue aborts callback chain and return_false_on_aborted_enqueue = true" do
2828
prev = ActiveJob::Base.return_false_on_aborted_enqueue
2929
ActiveJob::Base.return_false_on_aborted_enqueue = true
30-
assert_equal false, AbortBeforeEnqueueJob.new.enqueue
30+
31+
ActiveSupport::Deprecation.silence do
32+
assert_equal false, AbortBeforeEnqueueJob.new.enqueue
33+
end
3134
ensure
3235
ActiveJob::Base.return_false_on_aborted_enqueue = prev
3336
end
@@ -48,7 +51,9 @@ class CallbacksTest < ActiveSupport::TestCase
4851
ActiveJob::Base.skip_after_callbacks_if_terminated = true
4952
reload_job
5053
job = AbortBeforeEnqueueJob.new
51-
job.enqueue
54+
ActiveSupport::Deprecation.silence do
55+
job.enqueue
56+
end
5257

5358
assert_nil(job.flag)
5459
ensure
@@ -60,13 +65,31 @@ class CallbacksTest < ActiveSupport::TestCase
6065
ActiveJob::Base.skip_after_callbacks_if_terminated = false
6166
reload_job
6267
job = AbortBeforeEnqueueJob.new
63-
job.enqueue
68+
assert_deprecated(/`after_enqueue` callbacks will no longer run/) do
69+
job.enqueue
70+
end
6471

6572
assert_equal("after_enqueue", job.flag)
6673
ensure
6774
ActiveJob::Base.skip_after_callbacks_if_terminated = prev
6875
end
6976

77+
test "#enqueue does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do
78+
prev = ActiveJob::Base.skip_after_callbacks_if_terminated
79+
ActiveJob::Base.skip_after_callbacks_if_terminated = false
80+
81+
job = Class.new(ActiveJob::Base) do
82+
before_enqueue { throw(:abort) }
83+
self.return_false_on_aborted_enqueue = true
84+
end.new
85+
86+
assert_not_deprecated do
87+
job.enqueue
88+
end
89+
ensure
90+
ActiveJob::Base.skip_after_callbacks_if_terminated = prev
91+
end
92+
7093
test "#perform does not run after_perform callbacks when skip_after_callbacks_if_terminated is true" do
7194
prev = ActiveJob::Base.skip_after_callbacks_if_terminated
7295
ActiveJob::Base.skip_after_callbacks_if_terminated = true
@@ -84,13 +107,30 @@ class CallbacksTest < ActiveSupport::TestCase
84107
ActiveJob::Base.skip_after_callbacks_if_terminated = false
85108
reload_job
86109
job = AbortBeforeEnqueueJob.new
87-
job.perform_now
110+
assert_deprecated(/`after_perform` callbacks will no longer run/) do
111+
job.perform_now
112+
end
88113

89114
assert_equal("after_perform", job.flag)
90115
ensure
91116
ActiveJob::Base.skip_after_callbacks_if_terminated = prev
92117
end
93118

119+
test "#perform does not throw a deprecation warning when skip_after_callbacks_if_terminated_is false but job has no after callbacks" do
120+
prev = ActiveJob::Base.skip_after_callbacks_if_terminated
121+
ActiveJob::Base.skip_after_callbacks_if_terminated = false
122+
123+
job = Class.new(ActiveJob::Base) do
124+
before_perform { throw(:abort) }
125+
end
126+
127+
assert_not_deprecated do
128+
job.perform_now
129+
end
130+
ensure
131+
ActiveJob::Base.skip_after_callbacks_if_terminated = prev
132+
end
133+
94134
test "#enqueue returns self when the job was enqueued" do
95135
job = CallbackJob.new
96136
assert_equal job, job.enqueue

activejob/test/helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
require "support/integration/helper"
1313
else
1414
ActiveJob::Base.logger = Logger.new(nil)
15+
ActiveJob::Base.skip_after_callbacks_if_terminated = true
1516
require "adapters/#{@adapter}"
1617
end
1718

0 commit comments

Comments
 (0)