From 263dae11088b3544292a5a2b24bcdaab2b5aa9c5 Mon Sep 17 00:00:00 2001
From: Eloy Duran <eloy.de.enige@gmail.com>
Date: Sat, 11 Jul 2009 20:52:03 +0200
Subject: [PATCH] During autosave, ignore records that already have been
 destroyed. [#2537 state:resolved]

---
 .../lib/active_record/autosave_association.rb |  6 +++--
 .../test/cases/autosave_association_test.rb   | 26 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 51a8ca3cbe590..ffa9f294df9fe 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -282,6 +282,8 @@ def save_collection_association(reflection)
 
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
           records.each do |record|
+            next if record.destroyed?
+
             if autosave && record.marked_for_destruction?
               association.destroy(record)
             elsif autosave != false && (@new_record_before_save || record.new_record?)
@@ -310,7 +312,7 @@ def save_collection_association(reflection)
     # This all happens inside a transaction, _if_ the Transactions module is included into
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_has_one_association(reflection)
-      if (association = association_instance_get(reflection.name)) && !association.target.nil?
+      if (association = association_instance_get(reflection.name)) && !association.target.nil? && !association.destroyed?
         autosave = reflection.options[:autosave]
 
         if autosave && association.marked_for_destruction?
@@ -334,7 +336,7 @@ def save_has_one_association(reflection)
     # This all happens inside a transaction, _if_ the Transactions module is included into
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_belongs_to_association(reflection)
-      if association = association_instance_get(reflection.name)
+      if (association = association_instance_get(reflection.name)) && !association.destroyed?
         autosave = reflection.options[:autosave]
 
         if autosave && association.marked_for_destruction?
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 2c67bb39fb1da..75d40fbf50489 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -541,6 +541,13 @@ def test_should_skip_validation_on_a_child_association_if_marked_for_destruction
     assert_difference('Ship.count', -1) { @pirate.save! }
   end
 
+  def test_a_child_marked_for_destruction_should_not_be_destroyed_twice
+    @pirate.ship.mark_for_destruction
+    assert @pirate.save
+    @pirate.ship.expects(:destroy).never
+    assert @pirate.save
+  end
+
   def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_child
     # Stub the save method of the @pirate.ship instance to destroy and then raise an exception
     class << @pirate.ship
@@ -579,6 +586,13 @@ def test_should_skip_validation_on_a_parent_association_if_marked_for_destructio
     assert_difference('Pirate.count', -1) { @ship.save! }
   end
 
+  def test_a_parent_marked_for_destruction_should_not_be_destroyed_twice
+    @ship.pirate.mark_for_destruction
+    assert @ship.save
+    @ship.pirate.expects(:destroy).never
+    assert @ship.save
+  end
+
   def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_parent
     # Stub the save method of the @ship.pirate instance to destroy and then raise an exception
     class << @ship.pirate
@@ -637,6 +651,16 @@ def save(*args)
       assert @pirate.valid?
     end
 
+    define_method("test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_#{association_name}") do
+      @pirate.send(association_name).create!(:name => "#{association_name}_1")
+      children = @pirate.send(association_name)
+
+      children.each { |child| child.mark_for_destruction }
+      assert @pirate.save
+      children.each { |child| child.expects(:destroy).never }
+      assert @pirate.save
+    end
+
     define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do
       2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") }
       before = @pirate.send(association_name).map { |c| c }
@@ -1000,4 +1024,4 @@ def setup
   end
 
   include AutosaveAssociationOnACollectionAssociationTests
-end
\ No newline at end of file
+end