Use define_callbacks helper for ActiveRecord validations.

4 files changed, 25 insertions, 57 deletions

diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index 3b7b9050be..b15bdb06ca 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -3,23 +3,7 @@ module ActiveModel
     def self.included(base) # :nodoc:
       base.extend(ClassMethods)
       base.send!(:include, ActiveSupport::Callbacks)
-
-      %w( validate validate_on_create validate_on_update ).each do |validation_method|
-        base.class_eval <<-"end_eval"
-          def self.#{validation_method}(*methods, &block)
-            self.#{validation_method}_callback_chain | CallbackChain.build(:#{validation_method}, *methods, &block)
-          end
-
-          def self.#{validation_method}_callback_chain
-            if chain = read_inheritable_attribute(:#{validation_method})
-              return chain
-            else
-              write_inheritable_attribute(:#{validation_method}, CallbackChain.new)
-              return #{validation_method}_callback_chain
-            end
-          end
-        end_eval
-      end
+      base.define_callbacks :validate, :validate_on_create, :validate_on_update
     end
 
     module ClassMethods
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 7b70f78405..1d12ea8ad7 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -281,23 +281,7 @@ module ActiveRecord
       end
 
       base.send :include, ActiveSupport::Callbacks
-
-      VALIDATIONS.each do |validation_method|
-        base.class_eval <<-"end_eval"
-          def self.#{validation_method}(*methods, &block)
-            self.#{validation_method}_callback_chain | CallbackChain.build(:#{validation_method}, *methods, &block)
-          end
-
-          def self.#{validation_method}_callback_chain
-            if chain = read_inheritable_attribute(:#{validation_method})
-              return chain
-            else
-              write_inheritable_attribute(:#{validation_method}, CallbackChain.new)
-              return #{validation_method}_callback_chain
-            end
-          end
-        end_eval
-      end
+      base.define_callbacks *VALIDATIONS
     end
 
     # All of the following validations are defined in the class scope of the model that you're interested in validating.
@@ -403,7 +387,7 @@ module ActiveRecord
       #   method, proc or string should return or evaluate to a true or false value.
       # * <tt>unless</tt> - 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_confirmation_of(*attr_names)
         configuration = { :message => ActiveRecord::Errors.default_error_messages[:confirmation], :on => :save }
         configuration.update(attr_names.extract_options!)
@@ -437,7 +421,7 @@ module ActiveRecord
       #   method, proc or string should return or evaluate to a true or false value.
       # * <tt>unless</tt> - 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_acceptance_of(*attr_names)
         configuration = { :message => ActiveRecord::Errors.default_error_messages[:accepted], :on => :save, :allow_nil => true, :accept => "1" }
         configuration.update(attr_names.extract_options!)
@@ -519,7 +503,7 @@ module ActiveRecord
       #   method, proc or string should return or evaluate to a true or false value.
       # * <tt>unless</tt> - 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_length_of(*attrs)
         # Merge given options with defaults.
         options = {
@@ -596,7 +580,7 @@ module ActiveRecord
       # attribute (that maps to a column). When the record is updated, the same check is made but disregarding the record itself.
       #
       # Because this check is performed outside the database there is still a chance that duplicate values
-      # will be inserted in two parallel transactions.  To guarantee against this you should create a 
+      # will be inserted in two parallel transactions.  To guarantee against this you should create a
       # unique index on the field. See +add_index+ for more information.
       #
       # Configuration options:
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index 97ac22eaf3..ca36ad3581 100755
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -58,9 +58,9 @@ class ValidationsTest < ActiveRecord::TestCase
   fixtures :topics, :developers, 'warehouse-things'
 
   def setup
-    Topic.write_inheritable_attribute(:validate, nil)
-    Topic.write_inheritable_attribute(:validate_on_create, nil)
-    Topic.write_inheritable_attribute(:validate_on_update, nil)
+    Topic.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
+    Topic.instance_variable_set("@validate_on_create_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
+    Topic.instance_variable_set("@validate_on_update_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
   end
 
   def test_single_field_validation
@@ -839,16 +839,16 @@ class ValidationsTest < ActiveRecord::TestCase
     reply = t.replies.build('title' => 'areply', 'content' => 'whateveragain')
     assert t.valid?
   end
-  
+
   def test_validates_size_of_association_using_within
     assert_nothing_raised { Topic.validates_size_of :replies, :within => 1..2 }
     t = Topic.new('title' => 'noreplies', 'content' => 'whatever')
     assert !t.save
     assert t.errors.on(:replies)
-    
+
     reply = t.replies.build('title' => 'areply', 'content' => 'whateveragain')
     assert t.valid?
-    
+
     2.times { t.replies.build('title' => 'areply', 'content' => 'whateveragain') }
     assert !t.save
     assert t.errors.on(:replies)
@@ -1351,9 +1351,9 @@ class ValidatesNumericalityTest < ActiveRecord::TestCase
   JUNK = ["not a number", "42 not a number", "0xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"]
 
   def setup
-    Topic.write_inheritable_attribute(:validate, nil)
-    Topic.write_inheritable_attribute(:validate_on_create, nil)
-    Topic.write_inheritable_attribute(:validate_on_update, nil)
+    Topic.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
+    Topic.instance_variable_set("@validate_on_create_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
+    Topic.instance_variable_set("@validate_on_update_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
   end
 
   def test_default_validates_numericality_of
diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 5c51237049..9c59b7ac76 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -99,8 +99,8 @@ module ActiveSupport
       def |(chain)
         if chain.is_a?(CallbackChain)
           chain.each { |callback| self | callback }
-        elsif chain.is_a?(Callback)
-          if index = index(chain)
+        else
+          if (found_callback = find(chain)) && (index = index(chain))
             self[index] = chain
           else
             self << chain
@@ -224,8 +224,8 @@ module ActiveSupport
       end
     end
 
-    # Runs all the callbacks defined for the given options. 
-    # 
+    # Runs all the callbacks defined for the given options.
+    #
     # If a block is given it will be called after each callback receiving as arguments:
     #
     #  * the result from the callback
@@ -236,31 +236,31 @@ module ActiveSupport
     # Example:
     #   class Storage
     #     include ActiveSupport::Callbacks
-    #   
+    #
     #     define_callbacks :before_save, :after_save
     #   end
-    #   
+    #
     #   class ConfigStorage < Storage
     #     before_save :pass
     #     before_save :pass
     #     before_save :stop
     #     before_save :pass
-    #   
+    #
     #     def pass
     #       puts "pass"
     #     end
-    #   
+    #
     #     def stop
     #       puts "stop"
     #       return false
     #     end
-    #   
+    #
     #     def save
     #       result = run_callbacks(:before_save) { |result, object| result == false }
     #       puts "- save" if result
     #     end
     #   end
-    #   
+    #
     #   config = ConfigStorage.new
     #   config.save
     #