Further cleaning up new callbacks

2026-04-26 03:00:59 -04:00 · 2009-06-02 21:41:31 -07:00
parent 971e2438d9
commit 1126a85aed
5 changed files with 97 additions and 117 deletions
--- a/actionpack/lib/action_controller/abstract/callbacks.rb
+++ b/actionpack/lib/action_controller/abstract/callbacks.rb
@@ -45,19 +45,19 @@ module AbstractController
        class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
          def #{filter}_filter(*names, &blk)
            _insert_callbacks(names, blk) do |name, options|
-              _set_callback(:process_action, :#{filter}, name, options)
+              set_callback(:process_action, :#{filter}, name, options)
            end
          end

          def prepend_#{filter}_filter(*names, &blk)
            _insert_callbacks(names, blk) do |name, options|
-              _set_callback(:process_action, :#{filter}, name, options.merge(:prepend => true))
+              set_callback(:process_action, :#{filter}, name, options.merge(:prepend => true))
            end
          end

          def skip_#{filter}_filter(*names, &blk)
            _insert_callbacks(names, blk) do |name, options|
-              _skip_callback(:process_action, :#{filter}, name, options)
+              skip_callback(:process_action, :#{filter}, name, options)
            end
          end

--- a/actionpack/test/abstract_controller/callbacks_test.rb
+++ b/actionpack/test/abstract_controller/callbacks_test.rb
@@ -8,7 +8,7 @@ module AbstractController
    end
    
    class Callback1 < ControllerWithCallbacks
-      _set_callback :process_action, :before, :first
+      set_callback :process_action, :before, :first
      
      def first
        @text = "Hello world"
--- a/activesupport/lib/active_support/new_callbacks.rb
+++ b/activesupport/lib/active_support/new_callbacks.rb
@@ -91,9 +91,8 @@ module ActiveSupport
      @@_callback_sequence = 0
      
      attr_accessor :filter, :kind, :name, :options, :per_key, :klass
-      def initialize(filter, kind, options, klass, name)
+      def initialize(filter, kind, options, klass)
        @kind, @klass = kind, klass
-        @name         = name
        
        normalize_options!(options)

@@ -131,9 +130,8 @@ module ActiveSupport
        @@_callback_sequence += 1
      end
      
-      def matches?(_kind, _name, _filter)
+      def matches?(_kind, _filter)
        @kind   == _kind &&
-        @name   == _name &&
        @filter == _filter
      end

@@ -310,40 +308,35 @@ module ActiveSupport
          RUBY_EVAL
          method_name
        else
-          kind, name = @kind, @name
+          kind = @kind
          @klass.send(:define_method, "#{method_name}_object") { filter }

-          if kind == :around
-            @klass.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
-              def #{method_name}(&blk)
-                if :#{kind} == :around && #{method_name}_object.respond_to?(:filter)
-                  #{method_name}_object.send("filter", self, &blk)
-                # TODO: Deprecate this
-                elsif #{method_name}_object.respond_to?(:before) && #{method_name}_object.respond_to?(:after)
-                  should_continue = #{method_name}_object.before(self)
-                  yield if should_continue
-                  #{method_name}_object.after(self)
-                else
-                  #{method_name}_object.send("#{kind}_#{name}", self, &blk)
-                end
-              end
-            RUBY_EVAL
-          else
-            @klass.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
-              def #{method_name}(&blk)
-                if #{method_name}_object.respond_to?(:#{kind})
-                  #{method_name}_object.#{kind}(self, &blk)
-                elsif #{method_name}_object.respond_to?(:filter)
-                  #{method_name}_object.send("filter", self, &blk)
-                else
-                  #{method_name}_object.send("#{kind}_#{name}", self, &blk)
-                end
-              end
-            RUBY_EVAL
-          end
+          _normalize_legacy_filter(kind, filter)
+
+          @klass.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
+            def #{method_name}(&blk)
+              #{method_name}_object.send(:#{kind}, self, &blk)
+            end
+          RUBY_EVAL
+
          method_name
        end
      end
+
+      def _normalize_legacy_filter(kind, filter)
+        if !filter.respond_to?(kind) && filter.respond_to?(:filter)
+          filter.metaclass.class_eval(
+            "def #{kind}(context, &block) filter(context, &block) end",
+            __FILE__, __LINE__ - 1)
+        elsif filter.respond_to?(:before) && filter.respond_to?(:after) && kind == :around
+          def filter.around(context)
+            should_continue = before(context)
+            yield if should_continue
+            after(context)
+          end
+        end
+      end
+
    end

    # An Array with a compile method
@@ -385,22 +378,19 @@ module ActiveSupport
      # The _run_save_callbacks method can optionally take a key, which
      # will be used to compile an optimized callback method for each
      # key. See #define_callbacks for more information.
-      def _define_runner(symbol, callbacks, options)
+      def _define_runner(symbol, callbacks)
        body = callbacks.compile(nil, :terminator => send("_#{symbol}_terminator"))

-        body = <<-RUBY_EVAL
-          def _run_#{symbol}_callbacks(key = nil)
+        body, line = <<-RUBY_EVAL, __LINE__
+          def _run_#{symbol}_callbacks(key = nil, &blk)
            if key
-              key = key.hash.to_s.gsub(/-/, '_')
-              name = "_run__\#{self.class.name.split("::").last}__#{symbol}__\#{key}__callbacks"
+              name = "_run__\#{self.class.name.hash.abs}__#{symbol}__\#{key.hash.abs}__callbacks"
              
-              if respond_to?(name)
-                send(name) { yield if block_given? }
-              else
-                self.class._create_and_run_keyed_callback(
-                  self.class.name.split("::").last,
-                  :#{symbol}, key, self) { yield if block_given? }
+              unless respond_to?(name)
+                self.class._create_keyed_callback(name, :#{symbol}, self, &blk)
              end
+
+              send(name, &blk)
            else
              #{body}
            end
@@ -408,30 +398,22 @@ module ActiveSupport
        RUBY_EVAL
  
        undef_method "_run_#{symbol}_callbacks" if method_defined?("_run_#{symbol}_callbacks")
-        class_eval body, __FILE__, __LINE__
-        
-        before_name, around_name, after_name = 
-          options.values_at(:before, :after, :around)
+        class_eval body, __FILE__, line
      end
      
      # This is called the first time a callback is called with a particular
      # key. It creates a new callback method for the key, calculating
      # which callbacks can be omitted because of per_key conditions.
-      def _create_and_run_keyed_callback(klass, kind, key, obj, &blk)
+      def _create_keyed_callback(name, kind, obj, &blk)
        @_keyed_callbacks ||= {}
-        @_keyed_callbacks[[kind, key]] ||= begin
-          str = self.send("_#{kind}_callbacks").compile(key, :object => obj, :terminator => self.send("_#{kind}_terminator"))
+        @_keyed_callbacks[name] ||= begin
+          str = send("_#{kind}_callbacks").
+            compile(name, :object => obj, :terminator => send("_#{kind}_terminator"))

-          self.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1
-            def _run__#{klass.split("::").last}__#{kind}__#{key}__callbacks
-              #{str}
-            end
-          RUBY_EVAL
+          class_eval "def #{name}() #{str} end", __FILE__, __LINE__
                    
          true
        end
-                                  
-        obj.send("_run__#{klass.split("::").last}__#{kind}__#{key}__callbacks", &blk)
      end
      
      # Define callbacks.
@@ -473,32 +455,30 @@ module ActiveSupport
        callbacks = send("_#{name}_callbacks")
        yield callbacks, type, filters, options if block_given?

-        _define_runner(name, callbacks, options)
+        _define_runner(name, callbacks)
      end

-      def _set_callback(name, *filters, &block)
+      def set_callback(name, *filters, &block)
        _update_callbacks(name, filters, block) do |callbacks, type, filters, options|        
          filters.map! do |filter|
            # overrides parent class
-            callbacks.delete_if do |c|
-              c.matches?(type, name, filter)
-            end
-            Callback.new(filter, type, options.dup, self, name)
+            callbacks.delete_if {|c| c.matches?(type, filter) }
+            Callback.new(filter, type, options.dup, self)
          end

          options[:prepend] ? callbacks.unshift(*filters) : callbacks.push(*filters)
        end
      end

-      def _skip_callback(name, *filters, &block)
+      def skip_callback(name, *filters, &block)
        _update_callbacks(name, filters, block) do |callbacks, type, filters, options|
          filters.each do |filter|
            callbacks = send("_#{name}_callbacks=", callbacks.clone(self))

-            filter = callbacks.find {|c| c.matches?(type, name, filter) }
-            per_key = options[:per_key] || {}
+            filter = callbacks.find {|c| c.matches?(type, filter) }
+
            if filter && options.any?
-              filter.recompile!(options, per_key)
+              filter.recompile!(options, options[:per_key] || {})
            else
              callbacks.delete(filter)
            end
@@ -520,7 +500,7 @@ module ActiveSupport
              _update_callbacks(:#{symbol})
            end
            
-            self._set_callback(:#{symbol}, :before)
+            self.set_callback(:#{symbol}, :before)
          RUBY_EVAL
        end
      end
--- a/activesupport/test/new_callback_inheritance_test.rb
+++ b/activesupport/test/new_callback_inheritance_test.rb
@@ -11,8 +11,8 @@ class GrandParent
  end
  
  define_callbacks :dispatch
-  _set_callback :dispatch, :before, :before1, :before2, :per_key => {:if => proc {|c| c.action_name == "index" || c.action_name == "update" }}
-  _set_callback :dispatch, :after, :after1, :after2, :per_key => {:if => proc {|c| c.action_name == "update" || c.action_name == "delete" }}  
+  set_callback :dispatch, :before, :before1, :before2, :per_key => {:if => proc {|c| c.action_name == "index" || c.action_name == "update" }}
+  set_callback :dispatch, :after, :after1, :after2, :per_key => {:if => proc {|c| c.action_name == "update" || c.action_name == "delete" }}  
  
  def before1
    @log << "before1"
@@ -39,12 +39,12 @@ class GrandParent
 end

 class Parent < GrandParent
-  _skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }}
-  _skip_callback :dispatch, :after, :after2, :per_key => {:unless => proc {|c| c.action_name == "delete" }}
+  skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }}
+  skip_callback :dispatch, :after, :after2, :per_key => {:unless => proc {|c| c.action_name == "delete" }}
 end

 class Child < GrandParent
-  _skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }}, :if => :state_open?
+  skip_callback :dispatch, :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }}, :if => :state_open?
  
  def state_open?
    @state == :open
--- a/activesupport/test/new_callbacks_test.rb
+++ b/activesupport/test/new_callbacks_test.rb
@@ -10,11 +10,11 @@ module NewCallbacksTest
    define_callbacks :save
  
    def self.before_save(*filters, &blk)
-      _set_callback(:save, :before, *filters, &blk)
+      set_callback(:save, :before, *filters, &blk)
    end
  
    def self.after_save(*filters, &blk)
-      _set_callback(:save, :after, *filters, &blk)
+      set_callback(:save, :after, *filters, &blk)
    end

    class << self
@@ -37,7 +37,7 @@ module NewCallbacksTest
      def callback_object(callback_method)
        klass = Class.new
        klass.send(:define_method, callback_method) do |model|
-          model.history << [callback_method, :object]
+          model.history << [:"#{callback_method}_save", :object]
        end
        klass.new
      end
@@ -54,7 +54,7 @@ module NewCallbacksTest
      send(callback_method, callback_symbol(callback_method_sym))
      send(callback_method, callback_string(callback_method_sym))
      send(callback_method, callback_proc(callback_method_sym))
-      send(callback_method, callback_object(callback_method_sym))
+      send(callback_method, callback_object(callback_method_sym.to_s.gsub(/_save/, '')))
      send(callback_method) { |model| model.history << [callback_method_sym, :block] }
    end

@@ -64,10 +64,10 @@ module NewCallbacksTest
  end

  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, :before, :before_save_method, :unless => :no
+    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, :before, :before_save_method, :unless => :no
    def yes; true; end
    def no; false; end
  end
@@ -77,8 +77,8 @@ module NewCallbacksTest

    define_callbacks :dispatch
    
-    _set_callback :dispatch, :before, :log, :per_key => {:unless => proc {|c| c.action_name == :index || c.action_name == :show }}
-    _set_callback :dispatch, :after, :log2
+    set_callback :dispatch, :before, :log, :per_key => {:unless => proc {|c| c.action_name == :index || c.action_name == :show }}
+    set_callback :dispatch, :after, :log2
  
    attr_reader :action_name, :logger
    def initialize(action_name)
@@ -102,8 +102,8 @@ module NewCallbacksTest
  end

  class Child < ParentController
-    _skip_callback :dispatch, :before, :log, :per_key => {:if => proc {|c| c.action_name == :update} }
-    _skip_callback :dispatch, :after, :log2
+    skip_callback :dispatch, :before, :log, :per_key => {:if => proc {|c| c.action_name == :update} }
+    skip_callback :dispatch, :after, :log2
  end

  class OneTimeCompile < Record
@@ -188,19 +188,19 @@ module NewCallbacksTest
  class AroundPerson < MySuper
    attr_reader :history
  
-    _set_callback :save, :before, :nope,           :if =>     :no
-    _set_callback :save, :before, :nope,           :unless => :yes
-    _set_callback :save, :after,  :tweedle
-    _set_callback :save, :before, "tweedle_dee"
-    _set_callback :save, :before, proc {|m| m.history << "yup" }
-    _set_callback :save, :before, :nope,           :if =>     proc { false }
-    _set_callback :save, :before, :nope,           :unless => proc { true }
-    _set_callback :save, :before, :yup,            :if =>     proc { true }
-    _set_callback :save, :before, :yup,            :unless => proc { false }
-    _set_callback :save, :around, :tweedle_dum
-    _set_callback :save, :around, :w0tyes,         :if =>     :yes
-    _set_callback :save, :around, :w0tno,          :if =>     :no
-    _set_callback :save, :around, :tweedle_deedle
+    set_callback :save, :before, :nope,           :if =>     :no
+    set_callback :save, :before, :nope,           :unless => :yes
+    set_callback :save, :after,  :tweedle
+    set_callback :save, :before, "tweedle_dee"
+    set_callback :save, :before, proc {|m| m.history << "yup" }
+    set_callback :save, :before, :nope,           :if =>     proc { false }
+    set_callback :save, :before, :nope,           :unless => proc { true }
+    set_callback :save, :before, :yup,            :if =>     proc { true }
+    set_callback :save, :before, :yup,            :unless => proc { false }
+    set_callback :save, :around, :tweedle_dum
+    set_callback :save, :around, :w0tyes,         :if =>     :yes
+    set_callback :save, :around, :w0tno,          :if =>     :no
+    set_callback :save, :around, :tweedle_deedle
  
    def no; false; end
    def yes; true; end
@@ -260,7 +260,7 @@ module NewCallbacksTest
    define_callbacks :save
    attr_reader :stuff
    
-    _set_callback :save, :before, :omg, :per_key => {:if => :yes}
+    set_callback :save, :before, :omg, :per_key => {:if => :yes}
    
    def yes() true end
      
@@ -354,15 +354,15 @@ module NewCallbacksTest
  
    define_callbacks :save, "result == :halt"
  
-    _set_callback :save, :before, :first
-    _set_callback :save, :before, :second
-    _set_callback :save, :around, :around_it  
-    _set_callback :save, :before, :third
-    _set_callback :save, :after, :first
-    _set_callback :save, :around, :around_it
-    _set_callback :save, :after, :second
-    _set_callback :save, :around, :around_it
-    _set_callback :save, :after, :third
+    set_callback :save, :before, :first
+    set_callback :save, :before, :second
+    set_callback :save, :around, :around_it  
+    set_callback :save, :before, :third
+    set_callback :save, :after, :first
+    set_callback :save, :around, :around_it
+    set_callback :save, :after, :second
+    set_callback :save, :around, :around_it
+    set_callback :save, :after, :third

  
    attr_reader :history, :saved
@@ -397,11 +397,11 @@ module NewCallbacksTest
  end

  class CallbackObject
-    def before_save(caller)
+    def before(caller)
      caller.record << "before"
    end
    
-    def around_save(caller)
+    def around(caller)
      caller.record << "around before"
      yield
      caller.record << "around after"
@@ -412,7 +412,7 @@ module NewCallbacksTest
    include ActiveSupport::NewCallbacks
    
    define_callbacks :save
-    _set_callback :save, :before, CallbackObject.new
+    set_callback :save, :before, CallbackObject.new
    
    attr_accessor :record
    def initialize
@@ -430,7 +430,7 @@ module NewCallbacksTest
    include ActiveSupport::NewCallbacks
    
    define_callbacks :save
-    _set_callback :save, :around, CallbackObject.new
+    set_callback :save, :around, CallbackObject.new
    
    attr_accessor :record
    def initialize