Changed ActiveRecord to use new callbacks and speed up observers by only notifying events that are actually being consumed.

Signed-off-by: Joshua Peek <josh@joshpeek.com>

8 files changed, 38 insertions, 57 deletions

diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index 7d49e60790..72898726d1 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -6,7 +6,7 @@ require 'active_support/callbacks'
 module ActiveModel
   module Validations
     extend ActiveSupport::Concern
-    include ActiveSupport::Callbacks
+    include ActiveSupport::NewCallbacks
 
     included do
       define_callbacks :validate
@@ -64,7 +64,7 @@ module ActiveModel
         attrs   = attrs.flatten
 
         # Declare the validation.
-        send(validation_method(options[:on]), options) do |record|
+        validate options do |record|
           attrs.each do |attr|
             value = record.send(:read_attribute_for_validation, attr)
             next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])
@@ -73,10 +73,14 @@ module ActiveModel
         end
       end
 
-      private
-        def validation_method(on)
-          :validate
+      def validate(*args, &block)
+        options = args.last
+        if options.is_a?(Hash) && options.key?(:on)
+          options[:if] = Array(options[:if])
+          options[:if] << "@_on_validate == :#{options[:on]}"
         end
+        set_callback(:validate, :before, *args, &block)
+      end
     end
 
     # Returns the Errors object that holds all information about attribute error messages.
@@ -87,7 +91,7 @@ module ActiveModel
     # Runs all the specified validations and returns true if no errors were added otherwise false.
     def valid?
       errors.clear
-      run_callbacks(:validate)
+      _run_validate_callbacks
       errors.empty?
     end
 
diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb
index 72d6b1c6f0..3ff677c137 100644
--- a/activemodel/lib/active_model/validations/presence.rb
+++ b/activemodel/lib/active_model/validations/presence.rb
@@ -32,7 +32,7 @@ module ActiveModel
 
         # can't use validates_each here, because it cannot cope with nonexistent attributes,
         # while errors.add_on_empty can
-        send(validation_method(configuration[:on]), configuration) do |record|
+        validate configuration do |record|
           record.errors.add_on_blank(attr_names, configuration[:message])
         end
       end
diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb
index 851cdfebf0..edc2133ddc 100644
--- a/activemodel/lib/active_model/validations/with.rb
+++ b/activemodel/lib/active_model/validations/with.rb
@@ -51,7 +51,7 @@ module ActiveModel
       def validates_with(*args)
         configuration = args.extract_options!
 
-        send(validation_method(configuration[:on]), configuration) do |record|
+        validate configuration do |record|
           args.each do |klass|
             klass.new(record, configuration.except(:on, :if, :unless)).validate
           end
diff --git a/activemodel/lib/active_model/validations_repair_helper.rb b/activemodel/lib/active_model/validations_repair_helper.rb
index 0809e7c0d1..40741e6dbe 100644
--- a/activemodel/lib/active_model/validations_repair_helper.rb
+++ b/activemodel/lib/active_model/validations_repair_helper.rb
@@ -2,44 +2,34 @@ module ActiveModel
   module ValidationsRepairHelper
     extend ActiveSupport::Concern
 
-    module Toolbox
-      def self.record_validations(*model_classes)
-        model_classes.inject({}) do |repair, klass|
-          repair[klass] ||= {}
-          [:validate, :validate_on_create, :validate_on_update].each do |callback|
-            ivar = "@#{callback.to_s}_callbacks"
-            the_callback = klass.instance_variable_get(ivar) if klass.instance_variable_defined?(ivar)
-            repair[klass][callback] = (the_callback.nil? ? nil : the_callback.dup)
-          end
-          repair
-        end
-      end
-
-      def self.reset_validations(recorded)
-        recorded.each do |klass, repairs|
-          [:validate, :validate_on_create, :validate_on_update].each do |callback|
-            klass.instance_variable_set("@#{callback.to_s}_callbacks", repairs[callback])
-          end
-        end
-      end
-    end
-
     module ClassMethods
       def repair_validations(*model_classes)
         setup do
-          @validation_repairs = Toolbox.record_validations(*model_classes)
+          @_stored_callbacks = {}
+          model_classes.each do |k|
+            @_stored_callbacks[k] = k._validate_callbacks.dup
+          end
         end
         teardown do
-          Toolbox.reset_validations(@validation_repairs)
+          model_classes.each do |k|
+            k._validate_callbacks = @_stored_callbacks[k]
+            k.__update_callbacks(:validate)
+          end
         end
       end
     end
 
     def repair_validations(*model_classes, &block)
