Revert "Merge pull request #10183 from jholton/fix_association_auto_save"

This reverts commit e8727d37fc49d5bf9976c3cb5c46badb92cf4ced, reversing
changes made to d098e1c24bc145e0cc14532348436e14dc46d375.

Reason: it broke the mysql build

3 files changed, 9 insertions, 29 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9e5a9a15c3..278da322f2 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,12 +1,5 @@
 ## Rails 4.0.0 (unreleased) ##
 
-*   fixes bug introduced by #3329.  Now, when autosaving associations,
-    deletions happen before inserts and saves.  This prevents a 'duplicate
-    unique value' database error that would occur if a record being created had
-    the same value on a unique indexed field as that of a record being destroyed.
-
-    *Johnny Holton*
-
 *   Run `rake migrate:down` & `rake migrate:up` in transaction if database supports.
 
     *Alexander Bondarev*
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index a98cacc78c..44323ce9db 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -334,18 +334,15 @@ module ActiveRecord
         autosave = reflection.options[:autosave]
 
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
-
-          if autosave
-            records_to_destroy = records.select(&:marked_for_destruction?)
-            records_to_destroy.each { |record| association.destroy(record) }
-            records -= records_to_destroy
-          end
-
+          records_to_destroy = []
           records.each do |record|
+            next if record.destroyed?
 
             saved = true
 
-            if autosave != false && (@new_record_before_save || record.new_record?)
+            if autosave && record.marked_for_destruction?
+              records_to_destroy << record
+            elsif autosave != false && (@new_record_before_save || record.new_record?)
               if autosave
                 saved = association.insert_record(record, false)
               else
@@ -357,6 +354,10 @@ module ActiveRecord
 
             raise ActiveRecord::Rollback unless saved
           end
+
+          records_to_destroy.each do |record|
+            association.destroy(record)
+          end
         end
 
         # reconstruct the scope now that we know the owner's id
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 552bbd3f52..536ff4882c 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -764,20 +764,6 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     assert_equal 2, @pirate.birds.reload.length
   end
 
-  def test_should_save_new_record_that_has_same_value_as_existing_record_marked_for_destruction_on_field_that_has_unique_index
-    Bird.connection.add_index :birds, :name, unique: true
-
-    3.times { |i| @pirate.birds.create(name: "birds_#{i}") }
-
-    @pirate.birds[0].mark_for_destruction
-    @pirate.birds.build(name: @pirate.birds[0].name)
-    @pirate.save!
-
-    assert_equal 3, @pirate.birds.reload.length
-  ensure
-    Bird.connection.remove_index :birds, column: :name
-  end
-
   # Add and remove callbacks tests for association collections.
   %w{ method proc }.each do |callback_type|
     define_method("test_should_run_add_callback_#{callback_type}s_for_has_many") do