Merge branch 'master' into normalizecb

* master:
  extract getting an setting callbacks to methods
  call extracted method from callback manipulation methods
  extract callback param munging
  no reason to make these defaulted, we always pass values in

Conflicts:
	activesupport/lib/active_support/callbacks.rb

1 files changed, 34 insertions, 20 deletions

diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 4819fe71de..2eec360042 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -411,16 +411,19 @@ module ActiveSupport
 
     module ClassMethods
 
-      # This is used internally to append, prepend and skip callbacks to the
-      # CallbackChain.
-      def __update_callbacks(name, filters = [], block = nil) #:nodoc:
+      def normalize_callback_params(name, filters, block) # :nodoc:
         type = CALLBACK_FILTER_TYPES.include?(filters.first) ? filters.shift : :before
         options = filters.last.is_a?(Hash) ? filters.pop : {}
         filters.unshift(block) if block
+        [type, filters, options]
+      end
 
+      # This is used internally to append, prepend and skip callbacks to the
+      # CallbackChain.
+      def __update_callbacks(name) #:nodoc:
         ([self] + ActiveSupport::DescendantsTracker.descendants(self)).reverse.each do |target|
-          chain = target.send("_#{name}_callbacks")
-          yield target, chain.dup, type, filters, options
+          chain = target.get_callbacks name
+          yield target, chain.dup
         end
       end
 
@@ -460,16 +463,15 @@ module ActiveSupport
       # * <tt>:prepend</tt> - If +true+, the callback will be prepended to the
       #   existing chain rather than appended.
       def set_callback(name, *filter_list, &block)
-        mapped = nil
-
-        __update_callbacks(name, filter_list, block) do |target, chain, type, filters, options|
-          mapped ||= filters.map do |filter|
-            Callback.build(chain, filter, type, options.dup)
-          end
+        type, filters, options = normalize_callback_params(name, filter_list, block)
+        chain = get_callbacks name
+        mapped = filters.map do |filter|
+          Callback.build(chain, filter, type, options.dup)
+        end
 
+        __update_callbacks(name) do |target, chain|
           options[:prepend] ? chain.prepend(*mapped) : chain.append(*mapped)
-
-          target.send("_#{name}_callbacks=", chain)
+          target.set_callbacks name, chain
         end
       end
 
@@ -481,7 +483,9 @@ module ActiveSupport
       #      skip_callback :validate, :before, :check_membership, if: -> { self.age > 18 }
       #   end
       def skip_callback(name, *filter_list, &block)
-        __update_callbacks(name, filter_list, block) do |target, chain, type, filters, options|
+        type, filters, options = normalize_callback_params(name, filter_list, block)
+
+        __update_callbacks(name) do |target, chain|
           filters.each do |filter|
             filter = chain.find {|c| c.matches?(type, filter) }
 
@@ -492,21 +496,21 @@ module ActiveSupport
 
             chain.delete(filter)
           end
-          target.send("_#{name}_callbacks=", chain)
+          target.set_callbacks name, chain
         end
       end
 
       # Remove all set callbacks for the given event.
       def reset_callbacks(symbol)
-        callbacks = send("_#{symbol}_callbacks")
+        callbacks = get_callbacks symbol
 
         ActiveSupport::DescendantsTracker.descendants(self).each do |target|
-          chain = target.send("_#{symbol}_callbacks").dup
+          chain = target.get_callbacks(symbol).dup
           callbacks.each { |c| chain.delete(c) }
-          target.send("_#{symbol}_callbacks=", chain)
+          target.set_callbacks symbol, chain
         end
 
-        self.send("_#{symbol}_callbacks=", callbacks.dup.clear)
+        self.set_callbacks symbol, callbacks.dup.clear
       end
 
       # Define sets of events in the object lifecycle that support callbacks.
@@ -580,9 +584,19 @@ module ActiveSupport
         config = callbacks.last.is_a?(Hash) ? callbacks.pop : {}
         callbacks.each do |callback|
           class_attribute "_#{callback}_callbacks"
-          send("_#{callback}_callbacks=", CallbackChain.new(callback, config))
+          set_callbacks callback, CallbackChain.new(callback, config)
         end
       end
+
+      protected
+
+      def get_callbacks(name)
+        send "_#{name}_callbacks"
+      end
+
+      def set_callbacks(name, callbacks)
+        send "_#{name}_callbacks=", callbacks
+      end
     end
   end
 end