@@ -32,32 +32,32 @@ module AbstractController skip_around_filter(*names, &blk) end + def _insert_callbacks(names, block) + options = names.last.is_a?(Hash) ? names.pop : {} + _normalize_callback_options(options) + names.push(block) if block + names.each do |name| + yield name, options + end + end + [:before, :after, :around].each do |filter| class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{filter}_filter(*names, &blk) - options = names.last.is_a?(Hash) ? names.pop : {} - _normalize_callback_options(options) - names.push(blk) if block_given? - names.each do |name| - process_action_callback(:#{filter}, name, options) + _insert_callbacks(names, blk) do |name, options| + _set_callback(:process_action, :#{filter}, name, options) end end def prepend_#{filter}_filter(*names, &blk) - options = names.last.is_a?(Hash) ? names.pop : {} - _normalize_callback_options(options) - names.push(blk) if block_given? - names.each do |name| - process_action_callback(:#{filter}, name, options.merge(:prepend => true)) + _insert_callbacks(names, blk) do |name, options| + _set_callback(:process_action, :#{filter}, name, options.merge(:prepend => true)) end end def skip_#{filter}_filter(*names, &blk) - options = names.last.is_a?(Hash) ? names.pop : {} - _normalize_callback_options(options) - names.push(blk) if block_given? - names.each do |name| - skip_process_action_callback(:#{filter}, name, options) + _insert_callbacks(names, blk) do |name, options| + _skip_callback(:process_action, :#{filter}, name, options) end end actionpack/lib/action_controller/abstract/callbacks.rb @@ -8,7 +8,7 @@ module AbstractController end class Callback1 < ControllerWithCallbacks - process_action_callback :before, :first + _set_callback :process_action, :before, :first def first @text = "Hello world" actionpack/test/abstract_controller/callbacks_test.rb @@ -202,6 +202,8 @@ class Class def #{ivar}=(obj) self.class.#{ivar} = obj end RUBY end + + self.send("#{ivar}=", yield) if block_given? end end @@ -214,8 +216,8 @@ class Class # @return <Array[#to_s]> An Array of attributes turned into inheritable accessors. # # @api public - def extlib_inheritable_accessor(*syms) + def extlib_inheritable_accessor(*syms, &block) extlib_inheritable_reader(*syms) - extlib_inheritable_writer(*syms) + extlib_inheritable_writer(*syms, &block) end end activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb @@ -385,8 +385,10 @@ 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, str, options) - str = <<-RUBY_EVAL + def _define_runner(symbol, callbacks, options) + body = callbacks.compile(nil, :terminator => send("_#{symbol}_terminator")) + + body = <<-RUBY_EVAL def _run_#{symbol}_callbacks(key = nil) if key key = key.hash.to_s.gsub(/-/, '_') @@ -400,13 +402,13 @@ module ActiveSupport :#{symbol}, key, self) { yield if block_given? } end else - #{str} + #{body} end end RUBY_EVAL undef_method "_run_#{symbol}_callbacks" if method_defined?("_run_#{symbol}_callbacks") - class_eval str, __FILE__, __LINE__ + class_eval body, __FILE__, __LINE__ before_name, around_name, after_name = options.values_at(:before, :after, :around) @@ -463,60 +465,62 @@ module ActiveSupport # In that case, each action_name would get its own compiled callback # method that took into consideration the per_key conditions. This # is a speed improvement for ActionPack. + def _update_callbacks(name, filters = CallbackChain.new(name), block = nil) + type = [:before, :after, :around].include?(filters.first) ? filters.shift : :before + options = filters.last.is_a?(Hash) ? filters.pop : {} + filters.unshift(block) if block + + callbacks = send("_#{name}_callbacks") + yield callbacks, type, filters, options if block_given? + + _define_runner(name, callbacks, options) + end + + 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) + end + + options[:prepend] ? callbacks.unshift(*filters) : callbacks.push(*filters) + end + end + + 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] || {} + if filter && options.any? + filter.recompile!(options, per_key) + else + callbacks.delete(filter) + end + end + end + end + def define_callbacks(*symbols) terminator = symbols.pop if symbols.last.is_a?(String) symbols.each do |symbol| - self.extlib_inheritable_accessor("_#{symbol}_terminator") - self.send("_#{symbol}_terminator=", terminator) - self.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 - extlib_inheritable_accessor :_#{symbol}_callbacks - self._#{symbol}_callbacks = CallbackChain.new(:#{symbol}) + extlib_inheritable_accessor("_#{symbol}_terminator") { terminator } - def self.#{symbol}_callback(*filters, &blk) - type = [:before, :after, :around].include?(filters.first) ? filters.shift : :before - options = filters.last.is_a?(Hash) ? filters.pop : {} - filters.unshift(blk) if block_given? - - filters.map! do |filter| - # overrides parent class - self._#{symbol}_callbacks.delete_if {|c| c.matches?(type, :#{symbol}, filter)} - Callback.new(filter, type, options.dup, self, :#{symbol}) - end - options[:prepend] ? - self._#{symbol}_callbacks.unshift(*filters) : - self._#{symbol}_callbacks.push(*filters) - _define_runner(:#{symbol}, - self._#{symbol}_callbacks.compile(nil, :terminator => _#{symbol}_terminator), - options) - end - - def self.skip_#{symbol}_callback(*filters, &blk) - type = [:before, :after, :around].include?(filters.first) ? filters.shift : :before - options = filters.last.is_a?(Hash) ? filters.pop : {} - filters.unshift(blk) if block_given? - filters.each do |filter| - self._#{symbol}_callbacks = self._#{symbol}_callbacks.clone(self) - - filter = self._#{symbol}_callbacks.find {|c| c.matches?(type, :#{symbol}, filter) } - per_key = options[:per_key] || {} - if filter && options.any? - filter.recompile!(options, per_key) - else - self._#{symbol}_callbacks.delete(filter) - end - _define_runner(:#{symbol}, - self._#{symbol}_callbacks.compile(nil, :terminator => _#{symbol}_terminator), - options) - end - - end - + extlib_inheritable_accessor("_#{symbol}_callbacks") do + CallbackChain.new(symbol) + end + + self.class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def self.reset_#{symbol}_callbacks - self._#{symbol}_callbacks = CallbackChain.new(:#{symbol}) - _define_runner(:#{symbol}, self._#{symbol}_callbacks.compile, {}) + _update_callbacks(:#{symbol}) end - self.#{symbol}_callback(:before) + self._set_callback(:#{symbol}, :before) RUBY_EVAL end end activesupport/lib/active_support/new_callbacks.rb @@ -11,8 +11,8 @@ class GrandParent end define_callbacks :dispatch - dispatch_callback :before, :before1, :before2, :per_key => {:if => proc {|c| c.action_name == "index" || c.action_name == "update" }} - dispatch_callback :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_dispatch_callback :before, :before2, :per_key => {:unless => proc {|c| c.action_name == "update" }} - skip_dispatch_callback :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_dispatch_callback :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 activesupport/test/new_callback_inheritance_test.rb @@ -10,11 +10,11 @@ module NewCallbacksTest define_callbacks :save def self.before_save(*filters, &blk) - save_callback(:before, *filters, &blk) + _set_callback(:save, :before, *filters, &blk) end def self.after_save(*filters, &blk) - save_callback(:after, *filters, &blk) + _set_callback(:save, :after, *filters, &blk) end class << self @@ -64,10 +64,10 @@ module NewCallbacksTest end class PersonSkipper < Person - skip_save_callback :before, :before_save_method, :if => :yes - skip_save_callback :after, :before_save_method, :unless => :yes - skip_save_callback :after, :before_save_method, :if => :no - skip_save_callback :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 - dispatch_callback :before, :log, :per_key => {:unless => proc {|c| c.action_name == :index || c.action_name == :show }} - dispatch_callback :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_dispatch_callback :before, :log, :per_key => {:if => proc {|c| c.action_name == :update} } - skip_dispatch_callback :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 - save_callback :before, :nope, :if => :no - save_callback :before, :nope, :unless => :yes - save_callback :after, :tweedle - save_callback :before, "tweedle_dee" - save_callback :before, proc {|m| m.history << "yup" } - save_callback :before, :nope, :if => proc { false } - save_callback :before, :nope, :unless => proc { true } - save_callback :before, :yup, :if => proc { true } - save_callback :before, :yup, :unless => proc { false } - save_callback :around, :tweedle_dum - save_callback :around, :w0tyes, :if => :yes - save_callback :around, :w0tno, :if => :no - save_callback :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 - save_callback :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" - save_callback :before, :first - save_callback :before, :second - save_callback :around, :around_it - save_callback :before, :third - save_callback :after, :first - save_callback :around, :around_it - save_callback :after, :second - save_callback :around, :around_it - save_callback :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 @@ -412,7 +412,7 @@ module NewCallbacksTest include ActiveSupport::NewCallbacks define_callbacks :save - save_callback :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 - save_callback :around, CallbackObject.new + _set_callback :save, :around, CallbackObject.new attr_accessor :record def initialize activesupport/test/new_callbacks_test.rb 196f780e30fcece25e4d09c12f9b9f7374ebed29 Yehuda Katz wycats@yehuda-katzs-macbookpro41.local http://github.com/lifo/docrails/commit/971e2438d98326c994ec6d3ef8e37b7e868ed6e2 971e2438d98326c994ec6d3ef8e37b7e868ed6e2 2009-06-02T19:00:59-07:00 2009-06-02T19:00:59-07:00 Simplify callbacks to use less metaprogramming 3a746ec6e845b7956775055a1a2453bd46eb6eb4 Yehuda Katz wycats@yehuda-katzs-macbookpro41.local