Fixed that Base.save should always return false if the save didn't succeed, including if it has halted by before_save's (closes #1861, #2477) [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3707 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 18 insertions, 4 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 88baaecfd3..1fd247e47b 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -21,6 +21,8 @@ module ActiveRecord #:nodoc:
   end
   class RecordNotFound < ActiveRecordError #:nodoc:
   end
+  class RecordNotSaved < ActiveRecordError #:nodoc:
+  end
   class StatementInvalid < ActiveRecordError #:nodoc:
   end
   class PreparedStatementInvalid < ActiveRecordError #:nodoc:
@@ -1285,9 +1287,15 @@ module ActiveRecord #:nodoc:
       # * No record exists: Creates a new record with values matching those of the object attributes.
       # * A record does exist: Updates the record with values matching those of the object attributes.
       def save
-        raise ActiveRecord::ReadOnlyRecord if readonly?
+        raise ReadOnlyRecord if readonly?
         create_or_update
       end
+      
+      # Attempts to save the record, but instead of just returning false if it couldn't happen, it raises a 
+      # RecordNotSaved exception
+      def save!
+        raise RecordNotSaved unless save
+      end
 
       # Deletes the record in the database and freezes this instance to reflect that no changes should
       # be made (since they can't be persisted).
@@ -1505,6 +1513,8 @@ module ActiveRecord #:nodoc:
           "WHERE #{self.class.primary_key} = #{quote(id)}",
           "#{self.class.name} Update"
         )
+        
+        return true
       end
 
       # Creates a new record with values matching those of the instance attributes.
@@ -1522,6 +1532,8 @@ module ActiveRecord #:nodoc:
         )
 
         @new_record = false
+        
+        return true
       end
 
       # Sets the attribute used for single table inheritance to this class name if this is not the ActiveRecord descendent.
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 53c4f596fe..e206dfbbf1 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -216,6 +216,9 @@ module ActiveRecord
         alias_method :save_without_validation, :save
         alias_method :save, :save_with_validation
 
+        alias_method :save_without_validation!, :save!
+        alias_method :save!, :save_with_validation!
+
         alias_method :update_attribute_without_validation_skipping, :update_attribute
         alias_method :update_attribute, :update_attribute_with_validation_skipping
       end
@@ -719,7 +722,6 @@ module ActiveRecord
     def save_with_validation(perform_validation = true)
       if perform_validation && valid? || !perform_validation
         save_without_validation
-        true
       else
         false
       end
@@ -727,9 +729,9 @@ module ActiveRecord
 
     # Attempts to save the record just like Base#save but will raise a RecordInvalid exception instead of returning false
     # if the record is not valid.
-    def save!
+    def save_with_validation!
       if valid?
-        save(false)
+        save_without_validation!
       else
         raise RecordInvalid.new(self)
       end