Deprecate Model#validate/validate_on_create/validate_on_update. Use Model.validate :method and likewise

7 files changed, 74 insertions, 34 deletions

diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index cef107bb4d..64343f7a5c 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -82,10 +82,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)
-      validate if respond_to?(:validate)
-
       errors.empty?
     end
 
@@ -97,12 +94,6 @@ module ActiveModel
     def get_attribute_value(attribute)
       respond_to?(attribute.to_sym) ? send(attribute.to_sym) : instance_variable_get(:"@#{attribute}")
     end
-
-    protected
-    
-    # Overwrite this method for validation checks on all saves and use <tt>Errors.add(field, msg)</tt> for invalid attributes.
-    def validate
-    end
   end
 end
 
diff --git a/activemodel/test/models/reply.rb b/activemodel/test/models/reply.rb
index cfe256034c..acfd801674 100644
--- a/activemodel/test/models/reply.rb
+++ b/activemodel/test/models/reply.rb
@@ -4,9 +4,13 @@ class Reply < Topic
   validate :errors_on_empty_content
   validate_on_create :title_is_wrong_create
 
+  validate :check_empty_title
+  validate_on_create :check_content_mismatch
+  validate_on_update :check_wrong_update
+
   attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read
 
-  def validate
+  def check_empty_title
     errors[:title] << "Empty" unless attribute_present?("title")
   end
 
@@ -14,7 +18,7 @@ class Reply < Topic
     errors[:content] << "Empty" unless attribute_present?("content")
   end
 
-  def validate_on_create
+  def check_content_mismatch
     if attribute_present?("title") && attribute_present?("content") && content == "Mismatch"
       errors[:title] << "is Content Mismatch"
     end
@@ -24,7 +28,7 @@ class Reply < Topic
     errors[:title] << "is Wrong Create" if attribute_present?("title") && title == "Wrong Create"
   end
 
-  def validate_on_update
+  def check_wrong_update
     errors[:title] << "is Wrong Update" if attribute_present?("title") && title == "Wrong Update"
   end
 end
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 9a7b2a47bc..744205df1b 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -164,14 +164,25 @@ module ActiveRecord
 
         run_callbacks(:validate)
 
-        validate if respond_to?(:validate)
+        if respond_to?(:validate)
+          ActiveSupport::Deprecation.warn("Base#validate has been deprecated, please use Base.validate :method instead")
+          validate
+        end
 
         if new_record?
           run_callbacks(:validate_on_create)
-          validate_on_create if respond_to?(:validate_on_create)
+
+          if respond_to?(:validate_on_create)
+            ActiveSupport::Deprecation.warn("Base#validate_on_create has been deprecated, please use Base.validate_on_create :method instead")
+            validate_on_create
+          end
         else
           run_callbacks(:validate_on_update)
-          validate_on_update if respond_to?(:validate_on_update)
+
+          if respond_to?(:validate_on_update)
+            ActiveSupport::Deprecation.warn("Base#validate_on_update has been deprecated, please use Base.validate_on_update :method instead")
+            validate_on_update
+          end
         end
 
         errors.empty?
@@ -185,16 +196,6 @@ module ActiveRecord
       def get_attribute_value(attribute)
         respond_to?(attribute.to_sym) ? send(attribute.to_sym) : self[attribute.to_sym]
       end
-
-      protected
-
-      # Overwrite this method for validation checks used only on creation.
-      def validate_on_create
-      end
-
-      # Overwrite this method for validation checks used only on updates.
-      def validate_on_update
-      end
     end
 
   end
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index a220580a8d..a4e874e5e6 100644
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -16,6 +16,24 @@ class ProtectedPerson < ActiveRecord::Base
   attr_protected :first_name
 end
 
+class DeprecatedPerson < ActiveRecord::Base
+  set_table_name 'people'
+
+  protected
+
+  def validate
+    errors[:name] << "always invalid"
+  end
+
+  def validate_on_create
+    errors[:name] << "invalid on create"
+  end
+
+  def validate_on_update
+    errors[:name] << "invalid on update"
+  end
+end
+
 class ValidationsTest < ActiveRecord::TestCase
   fixtures :topics, :developers
 
@@ -150,4 +168,20 @@ class ValidationsTest < ActiveRecord::TestCase
       assert_equal "Dan Brown", reply["author_name"]
     end
   end
+
+  def test_deprecated_validation_instance_methods
+    tom = DeprecatedPerson.new
+
+    assert_deprecated do
+      assert tom.invalid?
+      assert_equal ["always invalid", "invalid on create"], tom.errors[:name]
+    end
+
+    tom.save(false)
+
+    assert_deprecated do
+      assert tom.invalid?
+      assert_equal ["always invalid", "invalid on update"], tom.errors[:name]
+    end
+  end
 end
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index 02a775f9ef..9c9d03dd5b 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -146,10 +146,13 @@ class Account < ActiveRecord::Base
     true
   end
 
+  validate :check_empty_credit_limit
+
   protected
-    def validate
-      errors.add_on_empty "credit_limit"
-    end
+
+  def check_empty_credit_limit
+    errors.add_on_empty "credit_limit"
+  end
 
   private
 
diff --git a/activerecord/test/models/company_in_module.rb b/activerecord/test/models/company_in_module.rb
index 7f02403d5a..1122c7256f 100644
--- a/activerecord/test/models/company_in_module.rb
+++ b/activerecord/test/models/company_in_module.rb
@@ -52,10 +52,13 @@ module MyApplication
         i.belongs_to :nested_unqualified_billing_firm, :class_name => 'Nested::Firm'
       end
 
+      validate :check_empty_credit_limit
+
       protected
-        def validate
-          errors.add_on_empty "credit_limit"
-        end
+
+      def check_empty_credit_limit
+        errors.add_on_empty "credit_limit"
+      end
     end
   end
 end
diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb
index 55e7ccd22f..616c07687c 100644
--- a/activerecord/test/models/reply.rb
+++ b/activerecord/test/models/reply.rb
@@ -11,7 +11,11 @@ class Reply < Topic
 
   attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read
 
-  def validate
+  validate :check_empty_title
+  validate_on_create :check_content_mismatch
+  validate_on_update :check_wrong_update
+
+  def check_empty_title
     errors[:title] << "Empty" unless attribute_present?("title")
   end
 
@@ -19,7 +23,7 @@ class Reply < Topic
     errors[:content] << "Empty" unless attribute_present?("content")
   end
 
-  def validate_on_create
+  def check_content_mismatch
     if attribute_present?("title") && attribute_present?("content") && content == "Mismatch"
       errors[:title] << "is Content Mismatch"
     end
@@ -29,7 +33,7 @@ class Reply < Topic
     errors[:title] << "is Wrong Create" if attribute_present?("title") && title == "Wrong Create"
   end
 
-  def validate_on_update
+  def check_wrong_update
     errors[:title] << "is Wrong Update" if attribute_present?("title") && title == "Wrong Update"
   end
 end