From c01be9de322ba846923340e41e69821d01541610 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Sat, 11 Jul 2009 20:04:18 +0200
Subject: [PATCH] Fix has_one with foreign_key and primary_key association bug
 which caused the associated object being lost when saving the owner. [#1756
 state:resolved]

Mixed in a bit from patch by ransom-briggs. [#2813 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 activerecord/lib/active_record/autosave_association.rb   | 9 ++++++---
 .../test/cases/associations/has_one_associations_test.rb | 9 +++++++++
 activerecord/test/cases/reflection_test.rb               | 4 ++--
 activerecord/test/fixtures/accounts.yml                  | 1 +
 activerecord/test/models/company.rb                      | 1 +
 activerecord/test/schema/schema.rb                       | 1 +
 6 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 10dd0b4f05653..75c49ecb2b4c3 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -314,9 +314,12 @@ def save_has_one_association(reflection)
 
         if autosave && association.marked_for_destruction?
           association.destroy
-        elsif autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != id || autosave)
-          association[reflection.primary_key_name] = id
-          association.save(!autosave)
+        else
+          key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
+          if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
+            association[reflection.primary_key_name] = key
+            association.save(!autosave)
+          end
         end
       end
     end
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 7140de77eac46..cdac86a3b9497 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -36,6 +36,15 @@ def test_finding_using_primary_key
     assert_equal accounts(:rails_core_account), firm.account_using_primary_key
   end
 
+  def test_update_with_foreign_and_primary_keys
+    firm = companies(:first_firm)
+    account = firm.account_using_foreign_and_primary_keys
+    assert_equal Account.find_by_firm_name(firm.name), account
+    firm.save
+    firm.reload
+    assert_equal account, firm.account_using_foreign_and_primary_keys
+  end
+
   def test_can_marshal_has_one_association_with_nil_target
     firm = Firm.new
     assert_nothing_raised do
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index aced946b1e2d3..0eb2da720ee3d 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -176,9 +176,9 @@ def test_association_reflection_in_modules
 
   def test_reflection_of_all_associations
     # FIXME these assertions bust a lot
-    assert_equal 34, Firm.reflect_on_all_associations.size
+    assert_equal 35, Firm.reflect_on_all_associations.size
     assert_equal 26, Firm.reflect_on_all_associations(:has_many).size
-    assert_equal 8, Firm.reflect_on_all_associations(:has_one).size
+    assert_equal 9, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end
 
diff --git a/activerecord/test/fixtures/accounts.yml b/activerecord/test/fixtures/accounts.yml
index b2d0191900bba..32583042a8a05 100644
--- a/activerecord/test/fixtures/accounts.yml
+++ b/activerecord/test/fixtures/accounts.yml
@@ -2,6 +2,7 @@ signals37:
   id: 1
   firm_id: 1
   credit_limit: 50
+  firm_name: 37signals
 
 unknown:
   id: 2
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index d69152ec34b2b..b1a3930e4ee2a 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -78,6 +78,7 @@ class Firm < Company
   # added order by id as in fixtures there are two accounts for Rails Core
   # Oracle tests were failing because of that as the second fixture was selected
   has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account", :order => "id"
+  has_one :account_using_foreign_and_primary_keys, :foreign_key => "firm_name", :primary_key => "name", :class_name => "Account"
   has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete
   
   has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 9ab4cf6f435cf..15e5e12d03dbf 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -22,6 +22,7 @@ def create_table(*args, &block)
   # unless the ordering matters.  In which case, define them below
   create_table :accounts, :force => true do |t|
     t.integer :firm_id
+    t.string  :firm_name
     t.integer :credit_limit
   end