diff options
Diffstat (limited to 'activesupport')
| -rw-r--r-- | activesupport/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | activesupport/lib/active_support.rb | 8 | ||||
| -rw-r--r-- | activesupport/lib/active_support/cache/file_store.rb | 1 | ||||
| -rw-r--r-- | activesupport/lib/active_support/callbacks.rb | 34 | ||||
| -rw-r--r-- | activesupport/lib/active_support/railtie.rb | 7 | ||||
| -rw-r--r-- | activesupport/test/caching_test.rb | 6 | ||||
| -rw-r--r-- | activesupport/test/callbacks_test.rb | 32 |
7 files changed, 62 insertions, 31 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.rb b/activesupport/lib/active_support.rb index 9af3f8b447..588d6c49f9 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -73,6 +73,14 @@ module ActiveSupport end cattr_accessor :test_order # :nodoc: + + def self.halt_callback_chains_on_return_false + Callbacks::CallbackChain.halt_and_display_warning_on_return_false + end + + def self.halt_callback_chains_on_return_false=(value) + Callbacks::CallbackChain.halt_and_display_warning_on_return_false = value + end end autoload :I18n, "active_support/i18n" diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index d08ecd2f7d..e6a8b84214 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -29,6 +29,7 @@ module ActiveSupport def clear(options = nil) root_dirs = Dir.entries(cache_path).reject {|f| (EXCLUDED_DIRS + [".gitkeep"]).include?(f)} FileUtils.rm_r(root_dirs.collect{|f| File.join(cache_path, f)}) + rescue Errno::ENOENT end # Preemptively iterates through all stored keys and removes the ones which have expired. diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 9cf09ab266..814fd288cf 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -80,14 +80,10 @@ module ActiveSupport # save # end def run_callbacks(kind, &block) - send "_run_#{kind}_callbacks", &block - end - - private + callbacks = send("_#{kind}_callbacks") - def _run_callbacks(callbacks, &block) if callbacks.empty? - block.call if block + yield if block_given? else runner = callbacks.compile e = Filters::Environment.new(self, false, nil, block) @@ -95,6 +91,8 @@ module ActiveSupport end end + private + # A hook invoked every time a before callback is halted. # This can be overridden in AS::Callback implementors in order # to provide better debugging/logging. @@ -691,19 +689,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 @@ -800,12 +806,6 @@ module ActiveSupport names.each do |name| class_attribute "_#{name}_callbacks" set_callbacks name, CallbackChain.new(name, options) - - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def _run_#{name}_callbacks(&block) - _run_callbacks(_#{name}_callbacks, &block) - end - RUBY end end diff --git a/activesupport/lib/active_support/railtie.rb b/activesupport/lib/active_support/railtie.rb index ef22433491..cd0fb51009 100644 --- a/activesupport/lib/active_support/railtie.rb +++ b/activesupport/lib/active_support/railtie.rb @@ -13,13 +13,6 @@ module ActiveSupport end end - initializer "active_support.halt_callback_chains_on_return_false", after: :load_config_initializers do |app| - if app.config.active_support.key? :halt_callback_chains_on_return_false - ActiveSupport::Callbacks::CallbackChain.halt_and_display_warning_on_return_false = \ - app.config.active_support.halt_callback_chains_on_return_false - end - end - # Sets the default value for Time.zone # If assigned value cannot be matched to a TimeZone, an exception will be raised. initializer "active_support.initialize_time_zone" do |app| diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 4953550c45..527538ed9a 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -684,6 +684,7 @@ class FileStoreTest < ActiveSupport::TestCase def teardown FileUtils.rm_r(cache_dir) + rescue Errno::ENOENT end def cache_dir @@ -703,6 +704,11 @@ class FileStoreTest < ActiveSupport::TestCase assert File.exist?(filepath) end + def test_clear_without_cache_dir + FileUtils.rm_r(cache_dir) + @cache.clear + end + def test_long_keys @cache.write("a"*10000, 1) assert_equal 1, @cache.read("a"*10000) 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 |