aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-01-15 04:30:37 +0900
committerRyuta Kamizono <kamipo@gmail.com>2018-01-15 04:52:01 +0900
commit562dd0494a90d9d47849f052e8913f0050f3e494 (patch)
tree786aa2778e1ba384669b7100e24e16f4af1fa4a9 /activerecord
parentf1af27fd9d9101684b26d0dcf2028859d67bec1f (diff)
downloadrails-562dd0494a90d9d47849f052e8913f0050f3e494.tar.gz
rails-562dd0494a90d9d47849f052e8913f0050f3e494.tar.bz2
rails-562dd0494a90d9d47849f052e8913f0050f3e494.zip
Don't allow destroyed object mutation after `save` or `save!` is called
Currently `object.save` will unfreeze the object, due to `changes_applied` replaces frozen `@attributes` to new `@attributes`. Since originally destroyed objects are not allowed to be mutated, `save` and `save!` should not return success in that case. Fixes #28563.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md4
-rw-r--r--activerecord/lib/active_record/persistence.rb1
-rw-r--r--activerecord/test/cases/persistence_test.rb26
3 files changed, 29 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 52c38edf81..9f1ca768a0 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Don't allow destroyed object mutation after `save` or `save!` is called.
+
+ *Ryuta Kamizono*
+
* Take into account association conditions when deleting through records.
Fixes #18424.
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 462e5e7aaf..c1b1a5334a 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -694,6 +694,7 @@ module ActiveRecord
def create_or_update(*args, &block)
_raise_readonly_record_error if readonly?
+ return false if destroyed?
result = new_record? ? _create_record(&block) : _update_record(*args, &block)
result != false
end
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 0fa8ea212f..0edca96cf5 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -599,9 +599,15 @@ class PersistenceTest < ActiveRecord::TestCase
end
def test_delete_new_record
- client = Client.new
+ client = Client.new(name: "37signals")
client.delete
assert client.frozen?
+
+ assert_not client.save
+ assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
+
+ assert client.frozen?
+ assert_raise(RuntimeError) { client.name = "something else" }
end
def test_delete_record_with_associations
@@ -609,13 +615,24 @@ class PersistenceTest < ActiveRecord::TestCase
client.delete
assert client.frozen?
assert_kind_of Firm, client.firm
+
+ assert_not client.save
+ assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
+
+ assert client.frozen?
assert_raise(RuntimeError) { client.name = "something else" }
end
def test_destroy_new_record
- client = Client.new
+ client = Client.new(name: "37signals")
client.destroy
assert client.frozen?
+
+ assert_not client.save
+ assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
+
+ assert client.frozen?
+ assert_raise(RuntimeError) { client.name = "something else" }
end
def test_destroy_record_with_associations
@@ -623,6 +640,11 @@ class PersistenceTest < ActiveRecord::TestCase
client.destroy
assert client.frozen?
assert_kind_of Firm, client.firm
+
+ assert_not client.save
+ assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
+
+ assert client.frozen?
assert_raise(RuntimeError) { client.name = "something else" }
end