diff options
Diffstat (limited to 'activesupport')
| -rw-r--r-- | activesupport/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 18 | ||||
| -rw-r--r-- | activesupport/test/callbacks_test.rb | 32 |
3 files changed, 43 insertions, 12 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7eaad6340f..3ad2392365 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -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 diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 9cf09ab266..19d2b1099d 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -691,19 +691,27 @@ module ActiveSupport # 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 !callback && options[:raise] + raise ArgumentError, "#{type.to_s.capitalize} #{name} callback #{filter.inspect} has not been defined" + end - 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.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(filter) + chain.delete(callback) end target.set_callbacks name, chain end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index f6abef8cee..cda9732cae 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -73,8 +73,8 @@ module CallbacksTest 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 @@ module CallbacksTest 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 @@ module CallbacksTest } 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 @@ module CallbacksTest 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 |