From fc85c665271578e55e7fe90a721ca1533289d923 Mon Sep 17 00:00:00 2001
From: George Ogata <george.ogata@gmail.com>
Date: Thu, 26 Nov 2009 00:05:57 -0500
Subject: [PATCH 1/8] Set inverse for #replace on a has_one association. [#3513
 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 .../associations/has_one_association.rb           |  1 +
 .../associations/inverse_associations_test.rb     | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index b85a40b2e5..081d6233c4 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -57,6 +57,7 @@ module ActiveRecord
           @target = (AssociationProxy === obj ? obj.target : obj)
         end
 
+        set_inverse_instance(obj, @owner)
         @loaded = true
 
         unless @owner.new_record? or obj.nil? or dont_save
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index 47f83db112..ee360dff10 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -135,6 +135,21 @@ class InverseHasOneTests < ActiveRecord::TestCase
     assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
   end
 
+  def test_parent_instance_should_be_shared_with_replaced_child
+    man = Man.find(:first)
+    old_face = man.face
+    new_face = Face.new
+
+    assert_not_nil man.face
+    man.face.replace(new_face)
+
+    assert_equal man.name, new_face.man.name, "Name of man should be the same before changes to parent instance"
+    man.name = 'Bongo'
+    assert_equal man.name, new_face.man.name, "Name of man should be the same after changes to parent instance"
+    new_face.man.name = 'Mungo'
+    assert_equal man.name, new_face.man.name, "Name of man should be the same after changes to replaced-parent-owned instance"
+  end
+
   def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
     assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face }
   end

From 6c8c85bc1eaf1639ea0df5f356e7105c74d128b2 Mon Sep 17 00:00:00 2001
From: Murray Steele <muz@h-lame.com>
Date: Thu, 17 Dec 2009 11:38:44 +0000
Subject: [PATCH 2/8] Add more tests for the various ways we can assign objects
 to associations. [#3513 state:resolved]

Get rid of a duplicate set_inverse_instance call if you use new_record(true) (e.g. you want to replace the existing instance).

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 .../associations/has_one_association.rb       |   3 +-
 .../associations/inverse_associations_test.rb | 170 ++++++++++++++++--
 2 files changed, 160 insertions(+), 13 deletions(-)

diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 081d6233c4..ea769fd48b 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -121,10 +121,9 @@ module ActiveRecord
           else
             record[@reflection.primary_key_name] = @owner.id unless @owner.new_record?
             self.target = record
+            set_inverse_instance(record, @owner)
           end
 
-          set_inverse_instance(record, @owner)
-
           record
         end
 
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index ee360dff10..c3d0d61cec 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -135,19 +135,84 @@ class InverseHasOneTests < ActiveRecord::TestCase
     assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
   end
 
-  def test_parent_instance_should_be_shared_with_replaced_child
-    man = Man.find(:first)
-    old_face = man.face
-    new_face = Face.new
+  def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method
+    m = Man.find(:first)
+    f = m.face.create!(:description => 'haunted')
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+  end
 
-    assert_not_nil man.face
-    man.face.replace(new_face)
+  def test_parent_instance_should_be_shared_with_newly_built_child_when_we_dont_replace_existing
+    m = Man.find(:first)
+    f = m.build_face({:description => 'haunted'}, false)
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to just-built-child-owned instance"
+  end
 
-    assert_equal man.name, new_face.man.name, "Name of man should be the same before changes to parent instance"
-    man.name = 'Bongo'
-    assert_equal man.name, new_face.man.name, "Name of man should be the same after changes to parent instance"
-    new_face.man.name = 'Mungo'
-    assert_equal man.name, new_face.man.name, "Name of man should be the same after changes to replaced-parent-owned instance"
+  def test_parent_instance_should_be_shared_with_newly_created_child_when_we_dont_replace_existing
+    m = Man.find(:first)
+    f = m.create_face({:description => 'haunted'}, false)
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method_when_we_dont_replace_existing
+    m = Man.find(:first)
+    f = m.face.create!({:description => 'haunted'}, false)
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_replaced_via_accessor_child
+    m = Man.find(:first)
+    f = Face.new(:description => 'haunted')
+    m.face = f
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_replaced_via_method_child
+    m = Man.find(:first)
+    f = Face.new(:description => 'haunted')
+    m.face.replace(f)
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_replaced_via_method_child_when_we_dont_replace_existing
+    m = Man.find(:first)
+    f = Face.new(:description => 'haunted')
+    m.face.replace(f, false)
+    assert_not_nil f.man
+    assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
+    f.man.name = 'Mungo'
+    assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance"
   end
 
   def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
@@ -204,6 +269,18 @@ class InverseHasManyTests < ActiveRecord::TestCase
     assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance"
   end
 
+  def test_parent_instance_should_be_shared_with_newly_block_style_built_child
+    m = Man.find(:first)
+    i = m.interests.build {|ii| ii.topic = 'Industrial Revolution Re-enactment'}
+    assert_not_nil i.topic, "Child attributes supplied to build via blocks should be populated"
+    assert_not_nil i.man
+    assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+    i.man.name = 'Mungo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance"
+  end
+
   def test_parent_instance_should_be_shared_with_newly_created_child
     m = Man.find(:first)
     i = m.interests.create(:topic => 'Industrial Revolution Re-enactment')
@@ -215,6 +292,29 @@ class InverseHasManyTests < ActiveRecord::TestCase
     assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
   end
 
+  def test_parent_instance_should_be_shared_with_newly_created_via_bang_method_child
+    m = Man.find(:first)
+    i = m.interests.create!(:topic => 'Industrial Revolution Re-enactment')
+    assert_not_nil i.man
+    assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+    i.man.name = 'Mungo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_newly_block_style_created_child
+    m = Man.find(:first)
+    i = m.interests.create {|ii| ii.topic = 'Industrial Revolution Re-enactment'}
+    assert_not_nil i.topic, "Child attributes supplied to create via blocks should be populated"
+    assert_not_nil i.man
+    assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+    i.man.name = 'Mungo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
+  end
+
   def test_parent_instance_should_be_shared_with_poked_in_child
     m = Man.find(:first)
     i = Interest.create(:topic => 'Industrial Revolution Re-enactment')
@@ -227,6 +327,30 @@ class InverseHasManyTests < ActiveRecord::TestCase
     assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance"
   end
 
+  def test_parent_instance_should_be_shared_with_replaced_via_accessor_children
+    m = Man.find(:first)
+    i = Interest.new(:topic => 'Industrial Revolution Re-enactment')
+    m.interests = [i]
+    assert_not_nil i.man
+    assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+    i.man.name = 'Mungo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance"
+  end
+
+  def test_parent_instance_should_be_shared_with_replaced_via_method_children
+    m = Man.find(:first)
+    i = Interest.new(:topic => 'Industrial Revolution Re-enactment')
+    m.interests.replace([i])
+    assert_not_nil i.man
+    assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
+    m.name = 'Bongo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance"
+    i.man.name = 'Mungo'
+    assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance"
+  end
+
   def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
     assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests }
   end