-      validation_repairs = Toolbox.record_validations(*model_classes)
+      @__stored_callbacks = {}
+      model_classes.each do |k|
+        @__stored_callbacks[k] = k._validate_callbacks.dup
+      end
       return block.call
     ensure
-      Toolbox.reset_validations(validation_repairs)
+      model_classes.each do |k|
+        k._validate_callbacks = @__stored_callbacks[k]
+        k.__update_callbacks(:validate)
+      end
     end
   end
 end
diff --git a/activemodel/test/cases/validations/i18n_generate_message_validation_test.rb b/activemodel/test/cases/validations/i18n_generate_message_validation_test.rb
index 37bba5e95e..07e4341289 100644
--- a/activemodel/test/cases/validations/i18n_generate_message_validation_test.rb
+++ b/activemodel/test/cases/validations/i18n_generate_message_validation_test.rb
@@ -5,7 +5,7 @@ require 'models/person'
 
 class I18nGenerateMessageValidationTest < Test::Unit::TestCase
   def setup
-    reset_callbacks Person
+    Person.reset_callbacks(:validate)
     @person = Person.new
 
     @old_load_path, @old_backend = I18n.load_path, I18n.backend
@@ -45,12 +45,6 @@ class I18nGenerateMessageValidationTest < Test::Unit::TestCase
     I18n.backend = @old_backend
   end
 
-  def reset_callbacks(*models)
-    models.each do |model|
-      model.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
-    end
-  end
-
   # validates_inclusion_of: generate_message(attr_name, :inclusion, :default => configuration[:message], :value => value)
   def test_generate_message_inclusion_with_default_message
     assert_equal 'is not included in the list', @person.errors.generate_message(:title, :inclusion, :default => nil, :value => 'title')
diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb
index 544b680b4b..fc4f1926b0 100644
--- a/activemodel/test/cases/validations/i18n_validation_test.rb
+++ b/activemodel/test/cases/validations/i18n_validation_test.rb
@@ -7,8 +7,7 @@ class I18nValidationTest < ActiveModel::TestCase
   include ActiveModel::TestsDatabase
 
   def setup
-    reset_callbacks Person
-
+    Person.reset_callbacks(:validate)
     @person = Person.new
 
     @old_load_path, @old_backend = I18n.load_path, I18n.backend
@@ -18,17 +17,11 @@ class I18nValidationTest < ActiveModel::TestCase
   end
 
   def teardown
-    reset_callbacks Person
+    Person.reset_callbacks(:validate)
     I18n.load_path.replace @old_load_path
     I18n.backend = @old_backend
   end
 
-  def reset_callbacks(*models)
-    models.each do |model|
-      model.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
-    end
-  end
-
   def test_percent_s_interpolation_syntax_in_error_messages_was_deprecated
     assert_not_deprecated do
       default = "%s interpolation syntax was deprecated"
@@ -532,4 +525,4 @@ class I18nValidationTest < ActiveModel::TestCase
     assert_equal ["I am a custom error"], @person.errors[:title]
   end
 
-end
\ No newline at end of file
+end
diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb
index 0b340e68bf..d44667e722 100644
--- a/activemodel/test/cases/validations_test.rb
+++ b/activemodel/test/cases/validations_test.rb
@@ -121,8 +121,8 @@ class ValidationsTest < ActiveModel::TestCase
   end
 
   def test_invalid_validator
-    Topic.validate 3
-    assert_raise(ArgumentError) { t = Topic.create }
+    Topic.validate :i_dont_exist
+    assert_raise(NameError) { t = Topic.create }
   end
 
   def test_errors_to_xml
@@ -189,4 +189,4 @@ class ValidationsTest < ActiveModel::TestCase
     all_errors = t.errors.to_a
     assert_deprecated { assert_equal all_errors, t.errors.each_full{|err| err} }
   end
-end
\ No newline at end of file
+end
diff --git a/activemodel/test/models/reply.rb b/activemodel/test/models/reply.rb
index acfd801674..e86692677f 100644
--- a/activemodel/test/models/reply.rb
+++ b/activemodel/test/models/reply.rb
@@ -2,11 +2,11 @@ require 'models/topic'
 
 class Reply < Topic
   validate :errors_on_empty_content
-  validate_on_create :title_is_wrong_create
+  validate :title_is_wrong_create, :on => :create
 
   validate :check_empty_title
-  validate_on_create :check_content_mismatch
-  validate_on_update :check_wrong_update
+  validate :check_content_mismatch, :on => :create
+  validate :check_wrong_update, :on => :update
 
   attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read