From cf04e621270bb2e5e9e7971d2c59e73d6797482d Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 17 Apr 2008 23:30:01 -0500 Subject: [PATCH] Tidy up ActiveSupport::Callbacks::CallbackChain instance API. --- .../lib/action_controller/dispatcher.rb | 2 +- actionpack/lib/action_controller/filters.rb | 6 ++-- activemodel/lib/active_model/validations.rb | 3 +- activerecord/lib/active_record/validations.rb | 3 +- activesupport/lib/active_support/callbacks.rb | 25 ++++++++++----- activesupport/test/callbacks_test.rb | 31 +++++++++++++++++++ 6 files changed, 54 insertions(+), 16 deletions(-) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 30db7d9f73d1a..99e1f74c0affd 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -22,7 +22,7 @@ def dispatch(cgi = nil, session_options = CgiRequest::DEFAULT_SESSION_OPTIONS, o def to_prepare(identifier = nil, &block) @prepare_dispatch_callbacks ||= ActiveSupport::Callbacks::CallbackChain.new callback = ActiveSupport::Callbacks::Callback.new(:prepare_dispatch, block, :identifier => identifier) - @prepare_dispatch_callbacks.replace_or_append_callback(callback) + @prepare_dispatch_callbacks | callback end # If the block raises, send status code as a last-ditch response. diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 73721cd1ec26a..8c977877413d5 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -265,7 +265,7 @@ def create_filters(filters, filter_type, &block) def skip_filter_in_chain(*filters, &test) filters, conditions = extract_options(filters) filters.each do |filter| - if callback = find_callback(filter) then delete(callback) end + if callback = find(filter) then delete(callback) end end if conditions.empty? update_filter_in_chain(filters, :skip => conditions, &test) end @@ -302,7 +302,7 @@ def find_filter_prepend_position(filters, filter_type) def find_or_create_filter(filter, filter_type, options = {}) update_filter_in_chain([filter], options) - if found_filter = find_callback(filter) { |f| f.type == filter_type } + if found_filter = find(filter) { |f| f.type == filter_type } found_filter else filter_kind = case @@ -326,7 +326,7 @@ def find_or_create_filter(filter, filter_type, options = {}) end def update_filter_in_chain(filters, options, &test) - filters.map! { |f| block_given? ? find_callback(f, &test) : find_callback(f) } + filters.map! { |f| block_given? ? find(f, &test) : find(f) } filters.compact! map! do |filter| diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 503fb10795e20..3b7b9050bed9e 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -7,8 +7,7 @@ def self.included(base) # :nodoc: %w( validate validate_on_create validate_on_update ).each do |validation_method| base.class_eval <<-"end_eval" def self.#{validation_method}(*methods, &block) - methods = CallbackChain.build(:#{validation_method}, *methods, &block) - self.#{validation_method}_callback_chain.replace(#{validation_method}_callback_chain | methods) + self.#{validation_method}_callback_chain | CallbackChain.build(:#{validation_method}, *methods, &block) end def self.#{validation_method}_callback_chain diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 86cda9aa66237..7b70f78405af9 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -285,8 +285,7 @@ def self.included(base) # :nodoc: VALIDATIONS.each do |validation_method| base.class_eval <<-"end_eval" def self.#{validation_method}(*methods, &block) - methods = CallbackChain.build(:#{validation_method}, *methods, &block) - self.#{validation_method}_callback_chain.replace(#{validation_method}_callback_chain | methods) + self.#{validation_method}_callback_chain | CallbackChain.build(:#{validation_method}, *methods, &block) end def self.#{validation_method}_callback_chain diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 329cc2fdc72f6..282b6cbcbc3ed 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -96,17 +96,26 @@ def run(object, options = {}, &terminator) end end - def find_callback(callback, &block) + def |(chain) + if chain.is_a?(Callback) + if found_callback = find(chain) + index = index(found_callback) + self[index] = chain + else + self << chain + end + else + chain.each { |callback| self | callback } + end + self + end + + def find(callback, &block) select { |c| c == callback && (!block_given? || yield(c)) }.first end - def replace_or_append_callback(callback) - if found_callback = find_callback(callback) - index = index(found_callback) - self[index] = callback - else - self << callback - end + def delete(callback) + super(find(callback)) end private diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 3f8cb7f01a974..7f71ca226267a 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -96,6 +96,8 @@ def test_save_conditional_person end class CallbackTest < Test::Unit::TestCase + include ActiveSupport::Callbacks + def test_eql callback = Callback.new(:before, :save, :identifier => :lifesaver) assert callback.eql?(Callback.new(:before, :save, :identifier => :lifesaver)) @@ -115,3 +117,32 @@ def test_dup assert_equal({}, a.options) end end + +class CallbackChainTest < Test::Unit::TestCase + include ActiveSupport::Callbacks + + def setup + @chain = CallbackChain.build(:make, :bacon, :lettuce, :tomato) + end + + def test_build + assert_equal 3, @chain.size + assert_equal [:bacon, :lettuce, :tomato], @chain.map(&:method) + end + + def test_find + assert_equal :bacon, @chain.find(:bacon).method + end + + def test_union + assert_equal [:bacon, :lettuce, :tomato], (@chain | Callback.new(:make, :bacon)).map(&:method) + assert_equal [:bacon, :lettuce, :tomato, :turkey], (@chain | CallbackChain.build(:make, :bacon, :lettuce, :tomato, :turkey)).map(&:method) + assert_equal [:bacon, :lettuce, :tomato, :turkey, :mayo], (@chain | Callback.new(:make, :mayo)).map(&:method) + end + + def test_delete + assert_equal [:bacon, :lettuce, :tomato], @chain.map(&:method) + @chain.delete(:bacon) + assert_equal [:lettuce, :tomato], @chain.map(&:method) + end +end