@@ -299,6 +423,30 @@ class InverseBelongsToTests < ActiveRecord::TestCase
     assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance"
   end
 
+  def test_child_instance_should_be_shared_with_replaced_via_accessor_parent
+    f = Face.find(:first)
+    m = Man.new(:name => 'Charles')
+    f.man = m
+    assert_not_nil m.face
+    assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+    f.description = 'gormless'
+    assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+    m.face.description = 'pleasing'
+    assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance"
+  end
+
+  def test_child_instance_should_be_shared_with_replaced_via_method_parent
+    f = Face.find(:first)
+    m = Man.new(:name => 'Charles')
+    f.man.replace(m)
+    assert_not_nil m.face
+    assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
+    f.description = 'gormless'
+    assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
+    m.face.description = 'pleasing'
+    assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance"
+  end
+
   def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
     assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man }
   end

From 81ca0cf2b074f4b868a84c427ef155607a956119 Mon Sep 17 00:00:00 2001
From: George Ogata <george.ogata@gmail.com>
Date: Sun, 29 Nov 2009 00:46:09 -0500
Subject: [PATCH 3/8] Add inverse polymorphic association support. [#3520
 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 .../belongs_to_polymorphic_association.rb     |  39 +++++--
 activerecord/lib/active_record/reflection.rb  |  14 ++-
 .../associations/inverse_associations_test.rb | 100 ++++++++++++++----
 activerecord/test/fixtures/faces.yml          |   4 +
 activerecord/test/fixtures/interests.yml      |   6 +-
 activerecord/test/models/face.rb              |   1 +
 activerecord/test/models/interest.rb          |   1 +
 activerecord/test/models/man.rb               |   2 +
 activerecord/test/schema/schema.rb            |   4 +
 9 files changed, 139 insertions(+), 32 deletions(-)

diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
index 67e18d692d..9678300003 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -13,6 +13,7 @@ module ActiveRecord
           @updated = true
         end
 
+        set_inverse_instance(record, @owner)
         loaded
         record
       end
@@ -22,19 +23,37 @@ module ActiveRecord
       end
 
       private
+
+        # NOTE - for now, we're only supporting inverse setting from belongs_to back onto
+        # has_one associations.
+        def we_can_set_the_inverse_on_this?(record)
+          @reflection.has_inverse? && @reflection.polymorphic_inverse_of(record.class).macro == :has_one
+        end
+
+        def set_inverse_instance(record, instance)
+          return if record.nil? || !we_can_set_the_inverse_on_this?(record)
+          inverse_relationship = @reflection.polymorphic_inverse_of(record.class)
+          unless inverse_relationship.nil?
+            record.send(:"set_#{inverse_relationship.name}_target", instance)
+          end
+        end
+
         def find_target
           return nil if association_class.nil?
 
-          if @reflection.options[:conditions]
-            association_class.find(
-              @owner[@reflection.primary_key_name],
-              :select     => @reflection.options[:select],
-              :conditions => conditions,
-              :include    => @reflection.options[:include]
-            )
-          else
-            association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
-          end
+          target =
+            if @reflection.options[:conditions]
+              association_class.find(
+                @owner[@reflection.primary_key_name],
+                :select     => @reflection.options[:select],
+                :conditions => conditions,
+                :include    => @reflection.options[:include]
+              )
+            else
+              association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
+            end
+          set_inverse_instance(target, @owner) if target
+          target
         end
 
         def foreign_key_present
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index db5d2b25ed..72f7df32c7 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -214,8 +214,10 @@ module ActiveRecord
       end
 
       def check_validity_of_inverse!
-        if has_inverse? && inverse_of.nil?
-          raise InverseOfAssociationNotFoundError.new(self)
+        unless options[:polymorphic]
+          if has_inverse? && inverse_of.nil?
+            raise InverseOfAssociationNotFoundError.new(self)
+          end
         end
       end
 
@@ -242,6 +244,14 @@ module ActiveRecord
         end
       end
 
+      def polymorphic_inverse_of(associated_class)
+        if has_inverse?
+          associated_class.reflect_on_association(options[:inverse_of])
+        else
+          nil
+        end
+      end
+
       private
         def derive_class_name
           class_name = name.to_s.camelize
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index c3d0d61cec..2ab3aa141a 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -85,7 +85,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
   fixtures :men, :faces
 
   def test_parent_instance_should_be_shared_with_child_on_find
-    m = Man.find(:first)
+    m = men(:gordon)
     f = m.face
     assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
     m.name = 'Bongo'
@@ -96,7 +96,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
 
 
   def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find
-    m = Man.find(:first, :include => :face)
+    m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face)
     f = m.face
     assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
     m.name = 'Bongo'
