Merge pull request #19029 from iainbeeston/skipping-undefined-callbacks
Raise ArgumentError if an unrecognised callback is skipped
This commit is contained in:
commit
8b88df94eb
@ -62,9 +62,9 @@ def _normalize_callback_option(options, from, to) # :nodoc:
|
||||
# using #skip_action_callback
|
||||
def skip_action_callback(*names)
|
||||
ActiveSupport::Deprecation.warn('`skip_action_callback` is deprecated and will be removed in the next major version of Rails. Please use skip_before_action, skip_after_action or skip_around_action instead.')
|
||||
skip_before_action(*names)
|
||||
skip_after_action(*names)
|
||||
skip_around_action(*names)
|
||||
skip_before_action(*names, raise: false)
|
||||
skip_after_action(*names, raise: false)
|
||||
skip_around_action(*names, raise: false)
|
||||
end
|
||||
|
||||
def skip_filter(*names)
|
||||
|
@ -1,3 +1,8 @@
|
||||
* `ActiveSupport::Callbacks#skip_callback` now raises an `ArgumentError` if
|
||||
an unrecognized callback is removed.
|
||||
|
||||
*Iain Beeston*
|
||||
|
||||
* Added `ActiveSupport::ArrayInquirer` and `Array#inquiry`.
|
||||
|
||||
Wrapping an array in an `ArrayInquirer` gives a friendlier way to check its
|
||||
|
@ -689,19 +689,27 @@ def set_callback(name, *filter_list, &block)
|
||||
# class Writer < Person
|
||||
# skip_callback :validate, :before, :check_membership, if: -> { self.age > 18 }
|
||||
# end
|
||||
#
|
||||
# An <tt>ArgumentError</tt> will be raised if the callback has not
|
||||
# already been set (unless the <tt>:raise</tt> option is set to <tt>false</tt>).
|
||||
def skip_callback(name, *filter_list, &block)
|
||||
type, filters, options = normalize_callback_params(filter_list, block)
|
||||
options[:raise] = true unless options.key?(:raise)
|
||||
|
||||
__update_callbacks(name) do |target, chain|
|
||||
filters.each do |filter|
|
||||
filter = chain.find {|c| c.matches?(type, filter) }
|
||||
callback = chain.find {|c| c.matches?(type, filter) }
|
||||
|
||||
if filter && options.any?
|
||||
new_filter = filter.merge_conditional_options(chain, if_option: options[:if], unless_option: options[:unless])
|
||||
chain.insert(chain.index(filter), new_filter)
|
||||
if !callback && options[:raise]
|
||||
raise ArgumentError, "#{type.to_s.capitalize} #{name} callback #{filter.inspect} has not been defined"
|
||||
end
|
||||
|
||||
chain.delete(filter)
|
||||
if callback && (options.key?(:if) || options.key?(:unless))
|
||||
new_callback = callback.merge_conditional_options(chain, if_option: options[:if], unless_option: options[:unless])
|
||||
chain.insert(chain.index(callback), new_callback)
|
||||
end
|
||||
|
||||
chain.delete(callback)
|
||||
end
|
||||
target.set_callbacks name, chain
|
||||
end
|
||||
|
@ -73,8 +73,8 @@ def save
|
||||
|
||||
class PersonSkipper < Person
|
||||
skip_callback :save, :before, :before_save_method, :if => :yes
|
||||
skip_callback :save, :after, :before_save_method, :unless => :yes
|
||||
skip_callback :save, :after, :before_save_method, :if => :no
|
||||
skip_callback :save, :after, :after_save_method, :unless => :yes
|
||||
skip_callback :save, :after, :after_save_method, :if => :no
|
||||
skip_callback :save, :before, :before_save_method, :unless => :no
|
||||
skip_callback :save, :before, CallbackClass , :if => :yes
|
||||
def yes; true; end
|
||||
@ -1021,7 +1021,7 @@ def build_class(callback, n = 10)
|
||||
define_callbacks :foo
|
||||
n.times { set_callback :foo, :before, callback }
|
||||
def run; run_callbacks :foo; end
|
||||
def self.skip(thing); skip_callback :foo, :before, thing; end
|
||||
def self.skip(*things); skip_callback :foo, :before, *things; end
|
||||
}
|
||||
end
|
||||
|
||||
@ -1070,11 +1070,11 @@ def test_skip_class # removes one at a time
|
||||
}
|
||||
end
|
||||
|
||||
def test_skip_lambda # removes nothing
|
||||
def test_skip_lambda # raises error
|
||||
calls = []
|
||||
callback = ->(o) { calls << o }
|
||||
klass = build_class(callback)
|
||||
10.times { klass.skip callback }
|
||||
assert_raises(ArgumentError) { klass.skip callback }
|
||||
klass.new.run
|
||||
assert_equal 10, calls.length
|
||||
end
|
||||
@ -1088,11 +1088,29 @@ def test_skip_symbol # removes all
|
||||
assert_equal 0, calls.length
|
||||
end
|
||||
|
||||
def test_skip_eval # removes nothing
|
||||
def test_skip_string # raises error
|
||||
calls = []
|
||||
klass = build_class("bar")
|
||||
klass.class_eval { define_method(:bar) { calls << klass } }
|
||||
klass.skip "bar"
|
||||
assert_raises(ArgumentError) { klass.skip "bar" }
|
||||
klass.new.run
|
||||
assert_equal 1, calls.length
|
||||
end
|
||||
|
||||
def test_skip_undefined_callback # raises error
|
||||
calls = []
|
||||
klass = build_class(:bar)
|
||||
klass.class_eval { define_method(:bar) { calls << klass } }
|
||||
assert_raises(ArgumentError) { klass.skip :qux }
|
||||
klass.new.run
|
||||
assert_equal 1, calls.length
|
||||
end
|
||||
|
||||
def test_skip_without_raise # removes nothing
|
||||
calls = []
|
||||
klass = build_class(:bar)
|
||||
klass.class_eval { define_method(:bar) { calls << klass } }
|
||||
klass.skip :qux, raise: false
|
||||
klass.new.run
|
||||
assert_equal 1, calls.length
|
||||
end
|
||||
|
Loading…
Reference in New Issue
Block a user