From db5529d45b270b82364d4a655eff7812b6c8b398 Mon Sep 17 00:00:00 2001
From: Nik Wakelin <nik@codetocustomer.com>
Date: Sun, 6 Jul 2008 16:10:27 +1200
Subject: [PATCH] Fix race condition for has_one and create_* methods when validations fail

---
 .../associations/has_one_association.rb            |    3 ++-
 .../associations/has_one_associations_test.rb      |   18 ++++++++++++++++++
 activerecord/test/schema/schema.rb                 |    1 +
 3 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 25a268e..4ff2b8d 100755
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -78,6 +78,7 @@ module ActiveRecord
         end
 
         def new_record(replace_existing)
+          
           # Make sure we load the target first, if we plan on replacing the existing
           # instance. Otherwise, if the target has not previously been loaded
           # elsewhere, the instance we create will get orphaned.
@@ -85,7 +86,7 @@ module ActiveRecord
           record = @reflection.klass.send(:with_scope, :create => construct_scope[:create]) { yield @reflection.klass }
 
           if replace_existing
-            replace(record, true) 
+            replace(record, true) if record.valid?
           else
             record[@reflection.primary_key_name] = @owner.id unless @owner.new_record?
             self.target = record
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index d3ca0ca..dac5ca8 100755
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -100,6 +100,24 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     hsbc.save
     assert_equal apple.id, citibank.firm_id
   end
+  
+  def test_dont_replace_existing_when_validation_fails
+    
+    Account.validates_uniqueness_of :name, :allow_blank => true
+        
+    apple          = Firm.create!(:name => "Apple")
+    first_account  = apple.create_account({ :credit_limit => 5, :name => "Savings" })
+    
+    assert first_account.valid?
+    
+    assert_equal first_account, apple.account
+    
+    second_account = apple.create_account({ :credit_limit => 100, :name => "Savings" })
+    assert !second_account.valid?
+    
+    # make sure we stayed with the first account
+    assert_equal first_account, apple.reload.account
+  end
 
   def test_dependence
     num_accounts = Account.count
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 234c434..240d08d 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -24,6 +24,7 @@ ActiveRecord::Schema.define do
   create_table :accounts, :force => true do |t|
     t.integer :firm_id
     t.integer :credit_limit
+    t.string  :name
   end
 
   create_table :audit_logs, :force => true do |t|
-- 
1.5.6.1