@@ -104,7 +104,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
     f.man.name = 'Mungo'
     assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"
 
-    m = Man.find(:first, :include => :face, :order => 'faces.id')
+    m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face, :order => 'faces.id')
     f = m.face
     assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
     m.name = 'Bongo'
@@ -114,7 +114,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
   end
 
   def test_parent_instance_should_be_shared_with_newly_built_child
-    m = Man.find(:first)
+    m = men(:gordon)
     f = m.build_face(:description => 'haunted')
     assert_not_nil f.man
     assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
@@ -125,7 +125,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
   end
 
   def test_parent_instance_should_be_shared_with_newly_created_child
-    m = Man.find(:first)
+    m = men(:gordon)
     f = m.create_face(:description => 'haunted')
     assert_not_nil f.man
     assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
@@ -224,7 +224,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
   fixtures :men, :interests
 
   def test_parent_instance_should_be_shared_with_every_child_on_find
-    m = Man.find(:first)
+    m = men(:gordon)
     is = m.interests
     is.each do |i|
       assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@@ -236,7 +236,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
   end
 
   def test_parent_instance_should_be_shared_with_eager_loaded_children
-    m = Man.find(:first, :include => :interests)
+    m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests)
     is = m.interests
     is.each do |i|
       assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@@ -246,7 +246,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
       assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
     end
 
-    m = Man.find(:first, :include => :interests, :order => 'interests.id')
+    m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests, :order => 'interests.id')
     is = m.interests
     is.each do |i|
       assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@@ -255,11 +255,10 @@ class InverseHasManyTests < ActiveRecord::TestCase
       i.man.name = 'Mungo'
       assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
     end
-
   end
 
   def test_parent_instance_should_be_shared_with_newly_built_child
-    m = Man.find(:first)
+    m = men(:gordon)
     i = m.interests.build(:topic => 'Industrial Revolution Re-enactment')
     assert_not_nil i.man
     assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@@ -282,7 +281,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
   end
 
   def test_parent_instance_should_be_shared_with_newly_created_child
-    m = Man.find(:first)
+    m = men(:gordon)
     i = m.interests.create(:topic => 'Industrial Revolution Re-enactment')
     assert_not_nil i.man
     assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
@@ -316,7 +315,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
   end
 
   def test_parent_instance_should_be_shared_with_poked_in_child
-    m = Man.find(:first)
+    m = men(:gordon)
     i = Interest.create(:topic => 'Industrial Revolution Re-enactment')
     m.interests << i
     assert_not_nil i.man
@@ -360,7 +359,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   fixtures :men, :faces, :interests
 
   def test_child_instance_should_be_shared_with_parent_on_find
-    f = Face.find(:first)
+    f = faces(:trusting)
     m = f.man
     assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
     f.description = 'gormless'
@@ -370,7 +369,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   end
 
   def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
-    f = Face.find(:first, :include => :man)
+    f = Face.find(:first, :include => :man, :conditions => {:description => 'trusting'})
     m = f.man
     assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
     f.description = 'gormless'
@@ -378,8 +377,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
     m.face.description = 'pleasing'
     assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"
 
-
-    f = Face.find(:first, :include => :man, :order => 'men.id')
+    f = Face.find(:first, :include => :man, :order => 'men.id', :conditions => {:description => 'trusting'})
     m = f.man
     assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
     f.description = 'gormless'
@@ -389,7 +387,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   end
 
   def test_child_instance_should_be_shared_with_newly_built_parent
-    f = Face.find(:first)
+    f = faces(:trusting)
     m = f.build_man(:name => 'Charles')
     assert_not_nil m.face
     assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
@@ -400,7 +398,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   end
 
   def test_child_instance_should_be_shared_with_newly_created_parent
-    f = Face.find(:first)
+    f = faces(:trusting)
     m = f.create_man(:name => 'Charles')
     assert_not_nil m.face
     assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
@@ -411,7 +409,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   end
 
   def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many
-    i = Interest.find(:first)
+    i = interests(:trainspotting)
     m = i.man
     assert_not_nil m.interests
     iz = m.interests.detect {|iz| iz.id == i.id}
@@ -452,6 +450,70 @@ class InverseBelongsToTests < ActiveRecord::TestCase
   end
 end
 
