Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed that Base.save should always return false if the save didn't su…

…cceed, 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
  • Loading branch information...
commit 4c7555aef78decb8f9d515be72741c273114bc21 1 parent 17ff70a
@dhh dhh authored
View
14 activerecord/lib/active_record/base.rb
@@ -21,6 +21,8 @@ class ConnectionFailed < ActiveRecordError #: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 @@ def new_record?
# * 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 @@ def update
"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 @@ def create
)
@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.
View
8 activerecord/lib/active_record/validations.rb
@@ -216,6 +216,9 @@ def self.append_features(base) # :nodoc:
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 @@ def validation_method(on)
def save_with_validation(perform_validation = true)
if perform_validation && valid? || !perform_validation
save_without_validation
- true
else
false
end
@@ -727,9 +729,9 @@ def save_with_validation(perform_validation = true)
# 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
View
28 activerecord/test/callbacks_test.rb
@@ -324,39 +324,21 @@ def test_delete
def test_before_save_returning_false
david = ImmutableDeveloper.find(1)
assert david.valid?
- assert david.save
- assert david.cancelled?
-
- david = ImmutableDeveloper.find(1)
- david.salary = 10_000_000
- assert !david.valid?
assert !david.save
- assert !david.cancelled?
-
- david = ImmutableMethodDeveloper.find(1)
- assert david.valid?
- assert david.save
- assert david.cancelled?
+ assert_raises(ActiveRecord::RecordNotSaved) { david.save! }
- david = ImmutableMethodDeveloper.find(1)
+ david = ImmutableDeveloper.find(1)
david.salary = 10_000_000
assert !david.valid?
assert !david.save
- assert !david.cancelled?
+ assert_raises(ActiveRecord::RecordInvalid) { david.save! }
end
def test_before_destroy_returning_false
david = ImmutableDeveloper.find(1)
- david.destroy
- assert david.cancelled?
+ assert !david.destroy
assert_not_nil ImmutableDeveloper.find_by_id(1)
-
- david = ImmutableMethodDeveloper.find(1)
- david.destroy
- assert david.cancelled?
- assert_not_nil ImmutableMethodDeveloper.find_by_id(1)
- end
-
+ end
def test_zzz_callback_returning_false # must be run last since we modify CallbackDeveloper
david = CallbackDeveloper.find(1)
Please sign in to comment.
Something went wrong with that request. Please try again.