During autosave, ignore records that already have been destroyed. [#2537 state:resolved]

activerecord/lib/active_record/autosave_association.rb

@@ -281,6 +281,8 @@ module ActiveRecord
 
         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?)
@@ -309,7 +311,7 @@ module ActiveRecord
     # 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?
@@ -333,7 +335,7 @@ module ActiveRecord
     # 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?

activerecord/test/cases/autosave_association_test.rb

@@ -548,6 +548,13 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     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
@@ -586,6 +593,13 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     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
@@ -644,6 +658,16 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
       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 }