+class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
+  fixtures :men, :faces, :interests
+
+  def test_child_instance_should_be_shared_with_parent_on_find
+    f = Face.find(:first, :conditions => {:description => 'confused'})
+    m = f.polymorphic_man
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
+    f.description = 'gormless'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
+    m.polymorphic_face.description = 'pleasing'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
+  end
+
+  def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
+    f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man)
+    m = f.polymorphic_man
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
+    f.description = 'gormless'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
+    m.polymorphic_face.description = 'pleasing'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
+
+    f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man, :order => 'men.id')
+    m = f.polymorphic_man
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
+    f.description = 'gormless'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
+    m.polymorphic_face.description = 'pleasing'
+    assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
+  end
+
+  def test_child_instance_should_be_shared_with_replaced_parent
+    face = faces(:confused)
+    old_man = face.polymorphic_man
+    new_man = Man.new
+
+    assert_not_nil face.polymorphic_man
+    face.polymorphic_man.replace(new_man)
+
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance"
+    face.description = 'Bongo'
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance"
+    new_man.polymorphic_face.description = 'Mungo'
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance"
+  end
+
+  def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many
+    i = interests(:llama_wrangling)
+    m = i.polymorphic_man
+    assert_not_nil m.polymorphic_interests
+    iz = m.polymorphic_interests.detect {|iz| iz.id == i.id}
+    assert_not_nil iz
+    assert_equal i.topic, iz.topic, "Interest topics should be the same before changes to child"
+    i.topic = 'Eating cheese with a spoon'
+    assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to child"
+    iz.topic = 'Cow tipping'
+    assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance"
+  end
+
+  def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
+    assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man }
+  end
+end
+
 # NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin
 # which would guess the inverse rather than look for an explicit configuration option.
 class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase
diff --git a/activerecord/test/fixtures/faces.yml b/activerecord/test/fixtures/faces.yml
index 1dd2907cf7..c8e4a34484 100644
--- a/activerecord/test/fixtures/faces.yml
+++ b/activerecord/test/fixtures/faces.yml
@@ -5,3 +5,7 @@ trusting:
 weather_beaten:
   description: weather beaten
   man: steve
+
+confused:
+  description: confused
+  polymorphic_man: gordon (Man)
diff --git a/activerecord/test/fixtures/interests.yml b/activerecord/test/fixtures/interests.yml
index ec71890ab6..9200a19d5a 100644
--- a/activerecord/test/fixtures/interests.yml
+++ b/activerecord/test/fixtures/interests.yml
@@ -23,7 +23,11 @@ woodsmanship:
   zine: going_out
   man: steve
 
-survial:
+survival:
   topic: Survival
   zine: going_out
   man: steve
+
+llama_wrangling:
+  topic: Llama Wrangling
+  polymorphic_man: gordon (Man)
diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb
index 1540dbf741..3e2bdc0307 100644
--- a/activerecord/test/models/face.rb
+++ b/activerecord/test/models/face.rb
@@ -1,5 +1,6 @@
 class Face < ActiveRecord::Base
   belongs_to :man, :inverse_of => :face
+  belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_face
   # This is a "broken" inverse_of for the purposes of testing
   belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face
 end
diff --git a/activerecord/test/models/interest.rb b/activerecord/test/models/interest.rb
index d8291d00cc..d5d9226204 100644
--- a/activerecord/test/models/interest.rb
+++ b/activerecord/test/models/interest.rb
@@ -1,4 +1,5 @@
 class Interest < ActiveRecord::Base
   belongs_to :man, :inverse_of => :interests
+  belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_interests
   belongs_to :zine, :inverse_of => :interests
 end
diff --git a/activerecord/test/models/man.rb b/activerecord/test/models/man.rb
index f40bc9d0fc..4bff92dc98 100644
--- a/activerecord/test/models/man.rb
+++ b/activerecord/test/models/man.rb
@@ -1,6 +1,8 @@
 class Man < ActiveRecord::Base
   has_one :face, :inverse_of => :man
+  has_one :polymorphic_face, :class_name => 'Face', :as => :polymorphic_man, :inverse_of => :polymorphic_man
   has_many :interests, :inverse_of => :man
+  has_many :polymorphic_interests, :class_name => 'Interest', :as => :polymorphic_man, :inverse_of => :polymorphic_man
   # These are "broken" inverse_of associations for the purposes of testing
   has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man
   has_many :secret_interests, :class_name => 'Interest', :inverse_of => :secret_man
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 0dd9da4c11..1ec36e7832 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -520,11 +520,15 @@ ActiveRecord::Schema.define do
   create_table :faces, :force => true do |t|
     t.string  :description
     t.integer :man_id
+    t.integer :polymorphic_man_id
+    t.string :polymorphic_man_type
   end
 
   create_table :interests, :force => true do |t|
     t.string :topic
     t.integer :man_id
+    t.integer :polymorphic_man_id
+    t.string :polymorphic_man_type
     t.integer :zine_id
   end
 

