From c2362461cdf9615d5704d6f2942921b84b854c3c Mon Sep 17 00:00:00 2001
From: Johnny Holton <johnny@holton.co>
Date: Sat, 18 May 2013 18:16:38 -0400
Subject: destroys association records before saving/inserting new association
 records

This is a backport of #10417

fixes bug introduced by  #3329
These are the conditions necessary to reproduce the bug:
- For an association, autosave => true.
- An association record is being destroyed
- A new association record is being created.
- There is a unique index one of the association's fields.
- The record being created has the same value as the record being
destroyed on the indexed field.

Before, the deletion of records was postponed until after all
insertions/saves.  Therefore the new record with the identical value in
the indexed field caused a non-unique value error to be thrown at the
database
level.

With this fix, the deletions happen first, before the insertions/saves.
Therefore the record with the duplicate value is gone from the database
before the new record is created, thereby avoiding the non-uniuqe value
error.
---
 .../lib/active_record/autosave_association.rb      | 48 +++++++++-------------
 1 file changed, 20 insertions(+), 28 deletions(-)

(limited to 'activerecord/lib')

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index e1499fc3b0..7bc103043e 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -331,41 +331,33 @@ module ActiveRecord
         autosave = reflection.options[:autosave]
 
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
-          begin
-            records_to_destroy = []
-
-            records.each do |record|
-              next if record.destroyed?
-
-              saved = true
-
-              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
-                  association.insert_record(record) unless reflection.nested?
-                end
-              elsif autosave
-                saved = record.save(:validate => false)
-              end
 
-              raise ActiveRecord::Rollback unless saved
-            end
+          if autosave
+            records_to_destroy = records.select(&:marked_for_destruction?)
+            records_to_destroy.each { |record| association.proxy.destroy(record) }
+            records -= records_to_destroy
+          end
+
+          records.each do |record|
 
-            records_to_destroy.each do |record|
-              association.proxy.destroy(record)
+            saved = true
+
+            if autosave != false && (@new_record_before_save || record.new_record?)
+              if autosave
+                saved = association.insert_record(record, false)
+              else
+                association.insert_record(record) unless reflection.nested?
+              end
+            elsif autosave
+              saved = record.save(:validate => false)
             end
-          rescue
-            records.each {|x| IdentityMap.remove(x) } if IdentityMap.enabled?
-            raise
-          end
 
+            raise ActiveRecord::Rollback unless saved
+          end
         end
 
         # reconstruct the scope now that we know the owner's id
-        association.send(:reset_scope) if association.respond_to?(:reset_scope)
+        association.reset_scope if association.respond_to?(:reset_scope)
       end
     end
 
-- 
cgit v1.2.3