Ensure replacing has_one associations respects the supplied :dependent option. [#1305 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

4 files changed, 33 insertions, 4 deletions

diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 960323004d..b92cbbdeab 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -29,8 +29,17 @@ module ActiveRecord
 
         unless @target.nil? || @target == obj
           if dependent? && !dont_save
-            @target.destroy unless @target.new_record?
-            @owner.clear_association_cache
+            case @reflection.options[:dependent]
+            when :delete
+              @target.delete unless @target.new_record?
+              @owner.clear_association_cache
+            when :destroy
+              @target.destroy unless @target.new_record?
+              @owner.clear_association_cache
+            when :nullify
+              @target[@reflection.primary_key_name] = nil
+              @target.save unless @owner.new_record? || @target.new_record?
+            end
           else
             @target[@reflection.primary_key_name] = nil
             @target.save unless @owner.new_record? || @target.new_record?
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index e81bba790a..0aafbcee36 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -79,6 +79,24 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) }
   end
 
+  def test_nullification_on_association_change
+    firm = companies(:rails_core)
+    old_account_id = firm.account.id
+    firm.account = Account.new
+    # account is dependent with nullify, therefore its firm_id should be nil
+    assert_nil Account.find(old_account_id).firm_id
+  end
+
+  def test_association_changecalls_delete
+    companies(:first_firm).deletable_account = Account.new
+    assert_equal [], Account.destroyed_account_ids[companies(:first_firm).id]
+  end
+
+  def test_association_change_calls_destroy
+    companies(:first_firm).account = Account.new
+    assert_equal [companies(:first_firm).id], Account.destroyed_account_ids[companies(:first_firm).id]
+  end
+
   def test_natural_assignment_to_already_associated_record
     company = companies(:first_firm)
     account = accounts(:signals37)
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 8b1c714ead..34ebd99989 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -4,6 +4,7 @@ require 'models/customer'
 require 'models/company'
 require 'models/company_in_module'
 require 'models/subscriber'
+require 'models/pirate'
 
 class ReflectionTest < ActiveRecord::TestCase
   fixtures :topics, :customers, :companies, :subscribers
@@ -169,9 +170,9 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_reflection_of_all_associations
     # FIXME these assertions bust a lot
-    assert_equal 26, Firm.reflect_on_all_associations.size
+    assert_equal 27, Firm.reflect_on_all_associations.size
     assert_equal 20, Firm.reflect_on_all_associations(:has_many).size
-    assert_equal 6, Firm.reflect_on_all_associations(:has_one).size
+    assert_equal 7, 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/models/company.rb b/activerecord/test/models/company.rb
index 3b27a9e272..a47992a8e7 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -69,6 +69,7 @@ class Firm < Company
   has_one :account_with_select, :foreign_key => "firm_id", :select => "id, firm_id", :class_name=>'Account'
   has_one :readonly_account, :foreign_key => "firm_id", :class_name => "Account", :readonly => true
   has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account"
+  has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete
 end
 
 class DependentFirm < Company