From 6a74ee7f4deea4a44520d3fcc9120e0bb848823f Mon Sep 17 00:00:00 2001
From: Murray Steele <muz@h-lame.com>
Date: Thu, 17 Dec 2009 12:19:10 +0000
Subject: [PATCH 4/8] Provide a slightly more robust
 we_can_set_the_inverse_on_this? method for polymorphic belongs_to
 associations. [#3520 state:resolved]

Also add a new test for polymorphic belongs_to that test direct accessor assignment, not just .replace assignment.

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 .../belongs_to_polymorphic_association.rb       |  9 +++++++--
 .../associations/inverse_associations_test.rb   | 17 ++++++++++++++++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
index 9678300003..f6edd6383c 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -27,7 +27,12 @@ module ActiveRecord
         # NOTE - for now, we're only supporting inverse setting from belongs_to back onto
         # has_one associations.
         def we_can_set_the_inverse_on_this?(record)
-          @reflection.has_inverse? && @reflection.polymorphic_inverse_of(record.class).macro == :has_one
+          if @reflection.has_inverse?
+            inverse_association = @reflection.polymorphic_inverse_of(record.class)
+            inverse_association && inverse_association.macro == :has_one
+          else
+            false
+          end
         end
 
         def set_inverse_instance(record, instance)
@@ -52,7 +57,7 @@ module ActiveRecord
             else
               association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
             end
-          set_inverse_instance(target, @owner) if target
+          set_inverse_instance(target, @owner)
           target
         end
 
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index 2ab3aa141a..0696f06e5b 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -481,7 +481,22 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
     assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
   end
 
-  def test_child_instance_should_be_shared_with_replaced_parent
+  def test_child_instance_should_be_shared_with_replaced_via_accessor_parent
+    face = faces(:confused)
+    old_man = face.polymorphic_man
+    new_man = Man.new
+
+    assert_not_nil face.polymorphic_man
+    face.polymorphic_man = new_man
+
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance"
+    face.description = 'Bongo'
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance"
+    new_man.polymorphic_face.description = 'Mungo'
+    assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance"
+  end
+
+  def test_child_instance_should_be_shared_with_replaced_via_method_parent
     face = faces(:confused)
     old_man = face.polymorphic_man
     new_man = Man.new

From ff508640e28914da2b546f6a8c9f215bab201b61 Mon Sep 17 00:00:00 2001
From: Murray Steele <muz@h-lame.com>
Date: Mon, 28 Dec 2009 14:13:33 +0100
Subject: [PATCH 5/8] Make polymorphic_inverse_of in Reflection throw an
 InverseOfAssociationNotFoundError if the supplied class doesn't have the
 appropriate association. [#3520 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 activerecord/lib/active_record/associations.rb  |  4 ++--
 activerecord/lib/active_record/reflection.rb    | 10 +++++-----
 .../associations/inverse_associations_test.rb   | 17 +++++++++++++++--
 activerecord/test/models/face.rb                |  3 ++-
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index c23c9f63f1..ff8c63bc91 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -3,8 +3,8 @@ require 'active_support/core_ext/enumerable'
 
 module ActiveRecord
   class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc:
-    def initialize(reflection)
-      super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{reflection.class_name})")
+    def initialize(reflection, associated_class = nil)
+      super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{associated_class.nil? ? reflection.class_name : associated_class.name})")
     end
   end
 
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 72f7df32c7..b751c9ad68 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -239,16 +239,16 @@ module ActiveRecord
       def inverse_of
         if has_inverse?
           @inverse_of ||= klass.reflect_on_association(options[:inverse_of])
-        else
-          nil
         end
       end
 
       def polymorphic_inverse_of(associated_class)
         if has_inverse?
-          associated_class.reflect_on_association(options[:inverse_of])
-        else
-          nil
+          if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of])
+            inverse_relationship
+          else
+            raise InverseOfAssociationNotFoundError.new(self, associated_class)
+          end
         end
       end
 
diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb
index 0696f06e5b..457c4da9bf 100644
--- a/activerecord/test/cases/associations/inverse_associations_test.rb
+++ b/activerecord/test/cases/associations/inverse_associations_test.rb
@@ -524,8 +524,21 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
     assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance"
   end
 
-  def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
-    assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man }
+  def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error
+    # Ideally this would, if only for symmetry's sake with other association types
+    assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man }
+  end
+
+  def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error
+    # fails because no class has the correct inverse_of for horrible_polymorphic_man
+    assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man = Man.first }
+  end
+
+  def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error
+    # passes because Man does have the correct inverse_of
+    assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Man.first }
+    # fails because Interest does have the correct inverse_of
+    assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Interest.first }
   end
 end
 
diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb
index 3e2bdc0307..edb75d333f 100644
--- a/activerecord/test/models/face.rb
+++ b/activerecord/test/models/face.rb
@@ -1,6 +1,7 @@
 class Face < ActiveRecord::Base
   belongs_to :man, :inverse_of => :face
   belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_face
-  # This is a "broken" inverse_of for the purposes of testing
+  # These is a "broken" inverse_of for the purposes of testing
   belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face
+  belongs_to :horrible_polymorphic_man, :polymorphic => true, :inverse_of => :horrible_polymorphic_face
 end

