Skip to content

Commit

Permalink
Simplify callbacks to use less metaprogramming
Browse files Browse the repository at this point in the history
  • Loading branch information
Yehuda Katz committed Jun 3, 2009
1 parent 196f780 commit 971e243
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 109 deletions.
30 changes: 15 additions & 15 deletions actionpack/lib/action_controller/abstract/callbacks.rb
View file
Expand Up @@ -32,32 +32,32 @@ def skip_filter(*names, &blk)
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
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/abstract_controller/callbacks_test.rb
View file
Expand Up @@ -8,7 +8,7 @@ class ControllerWithCallbacks < AbstractController::Base
end

class Callback1 < ControllerWithCallbacks
process_action_callback :before, :first
_set_callback :process_action, :before, :first

def first
@text = "Hello world"
Expand Down
6 changes: 4 additions & 2 deletions activesupport/lib/active_support/core_ext/class/inheritable_attributes.rb
View file
Expand Up @@ -202,6 +202,8 @@ def self.#{ivar}=(obj)
def #{ivar}=(obj) self.class.#{ivar} = obj end
RUBY
end

self.send("#{ivar}=", yield) if block_given?
end
end

Expand All @@ -214,8 +216,8 @@ def #{ivar}=(obj) self.class.#{ivar} = obj end
# @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
106 changes: 55 additions & 51 deletions activesupport/lib/active_support/new_callbacks.rb
View file
Expand Up @@ -385,8 +385,10 @@ module ClassMethods
# 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(/-/, '_')
Expand All @@ -400,13 +402,13 @@ def _run_#{symbol}_callbacks(key = nil)
:#{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)
Expand Down Expand Up @@ -463,60 +465,62 @@ def _run__#{klass.split("::").last}__#{kind}__#{key}__callbacks
# 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
Expand Down
10 changes: 5 additions & 5 deletions activesupport/test/new_callback_inheritance_test.rb
View file
Expand Up @@ -11,8 +11,8 @@ def initialize(action_name)
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"
Expand All @@ -39,12 +39,12 @@ def dispatch
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
Expand Down
70 changes: 35 additions & 35 deletions activesupport/test/new_callbacks_test.rb
View file
Expand Up @@ -10,11 +10,11 @@ class Record
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
Expand Down Expand Up @@ -64,10 +64,10 @@ def save
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
Expand All @@ -77,8 +77,8 @@ class ParentController

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)
Expand All @@ -102,8 +102,8 @@ def dispatch
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
Expand Down Expand Up @@ -188,19 +188,19 @@ class MySuper
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
Expand Down Expand Up @@ -260,7 +260,7 @@ class HyphenatedCallbacks
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

Expand Down Expand Up @@ -354,15 +354,15 @@ class CallbackTerminator

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
Expand Down Expand Up @@ -412,7 +412,7 @@ class UsingObjectBefore
include ActiveSupport::NewCallbacks

define_callbacks :save
save_callback :before, CallbackObject.new
_set_callback :save, :before, CallbackObject.new

attr_accessor :record
def initialize
Expand All @@ -430,7 +430,7 @@ class UsingObjectAround
include ActiveSupport::NewCallbacks

define_callbacks :save
save_callback :around, CallbackObject.new
_set_callback :save, :around, CallbackObject.new

attr_accessor :record
def initialize
Expand Down

0 comments on commit 971e243

Please sign in to comment.