Make sure the :if options of callbacks is not mutated
In some cases, the framework was mutating the :if option of callbacks. Since #38323, those options are frozen, so the framework could raise exception when trying to mutate those options if they were being resued with method like `with_options`.
This commit is contained in:
parent
03156c829d
commit
b4cab6a854
|
@ -147,7 +147,7 @@ module ActiveModel
|
|||
conditional = ActiveSupport::Callbacks::Conditionals::Value.new { |v|
|
||||
v != false
|
||||
}
|
||||
options[:if] = Array(options[:if]) << conditional
|
||||
options[:if] = Array(options[:if]) + [conditional]
|
||||
set_callback(:"#{callback}", :after, *args, options, &block)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -56,14 +56,7 @@ module ActiveModel
|
|||
def before_validation(*args, &block)
|
||||
options = args.extract_options!
|
||||
|
||||
if options.key?(:on)
|
||||
options = options.dup
|
||||
options[:on] = Array(options[:on])
|
||||
options[:if] = Array(options[:if])
|
||||
options[:if].unshift ->(o) {
|
||||
!(options[:on] & Array(o.validation_context)).empty?
|
||||
}
|
||||
end
|
||||
set_options_for_callback(options)
|
||||
|
||||
set_callback(:validation, :before, *args, options, &block)
|
||||
end
|
||||
|
@ -99,13 +92,7 @@ module ActiveModel
|
|||
options = options.dup
|
||||
options[:prepend] = true
|
||||
|
||||
if options.key?(:on)
|
||||
options[:on] = Array(options[:on])
|
||||
options[:if] = Array(options[:if])
|
||||
options[:if].unshift ->(o) {
|
||||
!(options[:on] & Array(o.validation_context)).empty?
|
||||
}
|
||||
end
|
||||
set_options_for_callback(options)
|
||||
|
||||
set_callback(:validation, :after, *args, options, &block)
|
||||
end
|
||||
|
@ -116,6 +103,18 @@ module ActiveModel
|
|||
def run_validations!
|
||||
_run_validation_callbacks { super }
|
||||
end
|
||||
|
||||
def set_options_for_callback(options)
|
||||
if options.key?(:on)
|
||||
options[:on] = Array(options[:on])
|
||||
options[:if] = [
|
||||
->(o) {
|
||||
!(options[:on] & Array(o.validation_context)).empty?
|
||||
},
|
||||
*options[:if]
|
||||
]
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -102,6 +102,16 @@ class CallbacksTest < ActiveModel::TestCase
|
|||
assert_not_respond_to ModelCallbacks, :after_empty
|
||||
end
|
||||
|
||||
test "the :if option array should not be mutated by an after callback" do
|
||||
opts = []
|
||||
|
||||
Class.new(ModelCallbacks) do
|
||||
after_create(if: opts) { }
|
||||
end
|
||||
|
||||
assert_empty opts
|
||||
end
|
||||
|
||||
class Violin
|
||||
attr_reader :history
|
||||
def initialize
|
||||
|
|
|
@ -185,4 +185,24 @@ class CallbacksWithMethodNamesShouldBeCalled < ActiveModel::TestCase
|
|||
assert_equal ["before_validation_marker"], d.history
|
||||
assert_equal false, output
|
||||
end
|
||||
|
||||
def test_before_validation_does_not_mutate_the_if_options_array
|
||||
opts = []
|
||||
|
||||
Class.new(Dog) do
|
||||
before_validation(if: opts, on: :create) { }
|
||||
end
|
||||
|
||||
assert_empty opts
|
||||
end
|
||||
|
||||
def test_after_validation_does_not_mutate_the_if_options_array
|
||||
opts = []
|
||||
|
||||
Class.new(Dog) do
|
||||
after_validation(if: opts, on: :create) { }
|
||||
end
|
||||
|
||||
assert_empty opts
|
||||
end
|
||||
end
|
||||
|
|
|
@ -271,8 +271,10 @@ module ActiveRecord
|
|||
if options[:on]
|
||||
fire_on = Array(options[:on])
|
||||
assert_valid_transaction_action(fire_on)
|
||||
options[:if] = Array(options[:if])
|
||||
options[:if].unshift(-> { transaction_include_any_action?(fire_on) })
|
||||
options[:if] = [
|
||||
-> { transaction_include_any_action?(fire_on) },
|
||||
*options[:if]
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -538,6 +538,18 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
|
|||
assert_equal [:after_rollback], records.second.history
|
||||
end
|
||||
|
||||
def test_after_commit_does_not_mutate_the_if_options_array
|
||||
opts = []
|
||||
|
||||
Class.new(ActiveRecord::Base) do
|
||||
self.table_name = "topics"
|
||||
|
||||
after_commit(if: opts, on: :create) { }
|
||||
end
|
||||
|
||||
assert_empty opts
|
||||
end
|
||||
|
||||
private
|
||||
def add_transaction_execution_blocks(record)
|
||||
record.after_commit_block(:create) { |r| r.history << :commit_on_create }
|
||||
|
|
Loading…
Reference in New Issue