From 9c771a9608f54ebdfcb6fca819c83038489ce50d Mon Sep 17 00:00:00 2001
From: Eloy Duran <eloy.de.enige@gmail.com>
Date: Tue, 15 Dec 2009 16:40:02 +0100
Subject: [PATCH 6/8] Make sure to not add autosave callbacks multiple times.
 [#3575 state:resolved]

This makes sure that, in a HABTM association, only one join record is craeted.
---
 .../lib/active_record/autosave_association.rb | 40 ++++++++++++-------
 .../lib/active_record/nested_attributes.rb    |  4 +-
 .../test/cases/autosave_association_test.rb   | 29 ++++++++++++++
 .../test/cases/nested_attributes_test.rb      |  9 +++++
 4 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index c0d8904bc8..44c668b619 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -155,6 +155,13 @@ module ActiveRecord
 
       # Adds a validate and save callback for the association as specified by
       # the +reflection+.
+      #
+      # For performance reasons, we don't check whether to validate at runtime,
+      # but instead only define the method and callback when needed. However,
+      # this can change, for instance, when using nested attributes. Since we
+      # don't want the callbacks to get defined multiple times, there are
+      # guards that check if the save or validation methods have already been
+      # defined before actually defining them.
       def add_autosave_association_callbacks(reflection)
         save_method = "autosave_associated_records_for_#{reflection.name}"
         validation_method = "validate_associated_records_for_#{reflection.name}"
@@ -162,28 +169,33 @@ module ActiveRecord
 
         case reflection.macro
         when :has_many, :has_and_belongs_to_many
-          before_save :before_save_collection_association
+          unless method_defined?(save_method)
+            before_save :before_save_collection_association
 
-          define_method(save_method) { save_collection_association(reflection) }
-          # Doesn't use after_save as that would save associations added in after_create/after_update twice
-          after_create save_method
-          after_update save_method
+            define_method(save_method) { save_collection_association(reflection) }
+            # Doesn't use after_save as that would save associations added in after_create/after_update twice
+            after_create save_method
+            after_update save_method
+          end
 
-          if force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false)
+          if !method_defined?(validation_method) &&
+              (force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false))
             define_method(validation_method) { validate_collection_association(reflection) }
             validate validation_method
           end
         else
-          case reflection.macro
-          when :has_one
-            define_method(save_method) { save_has_one_association(reflection) }
-            after_save save_method
-          when :belongs_to
-            define_method(save_method) { save_belongs_to_association(reflection) }
-            before_save save_method
+          unless method_defined?(save_method)
+            case reflection.macro
+            when :has_one
+              define_method(save_method) { save_has_one_association(reflection) }
+              after_save save_method
+            when :belongs_to
+              define_method(save_method) { save_belongs_to_association(reflection) }
+              before_save save_method
+            end
           end
 
-          if force_validation
+          if !method_defined?(validation_method) && force_validation
             define_method(validation_method) { validate_single_association(reflection) }
             validate validation_method
           end
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index ca3110a374..386f0e7ca7 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -235,7 +235,7 @@ module ActiveRecord
             end
 
             reflection.options[:autosave] = true
-
+            add_autosave_association_callbacks(reflection)
             self.nested_attributes_options[association_name.to_sym] = options
 
             if options[:reject_if] == :all_blank
@@ -250,8 +250,6 @@ module ActiveRecord
                 assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes)
               end
             }, __FILE__, __LINE__
-
-            add_autosave_association_callbacks(reflection)
           else
             raise ArgumentError, "No association found for name `#{association_name}'. Has it been defined yet?"
           end
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 9164701601..803e5b25b1 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -31,11 +31,40 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
     assert base.valid_keys_for_has_and_belongs_to_many_association.include?(:autosave)
   end
 
+  def test_should_not_add_the_same_callbacks_multiple_times_for_has_one
+    assert_no_difference_when_adding_callbacks_twice_for Pirate, :ship
+  end
+
+  def test_should_not_add_the_same_callbacks_multiple_times_for_belongs_to
+    assert_no_difference_when_adding_callbacks_twice_for Ship, :pirate
+  end
+
+  def test_should_not_add_the_same_callbacks_multiple_times_for_has_many
+    assert_no_difference_when_adding_callbacks_twice_for Pirate, :birds
+  end
+
+  def test_should_not_add_the_same_callbacks_multiple_times_for_has_and_belongs_to_many
+    assert_no_difference_when_adding_callbacks_twice_for Pirate, :parrots
+  end
+
   private
 
   def base
     ActiveRecord::Base
   end
+
+  def assert_no_difference_when_adding_callbacks_twice_for(model, association_name)
+    reflection = model.reflect_on_association(association_name)
+    assert_no_difference "callbacks_for_model(#{model.name}).length" do
+      model.send(:add_autosave_association_callbacks, reflection)
+    end
+  end
+
+  def callbacks_for_model(model)
+    model.instance_variables.grep(/_callbacks$/).map do |ivar|
+      model.instance_variable_get(ivar)
+    end.flatten
+  end
 end
 
 class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 53fd168e1b..5367907fb7 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -371,6 +371,15 @@ module NestedAttributesOnACollectionAssociationTests
     assert_respond_to @pirate, association_setter
   end
 
+  def test_should_save_only_one_association_on_create
+    pirate = Pirate.create!({
+      :catchphrase => 'Arr',
+      association_getter => { 'foo' => { :name => 'Grace OMalley' } }
+    })
+
+    assert_equal 1, pirate.reload.send(@association_name).count
+  end
+
   def test_should_take_a_hash_with_string_keys_and_assign_the_attributes_to_the_associated_models
     @alternate_params[association_getter].stringify_keys!
     @pirate.update_attributes @alternate_params

