mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Removed deprecated behavior that was not halting after_enqueue
/after_perform
callbacks when a previous callback was halted with throw :abort
.
This commit is contained in:
parent
d4ad739454
commit
10bd5e59c3
8 changed files with 20 additions and 175 deletions
|
@ -1,3 +1,12 @@
|
|||
* 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.
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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`
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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"
|
||||
|
||||
|
|
Loading…
Reference in a new issue