From 562dd0494a90d9d47849f052e8913f0050f3e494 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 15 Jan 2018 04:30:37 +0900
Subject: 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.
---
 activerecord/CHANGELOG.md                     |  4 ++++
 activerecord/lib/active_record/persistence.rb |  1 +
 activerecord/test/cases/persistence_test.rb   | 26 ++++++++++++++++++++++++--
 3 files changed, 29 insertions(+), 2 deletions(-)

(limited to 'activerecord')

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
 
-- 
cgit v1.2.3