From 07b615fb897017d7acfaafa88606bc88be30f6e4 Mon Sep 17 00:00:00 2001
From: Michael Siebert <siebertm85@googlemail.com>
Date: Mon, 28 Dec 2009 19:02:01 +0100
Subject: [PATCH 7/8] Add an :update_only option to
 accepts_nested_attributes_for for to-one associations. [#2563 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
---
 .../lib/active_record/nested_attributes.rb    | 26 ++++++++++++++--
 .../test/cases/nested_attributes_test.rb      | 31 +++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 386f0e7ca7..5143e804d1 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -212,6 +212,11 @@ module ActiveRecord
       #   nested attributes array exceeds the specified limit, NestedAttributes::TooManyRecords
       #   exception is raised. If omitted, any number associations can be processed.
       #   Note that the :limit option is only applicable to one-to-many associations.
+      # [:update_only]
+      #   Allows to specify that the an existing record can only be updated.
+      #   A new record in only created when there is no existing record. This
+      #   option only works for on-to-one associations and is ignored for
+      #   collection associations. This option is off by default.
       #
       # Examples:
       #   # creates avatar_attributes=
@@ -221,9 +226,9 @@ module ActiveRecord
       #   # creates avatar_attributes= and posts_attributes=
       #   accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true
       def accepts_nested_attributes_for(*attr_names)
-        options = { :allow_destroy => false }
+        options = { :allow_destroy => false, :update_only => false }
         options.update(attr_names.extract_options!)
-        options.assert_valid_keys(:allow_destroy, :reject_if, :limit)
+        options.assert_valid_keys(:allow_destroy, :reject_if, :limit, :update_only)
 
         attr_names.each do |association_name|
           if reflection = reflect_on_association(association_name)
@@ -288,6 +293,13 @@ module ActiveRecord
     # record’s id, then the existing record will be modified. Otherwise a new
     # record will be built.
     #
+    # If update_only is true, a new record is only created when no object exists,
+    # otherwise it will be updated
+    #
+    # If update_only is false and the given attributes include an <tt>:id</tt>
+    # that matches the existing record’s id, then the existing record will be
+    # modified. Otherwise a new record will be built.
+    #
     # If the given attributes include a matching <tt>:id</tt> attribute _and_ a
     # <tt>:_destroy</tt> key set to a truthy value, then the existing record
     # will be marked for destruction.
@@ -295,7 +307,15 @@ module ActiveRecord
       options = self.nested_attributes_options[association_name]
       attributes = attributes.with_indifferent_access
 
-      if attributes['id'].blank?
+      if options[:update_only]
+        if existing_record = send(association_name)
+          assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
+        else
+          unless reject_new_record?(association_name, attributes)
+            send("build_#{association_name}", attributes.except(*UNASSIGNABLE_KEYS))
+          end
+        end
+      elsif attributes['id'].blank?
         unless reject_new_record?(association_name, attributes)
           method = "build_#{association_name}"
           if respond_to?(method)
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 5367907fb7..5e4fc2b8d9 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -245,6 +245,37 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase
   def test_should_automatically_enable_autosave_on_the_association
     assert Pirate.reflect_on_association(:ship).options[:autosave]
   end
+
+  def test_should_accept_update_only_option
+    Pirate.accepts_nested_attributes_for :ship, :update_only => true
+    @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' })
+
+    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  end
+
+  def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
+    Pirate.accepts_nested_attributes_for :ship, :update_only => true
+    @ship.delete
+
+    assert_difference('Ship.count', 1) do
+      @pirate.reload.update_attribute(:ship_attributes, { :name => 'Mayflower' })
+    end
+
+    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  end
+
+
+  def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
+    Pirate.accepts_nested_attributes_for :ship, :update_only => true
+
+    assert_no_difference('Ship.count') do
+      @pirate.reload.update_attributes(:ship_attributes => { :name => 'Mayflower' })
+    end
+
+    assert_equal 'Mayflower', @ship.reload.name
+
+    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  end
 end
 
 class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase

From c23fbd0d475612fe9cd493bd08c8da2f8d7e6f03 Mon Sep 17 00:00:00 2001
From: Eloy Duran <eloy.de.enige@gmail.com>
Date: Mon, 28 Dec 2009 21:08:20 +0100
Subject: [PATCH 8/8] Refactored previous changes to nested attributes.

---
 .../lib/active_record/nested_attributes.rb    | 50 +++++++------------
 .../test/cases/nested_attributes_test.rb      | 41 +++++++++------
 activerecord/test/models/pirate.rb            |  2 +
 activerecord/test/models/ship.rb              |  2 +
 4 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 5143e804d1..ff3a51d5c0 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -213,9 +213,9 @@ module ActiveRecord
       #   exception is raised. If omitted, any number associations can be processed.
       #   Note that the :limit option is only applicable to one-to-many associations.
       # [:update_only]
-      #   Allows to specify that the an existing record can only be updated.
-      #   A new record in only created when there is no existing record. This
-      #   option only works for on-to-one associations and is ignored for
+      #   Allows you to specify that an existing record may only be updated.
+      #   A new record may only be created when there is no existing record.
+      #   This option only works for one-to-one associations and is ignored for
       #   collection associations. This option is off by default.
       #
       # Examples:
@@ -248,7 +248,7 @@ module ActiveRecord
             end
 
             # def pirate_attributes=(attributes)
-            #   assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false)
+            #   assign_nested_attributes_for_one_to_one_association(:pirate, attributes)
             # end
             class_eval %{
               def #{association_name}_attributes=(attributes)
@@ -289,43 +289,29 @@ module ActiveRecord
 
     # Assigns the given attributes to the association.
     #
-    # If the given attributes include an <tt>:id</tt> that matches the existing
-    # record’s id, then the existing record will be modified. Otherwise a new
-    # record will be built.
-    #
-    # If update_only is true, a new record is only created when no object exists,
-    # otherwise it will be updated
-    #
     # If update_only is false and the given attributes include an <tt>:id</tt>
     # that matches the existing record’s id, then the existing record will be
-    # modified. Otherwise a new record will be built.
+    # modified. If update_only is true, a new record is only created when no
+    # object exists. Otherwise a new record will be built.
     #
-    # If the given attributes include a matching <tt>:id</tt> attribute _and_ a
-    # <tt>:_destroy</tt> key set to a truthy value, then the existing record
-    # will be marked for destruction.
+    # If the given attributes include a matching <tt>:id</tt> attribute, or
+    # update_only is true, and a <tt>:_destroy</tt> key set to a truthy value,
+    # then the existing record will be marked for destruction.
     def assign_nested_attributes_for_one_to_one_association(association_name, attributes)
       options = self.nested_attributes_options[association_name]
       attributes = attributes.with_indifferent_access
+      check_existing_record = (options[:update_only] || !attributes['id'].blank?)
 
-      if options[:update_only]
-        if existing_record = send(association_name)
-          assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
+      if check_existing_record && (record = send(association_name)) &&
+          (options[:update_only] || record.id.to_s == attributes['id'].to_s)
+        assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy])
+      elsif !reject_new_record?(association_name, attributes)
+        method = "build_#{association_name}"
+        if respond_to?(method)
+          send(method, attributes.except(*UNASSIGNABLE_KEYS))
         else
