Skip to content

Commit 562dd04

Browse files
committed
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.
1 parent f1af27f commit 562dd04

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

activerecord/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Don't allow destroyed object mutation after `save` or `save!` is called.
2+
3+
*Ryuta Kamizono*
4+
15
* Take into account association conditions when deleting through records.
26

37
Fixes #18424.

activerecord/lib/active_record/persistence.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,7 @@ def _relation_for_itself
694694

695695
def create_or_update(*args, &block)
696696
_raise_readonly_record_error if readonly?
697+
return false if destroyed?
697698
result = new_record? ? _create_record(&block) : _update_record(*args, &block)
698699
result != false
699700
end

activerecord/test/cases/persistence_test.rb

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -599,30 +599,52 @@ def test_update_all_with_non_standard_table_name
599599
end
600600

601601
def test_delete_new_record
602-
client = Client.new
602+
client = Client.new(name: "37signals")
603603
client.delete
604604
assert client.frozen?
605+
606+
assert_not client.save
607+
assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
608+
609+
assert client.frozen?
610+
assert_raise(RuntimeError) { client.name = "something else" }
605611
end
606612

607613
def test_delete_record_with_associations
608614
client = Client.find(3)
609615
client.delete
610616
assert client.frozen?
611617
assert_kind_of Firm, client.firm
618+
619+
assert_not client.save
620+
assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
621+
622+
assert client.frozen?
612623
assert_raise(RuntimeError) { client.name = "something else" }
613624
end
614625

615626
def test_destroy_new_record
616-
client = Client.new
627+
client = Client.new(name: "37signals")
617628
client.destroy
618629
assert client.frozen?
630+
631+
assert_not client.save
632+
assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
633+
634+
assert client.frozen?
635+
assert_raise(RuntimeError) { client.name = "something else" }
619636
end
620637

621638
def test_destroy_record_with_associations
622639
client = Client.find(3)
623640
client.destroy
624641
assert client.frozen?
625642
assert_kind_of Firm, client.firm
643+
644+
assert_not client.save
645+
assert_raise(ActiveRecord::RecordNotSaved) { client.save! }
646+
647+
assert client.frozen?
626648
assert_raise(RuntimeError) { client.name = "something else" }
627649
end
628650

0 commit comments

Comments
 (0)