diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb
index 4f1094e497..6a02f602e4 100644
--- a/actionpack/lib/action_controller/dispatcher.rb
+++ b/actionpack/lib/action_controller/dispatcher.rb
@@ -11,20 +11,6 @@ def dispatch(cgi = nil, session_options = CgiRequest::DEFAULT_SESSION_OPTIONS, o
new(output).dispatch_cgi(cgi, session_options)
end
- # Declare a block to be called before each dispatch.
- # Run in the order declared.
- def before_dispatch(*method_names, &block)
- callbacks[:before].concat method_names
- callbacks[:before] << block if block_given?
- end
-
- # Declare a block to be called after each dispatch.
- # Run in reverse of the order declared.
- def after_dispatch(*method_names, &block)
- callbacks[:after].concat method_names
- callbacks[:after] << block if block_given?
- end
-
# Add a preparation callback. Preparation callbacks are run before every
# request in development mode, and before the first request in production
# mode.
@@ -34,15 +20,16 @@ def after_dispatch(*method_names, &block)
# existing callback. Passing an identifier is a suggested practice if the
# code adding a preparation block may be reloaded.
def to_prepare(identifier = nil, &block)
+ @prepare_dispatch_callbacks ||= []
+ callback = ActiveSupport::Callbacks::Callback.new(:prepare_dispatch, block, :identifier => identifier)
+
# Already registered: update the existing callback
- if identifier
- if callback = callbacks[:prepare].assoc(identifier)
- callback[1] = block
- else
- callbacks[:prepare] << [identifier, block]
- end
+ # TODO: Ruby one liner for Array#find returning index
+ if identifier && callback_for_identifier = @prepare_dispatch_callbacks.find { |c| c.identifier == identifier }
+ index = @prepare_dispatch_callbacks.index(callback_for_identifier)
+ @prepare_dispatch_callbacks[index] = callback
else
- callbacks[:prepare] << block
+ @prepare_dispatch_callbacks.concat([callback])
end
end
@@ -90,12 +77,11 @@ def failsafe_logger
cattr_accessor :error_file_path
self.error_file_path = "#{::RAILS_ROOT}/public" if defined? ::RAILS_ROOT
- cattr_accessor :callbacks
- self.callbacks = Hash.new { |h, k| h[k] = [] }
-
cattr_accessor :unprepared
self.unprepared = true
+ include ActiveSupport::Callbacks
+ define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch
before_dispatch :reload_application
before_dispatch :prepare_application
@@ -115,12 +101,12 @@ def initialize(output, request = nil, response = nil)
def dispatch
@@guard.synchronize do
begin
- run_callbacks :before
+ run_callbacks :before_dispatch
handle_request
rescue Exception => exception
failsafe_rescue exception
ensure
- run_callbacks :after, :reverse_each
+ run_callbacks :after_dispatch, :enumerator => :reverse_each
end
end
end
@@ -152,7 +138,7 @@ def prepare_application(force = false)
ActiveRecord::Base.verify_active_connections! if defined?(ActiveRecord)
if unprepared || force
- run_callbacks :prepare
+ run_callbacks :prepare_dispatch
self.unprepared = false
end
end
@@ -177,17 +163,6 @@ def handle_request
@controller.process(@request, @response).out(@output)
end
- def run_callbacks(kind, enumerator = :each)
- callbacks[kind].send!(enumerator) do |callback|
- case callback
- when Proc; callback.call(self)
- when String, Symbol; send!(callback)
- when Array; callback[1].call(self)
- else raise ArgumentError, "Unrecognized callback #{callback.inspect}"
- end
- end
- end
-
def failsafe_rescue(exception)
self.class.failsafe_response(@output, '500 Internal Server Error', exception) do
if @controller ||= defined?(::ApplicationController) ? ::ApplicationController : Base
diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb
index c174e51189..fe9789c698 100644
--- a/actionpack/test/controller/dispatcher_test.rb
+++ b/actionpack/test/controller/dispatcher_test.rb
@@ -11,7 +11,7 @@ def setup
@output = StringIO.new
ENV['REQUEST_METHOD'] = 'GET'
- Dispatcher.callbacks[:prepare].clear
+ Dispatcher.instance_variable_set("@prepare_dispatch_callbacks", [])
@dispatcher = Dispatcher.new(@output)
end
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 9a9bf28323..67a4117d20 100755
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -183,14 +183,8 @@ def self.included(base) #:nodoc:
base.send :alias_method_chain, method, :callbacks
end
- CALLBACKS.each do |method|
- base.class_eval <<-"end_eval"
- def self.#{method}(*callbacks, &block)
- callbacks << block if block_given?
- write_inheritable_array(#{method.to_sym.inspect}, callbacks)
- end
- end_eval
- end
+ base.send :include, ActiveSupport::Callbacks
+ base.define_callbacks *CALLBACKS
end
# Is called when the object was instantiated by one of the finders, like Base.find.
@@ -301,38 +295,15 @@ def destroy_with_callbacks #:nodoc:
def callback(method)
notify(method)
- callbacks_for(method).each do |callback|
- result = case callback
- when Symbol
- self.send(callback)
- when String
- eval(callback, binding)
- when Proc, Method
- callback.call(self)
- else
- if callback.respond_to?(method)
- callback.send(method, self)
- else
- raise ActiveRecordError, "Callbacks must be a symbol denoting the method to call, a string to be evaluated, a block to be invoked, or an object responding to the callback method."
- end
- end
- return false if result == false
+ result = run_callbacks(method) { |result, object| result == false }
+
+ if result != false && respond_to_without_attributes?(method)
+ result = send(method)
end
- result = send(method) if respond_to_without_attributes?(method)
-
return result
end
- def callbacks_for(method)
- self.class.read_inheritable_attribute(method.to_sym) or []
- end
-
- def invoke_and_notify(method)
- notify(method)
- send(method) if respond_to_without_attributes?(method)
- end
-
def notify(method) #:nodoc:
self.class.changed
self.class.notify_observers(method, self)
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 98dfcece8b..87f128e9a3 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -279,6 +279,25 @@ def self.included(base) # :nodoc:
alias_method_chain :save!, :validation
alias_method_chain :update_attribute, :validation_skipping
end
+
+ base.send :include, ActiveSupport::Callbacks
+
+ # TODO: Use helper ActiveSupport::Callbacks#define_callbacks instead
+ %w( validate validate_on_create validate_on_update ).each do |validation_method|
+ base.class_eval <<-"end_eval"
+ def self.#{validation_method}(*methods, &block)
+ options = methods.extract_options!
+ methods << block if block_given?
+ methods.map! { |method| Callback.new(:#{validation_method}, method, options) }
+ existing_methods = read_inheritable_attribute(:#{validation_method}) || []
+ write_inheritable_attribute(:#{validation_method}, existing_methods | methods)
+ end
+
+ def self.#{validation_method}_callback_chain
+ read_inheritable_attribute(:#{validation_method}) || []
+ end
+ end_eval
+ end
end
# All of the following validations are defined in the class scope of the model that you're interested in validating.
@@ -324,43 +343,6 @@ module ClassMethods
# end
#
# This usage applies to #validate_on_create and #validate_on_update as well.
- def validate(*methods, &block)
- methods << block if block_given?
- write_inheritable_set(:validate, methods)
- end
-
- def validate_on_create(*methods, &block)
- methods << block if block_given?
- write_inheritable_set(:validate_on_create, methods)
- end
-
- def validate_on_update(*methods, &block)
- methods << block if block_given?
- write_inheritable_set(:validate_on_update, methods)
- end
-
- def condition_block?(condition)
- condition.respond_to?("call") && (condition.arity == 1 || condition.arity == -1)
- end
-
- # Determine from the given condition (whether a block, procedure, method or string)
- # whether or not to validate the record. See #validates_each.
- def evaluate_condition(condition, record)
- case condition
- when Symbol; record.send(condition)
- when String; eval(condition, record.instance_eval { binding })
- else
- if condition_block?(condition)
- condition.call(record)
- else
- raise(
- ActiveRecordError,
- "Validations need to be either a symbol, string (to be eval'ed), proc/method, or " +
- "class implementing a static validation method"
- )
- end
- end
- end
# Validates each attribute against a block.
#
@@ -379,20 +361,17 @@ def evaluate_condition(condition, record)
# method, proc or string should return or evaluate to a true or false value.
# * unless - Specifies a method, proc or string to call to determine if the validation should
# not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The
- # method, proc or string should return or evaluate to a true or false value.
+ # method, proc or string should return or evaluate to a true or false value.
def validates_each(*attrs)
options = attrs.extract_options!.symbolize_keys
attrs = attrs.flatten
# Declare the validation.
- send(validation_method(options[:on] || :save)) do |record|
- # Don't validate when there is an :if condition and that condition is false or there is an :unless condition and that condition is true
- unless (options[:if] && !evaluate_condition(options[:if], record)) || (options[:unless] && evaluate_condition(options[:unless], record))
- attrs.each do |attr|
- value = record.send(attr)
- next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])
- yield record, attr, value
- end
+ send(validation_method(options[:on] || :save), options) do |record|
+ attrs.each do |attr|
+ value = record.send(attr)
+ next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])
+ yield record, attr, value
end
end
end
@@ -515,11 +494,9 @@ def validates_presence_of(*attr_names)
# can't use validates_each here, because it cannot cope with nonexistent attributes,
# while errors.add_on_empty can
- send(validation_method(configuration[:on])) do |record|
- unless (configuration[:if] && !evaluate_condition(configuration[:if], record)) || (configuration[:unless] && evaluate_condition(configuration[:unless], record))
- record.errors.add_on_blank(attr_names, configuration[:message])
- end
- end
+ send(validation_method(configuration[:on]), configuration) do |record|
+ record.errors.add_on_blank(attr_names, configuration[:message])
+ end
end
# Validates that the specified attribute matches the length restrictions supplied. Only one option can be used at a time:
@@ -911,13 +888,7 @@ def create!(attributes = nil)
end
end
-
private
- def write_inheritable_set(key, methods)
- existing_methods = read_inheritable_attribute(key) || []
- write_inheritable_attribute(key, existing_methods | methods)
- end
-
def validation_method(on)
case on
when :save then :validate
@@ -959,14 +930,14 @@ def update_attribute_with_validation_skipping(name, value)
def valid?
errors.clear
- run_validations(:validate)
+ run_callbacks(:validate)
validate
if new_record?
- run_validations(:validate_on_create)
+ run_callbacks(:validate_on_create)
validate_on_create
else
- run_validations(:validate_on_update)
+ run_callbacks(:validate_on_update)
validate_on_update
end
@@ -990,36 +961,5 @@ def validate_on_create #:doc:
# Overwrite this method for validation checks used only on updates.
def validate_on_update # :doc:
end
-
- private
- def run_validations(validation_method)
- validations = self.class.read_inheritable_attribute(validation_method.to_sym)
- if validations.nil? then return end
- validations.each do |validation|
- if validation.is_a?(Symbol)
- self.send(validation)
- elsif validation.is_a?(String)
- eval(validation, binding)
- elsif validation_block?(validation)
- validation.call(self)
- elsif validation_class?(validation, validation_method)
- validation.send(validation_method, self)
- else
- raise(
- ActiveRecordError,
- "Validations need to be either a symbol, string (to be eval'ed), proc/method, or " +
- "class implementing a static validation method"
- )
- end
- end
- end
-
- def validation_block?(validation)
- validation.respond_to?("call") && (validation.arity == 1 || validation.arity == -1)
- end
-
- def validation_class?(validation, validation_method)
- validation.respond_to?(validation_method)
- end
end
end
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index e48844ece7..07936a783d 100755
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -1017,7 +1017,7 @@ def test_validate_block
def test_invalid_validator
Topic.validate 3
- assert_raise(ActiveRecord::ActiveRecordError) { t = Topic.create }
+ assert_raise(ArgumentError) { t = Topic.create }
end
def test_throw_away_typing
diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG
index db46ce4f4d..5cd4a483e4 100644
--- a/activesupport/CHANGELOG
+++ b/activesupport/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Extract ActiveSupport::Callbacks from Active Record, test case setup and teardown, and ActionController::Dispatcher. #10727 [Josh Peek]
+
* Introducing DateTime #utc, #utc? and #utc_offset, for duck-typing compatibility with Time. Closes #10002 [Geoff Buesing]
* Time#to_json uses Numeric#to_utc_offset_s to output a cross-platform-consistent representation without having to convert to DateTime. References #9750 [Geoff Buesing]
diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb
index c4b7cc8cba..7a0476b729 100644
--- a/activesupport/lib/active_support.rb
+++ b/activesupport/lib/active_support.rb
@@ -26,6 +26,7 @@
require 'active_support/vendor'
require 'active_support/basic_object'
require 'active_support/inflector'
+require 'active_support/callbacks'
require 'active_support/core_ext'
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
new file mode 100644
index 0000000000..ac9f1a9d5f
--- /dev/null
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -0,0 +1,94 @@
+module ActiveSupport
+ module Callbacks
+ class Callback
+ def self.run(callbacks, object, options = {}, &terminator)
+ enumerator = options[:enumerator] || :each
+
+ unless block_given?
+ callbacks.send(enumerator) { |callback| callback.call(object) }
+ else
+ callbacks.send(enumerator) do |callback|
+ result = callback.call(object)
+ break result if terminator.call(result, object)
+ end
+ end
+ end
+
+ attr_reader :kind, :method, :identifier, :options
+
+ def initialize(kind, method, options = {})
+ @kind = kind
+ @method = method
+ @identifier = options[:identifier]
+ @options = options
+ end
+
+ def call(object)
+ evaluate_method(method, object) if should_run_callback?(object)
+ end
+
+ private
+ def evaluate_method(method, object)
+ case method
+ when Symbol
+ object.send(method)
+ when String
+ eval(method, object.instance_eval { binding })
+ when Proc, Method
+ method.call(object)
+ else
+ if method.respond_to?(kind)
+ method.send(kind, object)
+ else
+ raise ArgumentError,
+ "Callbacks must be a symbol denoting the method to call, a string to be evaluated, " +
+ "a block to be invoked, or an object responding to the callback method."
+ end
+ end
+ end
+
+ def should_run_callback?(object)
+ if options[:if]
+ evaluate_method(options[:if], object)
+ elsif options[:unless]
+ !evaluate_method(options[:unless], object)
+ else
+ true
+ end
+ end
+ end
+
+ def self.included(base)
+ base.extend ClassMethods
+ end
+
+ module ClassMethods
+ def define_callbacks(*callbacks)
+ callbacks.each do |callback|
+ class_eval <<-"end_eval"
+ def self.#{callback}(*methods, &block)
+ options = methods.extract_options!
+ methods << block if block_given?
+ callbacks = methods.map { |method| Callback.new(:#{callback}, method, options) }
+ (@#{callback}_callbacks ||= []).concat callbacks
+ end
+
+ def self.#{callback}_callback_chain
+ @#{callback}_callbacks ||= []
+
+ if superclass.respond_to?(:#{callback}_callback_chain)
+ superclass.#{callback}_callback_chain + @#{callback}_callbacks
+ else
+ @#{callback}_callbacks
+ end
+ end
+ end_eval
+ end
+ end
+ end
+
+ def run_callbacks(kind, options = {}, &block)
+ Callback.run(self.class.send("#{kind}_callback_chain"), self, options, &block)
+ end
+ end
+end
diff --git a/activesupport/lib/active_support/testing/setup_and_teardown.rb b/activesupport/lib/active_support/testing/setup_and_teardown.rb
index 1639462fae..b2a937edd0 100644
--- a/activesupport/lib/active_support/testing/setup_and_teardown.rb
+++ b/activesupport/lib/active_support/testing/setup_and_teardown.rb
@@ -2,7 +2,8 @@ module ActiveSupport
module Testing
module SetupAndTeardown
def self.included(base)
- base.extend ClassMethods
+ base.send :include, ActiveSupport::Callbacks
+ base.define_callbacks :setup, :teardown
begin
require 'mocha'
@@ -12,38 +13,6 @@ def self.included(base)
end
end
- module ClassMethods
- def setup(*method_names, &block)
- method_names << block if block_given?
- (@setup_callbacks ||= []).concat method_names
- end
-
- def teardown(*method_names, &block)
- method_names << block if block_given?
- (@teardown_callbacks ||= []).concat method_names
- end
-
- def setup_callback_chain
- @setup_callbacks ||= []
-
- if superclass.respond_to?(:setup_callback_chain)
- superclass.setup_callback_chain + @setup_callbacks
- else
- @setup_callbacks
- end
- end
-
- def teardown_callback_chain
- @teardown_callbacks ||= []
-
- if superclass.respond_to?(:teardown_callback_chain)
- superclass.teardown_callback_chain + @teardown_callbacks
- else
- @teardown_callbacks
- end
- end
- end
-
# This redefinition is unfortunate but test/unit shows us no alternative.
def run_with_callbacks(result) #:nodoc:
return if @method_name.to_s == "default_test"
@@ -63,7 +32,7 @@ def run_with_callbacks(result) #:nodoc:
ensure
begin
teardown
- run_callbacks :teardown, :reverse_each
+ run_callbacks :teardown, :enumerator => :reverse_each
rescue Test::Unit::AssertionFailedError => e
add_failure(e.message, e.backtrace)
rescue *Test::Unit::TestCase::PASSTHROUGH_EXCEPTIONS
@@ -98,7 +67,7 @@ def run_with_callbacks_and_mocha(result)
ensure
begin
teardown
- run_callbacks :teardown, :reverse_each
+ run_callbacks :teardown, :enumerator => :reverse_each
rescue Test::Unit::AssertionFailedError => e
add_failure(e.message, e.backtrace)
rescue StandardError, ScriptError
@@ -111,17 +80,6 @@ def run_with_callbacks_and_mocha(result)
result.add_run
yield(Test::Unit::TestCase::FINISHED, name)
end
-
- protected
- def run_callbacks(kind, enumerator = :each)
- self.class.send("#{kind}_callback_chain").send(enumerator) do |callback|
- case callback
- when Proc; callback.call(self)
- when String, Symbol; send!(callback)
- else raise ArgumentError, "Unrecognized callback #{callback.inspect}"
- end
- end
- end
end
end
end
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
new file mode 100644
index 0000000000..6d390bbc5c
--- /dev/null
+++ b/activesupport/test/callbacks_test.rb
@@ -0,0 +1,96 @@
+require 'abstract_unit'
+
+class Record
+ include ActiveSupport::Callbacks
+
+ define_callbacks :before_save, :after_save
+
+ class << self
+ def callback_symbol(callback_method)
+ returning("#{callback_method}_method") do |method_name|
+ define_method(method_name) do
+ history << [callback_method, :symbol]
+ end
+ end
+ end
+
+ def callback_string(callback_method)
+ "history << [#{callback_method.to_sym.inspect}, :string]"
+ end
+
+ def callback_proc(callback_method)
+ Proc.new { |model| model.history << [callback_method, :proc] }
+ end
+
+ def callback_object(callback_method)
+ klass = Class.new
+ klass.send(:define_method, callback_method) do |model|
+ model.history << [callback_method, :object]
+ end
+ klass.new
+ end
+ end
+
+ def history
+ @history ||= []
+ end
+end
+
+class Person < Record
+ [:before_save, :after_save].each do |callback_method|
+ callback_method_sym = callback_method.to_sym
+ 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) { |model| model.history << [callback_method_sym, :block] }
+ end
+
+ def save
+ run_callbacks(:before_save)
+ run_callbacks(:after_save)
+ end
+end
+
+class ConditionalPerson < Record
+ before_save Proc.new { |r| r.history << [:before_save, :proc] }, :if => Proc.new { |r| true }
+ before_save Proc.new { |r| r.history << "b00m" }, :if => Proc.new { |r| false }
+ before_save Proc.new { |r| r.history << [:before_save, :proc] }, :unless => Proc.new { |r| false }
+ before_save Proc.new { |r| r.history << "b00m" }, :unless => Proc.new { |r| true }
+
+ def save
+ run_callbacks(:before_save)
+ run_callbacks(:after_save)
+ end
+end
+
+class CallbacksTest < Test::Unit::TestCase
+ def test_save_person
+ person = Person.new
+ assert_equal [], person.history
+ person.save
+ assert_equal [
+ [:before_save, :symbol],
+ [:before_save, :string],
+ [:before_save, :proc],
+ [:before_save, :object],
+ [:before_save, :block],
+ [:after_save, :symbol],
+ [:after_save, :string],
+ [:after_save, :proc],
+ [:after_save, :object],
+ [:after_save, :block]
+ ], person.history
+ end
+end
+
+class ConditionalCallbackTest < Test::Unit::TestCase
+ def test_save_conditional_person
+ person = ConditionalPerson.new
+ person.save
+ assert_equal [
+ [:before_save, :proc],
+ [:before_save, :proc]
+ ], person.history
+ end
+end
diff --git a/activesupport/test/test_test.rb b/activesupport/test/test_test.rb
index 88b505e59c..1e75e18602 100644
--- a/activesupport/test/test_test.rb
+++ b/activesupport/test/test_test.rb
@@ -79,9 +79,9 @@ class SetupAndTeardownTest < Test::Unit::TestCase
teardown :foo, :sentinel, :foo
def test_inherited_setup_callbacks
- assert_equal [:reset_callback_record, :foo], self.class.setup_callback_chain
+ assert_equal [:reset_callback_record, :foo], self.class.setup_callback_chain.map(&:method)
assert_equal [:foo], @called_back
- assert_equal [:foo, :sentinel, :foo], self.class.teardown_callback_chain
+ assert_equal [:foo, :sentinel, :foo], self.class.teardown_callback_chain.map(&:method)
end
protected
@@ -104,9 +104,9 @@ class SubclassSetupAndTeardownTest < SetupAndTeardownTest
teardown :bar
def test_inherited_setup_callbacks
- assert_equal [:reset_callback_record, :foo, :bar], self.class.setup_callback_chain
+ assert_equal [:reset_callback_record, :foo, :bar], self.class.setup_callback_chain.map(&:method)
assert_equal [:foo, :bar], @called_back
- assert_equal [:foo, :sentinel, :foo, :bar], self.class.teardown_callback_chain
+ assert_equal [:foo, :sentinel, :foo, :bar], self.class.teardown_callback_chain.map(&:method)
end
protected