-          unless reject_new_record?(association_name, attributes)
-            send("build_#{association_name}", attributes.except(*UNASSIGNABLE_KEYS))
-          end
+          raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?"
         end
-      elsif attributes['id'].blank?
-        unless reject_new_record?(association_name, attributes)
-          method = "build_#{association_name}"
-          if respond_to?(method)
-            send(method, attributes.except(*UNASSIGNABLE_KEYS))
-          else
-            raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?"
-          end
-        end
-      elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s
-        assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
       end
     end
 
diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb
index 5e4fc2b8d9..60c5bad225 100644
--- a/activerecord/test/cases/nested_attributes_test.rb
+++ b/activerecord/test/cases/nested_attributes_test.rb
@@ -247,34 +247,24 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase
   end
 
   def test_should_accept_update_only_option
-    Pirate.accepts_nested_attributes_for :ship, :update_only => true
-    @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' })
-
-    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+    @pirate.update_attribute(:update_only_ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' })
   end
 
   def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
-    Pirate.accepts_nested_attributes_for :ship, :update_only => true
     @ship.delete
-
     assert_difference('Ship.count', 1) do
-      @pirate.reload.update_attribute(:ship_attributes, { :name => 'Mayflower' })
+      @pirate.reload.update_attribute(:update_only_ship_attributes, { :name => 'Mayflower' })
     end
-
-    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
   end
 
-
   def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
-    Pirate.accepts_nested_attributes_for :ship, :update_only => true
+    @ship.delete
+    @ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning')
 
     assert_no_difference('Ship.count') do
-      @pirate.reload.update_attributes(:ship_attributes => { :name => 'Mayflower' })
+      @pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
     end
-
     assert_equal 'Mayflower', @ship.reload.name
-
-    Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
   end
 end
 
@@ -393,6 +383,27 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase
   def test_should_automatically_enable_autosave_on_the_association
     assert Ship.reflect_on_association(:pirate).options[:autosave]
   end
+
+  def test_should_accept_update_only_option
+    @ship.update_attribute(:update_only_pirate_attributes, { :id => @pirate.ship.id, :catchphrase => 'Arr' })
+  end
+
+  def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
+    @pirate.delete
+    assert_difference('Pirate.count', 1) do
+      @ship.reload.update_attribute(:update_only_pirate_attributes, { :catchphrase => 'Arr' })
+    end
+  end
+
+  def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
+    @pirate.delete
+    @pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye')
+
+    assert_no_difference('Pirate.count') do
+      @ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' })
+    end
+    assert_equal 'Arr', @pirate.reload.catchphrase
+  end
 end
 
 module NestedAttributesOnACollectionAssociationTests
diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb
index f2c05dd48f..88c1634717 100644
--- a/activerecord/test/models/pirate.rb
+++ b/activerecord/test/models/pirate.rb
@@ -19,6 +19,7 @@ class Pirate < ActiveRecord::Base
 
   # These both have :autosave enabled because accepts_nested_attributes_for is used on them.
   has_one :ship
+  has_one :update_only_ship, :class_name => 'Ship'
   has_one :non_validated_ship, :class_name => 'Ship'
   has_many :birds
   has_many :birds_with_method_callbacks, :class_name => "Bird",
@@ -35,6 +36,7 @@ class Pirate < ActiveRecord::Base
 
   accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
   accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  accepts_nested_attributes_for :update_only_ship, :update_only => true
   accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks,
     :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true
   accepts_nested_attributes_for :birds_with_reject_all_blank, :reject_if => :all_blank
diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb
index 06759d64b8..a96e38ab41 100644
--- a/activerecord/test/models/ship.rb
+++ b/activerecord/test/models/ship.rb
@@ -2,9 +2,11 @@ class Ship < ActiveRecord::Base
   self.record_timestamps = false
 
   belongs_to :pirate
+  belongs_to :update_only_pirate, :class_name => 'Pirate'
   has_many :parts, :class_name => 'ShipPart', :autosave => true
 
   accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  accepts_nested_attributes_for :update_only_pirate, :update_only => true
 
   validates_presence_of :name
 end