From a0be389d39b790e0625339251d2674b8250b16b1 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 2 Jan 2011 14:28:53 +0000 Subject: Allow assignment on has_one :through where the owner is a new record [#5137 state:resolved] This required changing the code to keep the association proxy for a belongs_to around, despite its target being nil. Which in turn required various changes to the way that stale target checking is handled, in order to support various edge cases (loaded target is nil then foreign key added, foreign key is changed and then changed back, etc). A side effect is that the code is nicer and more succinct. Note that I am removing test_no_unexpected_aliasing since that is basically checking that the proxy for a belongs_to *does* change, which is the exact opposite of the intention of this commit. Also adding various tests for various edge cases and related things. Phew, long commit message! --- .../associations/belongs_to_associations_test.rb | 33 +++++++++------------- activerecord/test/cases/associations/eager_test.rb | 20 +++++++++++-- .../has_one_through_associations_test.rb | 22 +++++++++++++++ .../test/cases/autosave_association_test.rb | 7 +++++ activerecord/test/cases/nested_attributes_test.rb | 1 - 5 files changed, 60 insertions(+), 23 deletions(-) (limited to 'activerecord/test/cases') diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index f697fdf628..38ee4ad4e0 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -88,19 +88,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_not_nil citibank_result.instance_variable_get("@firm_with_primary_key_symbols") end - def test_no_unexpected_aliasing - first_firm = companies(:first_firm) - another_firm = companies(:another_firm) - - citibank = Account.create("credit_limit" => 10) - citibank.firm = first_firm - original_proxy = citibank.firm - citibank.firm = another_firm - - assert_equal first_firm.object_id, original_proxy.target.object_id - assert_equal another_firm.object_id, citibank.firm.target.object_id - end - def test_creating_the_belonging_object citibank = Account.create("credit_limit" => 10) apple = citibank.create_firm("name" => "Apple") @@ -318,13 +305,21 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal Firm.find(:first, :order => "id"), c.firm_with_basic_id end - def test_forgetting_the_load_when_foreign_key_enters_late - c = Client.new - assert_nil c.firm_with_basic_id + def test_setting_foreign_key_after_nil_target_loaded + client = Client.new + client.firm_with_basic_id + client.firm_id = 1 - c.firm_id = 1 - # sometimes tests on Oracle fail if ORDER BY is not provided therefore add always :order with :first - assert_equal Firm.find(:first, :order => "id"), c.firm_with_basic_id + assert_equal companies(:first_firm), client.firm_with_basic_id + end + + def test_polymorphic_setting_foreign_key_after_nil_target_loaded + sponsor = Sponsor.new + sponsor.sponsorable + sponsor.sponsorable_id = 1 + sponsor.sponsorable_type = "Member" + + assert_equal members(:groucho), sponsor.sponsorable end def test_field_name_same_as_foreign_key diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 19c69ed7bc..2a9351507a 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -902,11 +902,25 @@ class EagerAssociationTest < ActiveRecord::TestCase end end - def test_preloading_empty_polymorphic_parent + def test_preloading_empty_belongs_to + c = Client.create!(:name => 'Foo', :client_of => Company.maximum(:id) + 1) + + client = assert_queries(2) { Client.preload(:firm).find(c.id) } + assert_no_queries { assert_nil client.firm } + end + + def test_preloading_empty_belongs_to_polymorphic t = Tagging.create!(:taggable_type => 'Post', :taggable_id => Post.maximum(:id) + 1, :tag => tags(:general)) - assert_queries(2) { @tagging = Tagging.preload(:taggable).find(t.id) } - assert_no_queries { assert ! @tagging.taggable } + tagging = assert_queries(2) { Tagging.preload(:taggable).find(t.id) } + assert_no_queries { assert_nil tagging.taggable } + end + + def test_preloading_through_empty_belongs_to + c = Client.create!(:name => 'Foo', :client_of => Company.maximum(:id) + 1) + + client = assert_queries(2) { Client.preload(:accounts).find(c.id) } + assert_no_queries { assert client.accounts.empty? } end def test_preloading_has_many_through_with_uniq diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 7d55d909a7..0afbef5c87 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -266,4 +266,26 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert proxy.stale_target? assert_equal dashboards(:second), minivan.dashboard end + + def test_has_one_through_belongs_to_setting_belongs_to_foreign_key_after_nil_target_loaded + minivan = Minivan.new + + minivan.dashboard + proxy = minivan.send(:association_instance_get, :dashboard) + + minivan.speedometer_id = speedometers(:second).id + + assert proxy.stale_target? + assert_equal dashboards(:second), minivan.dashboard + end + + def test_assigning_has_one_through_belongs_to_with_new_record_owner + minivan = Minivan.new + dashboard = dashboards(:cool_first) + + minivan.dashboard = dashboard + + assert_equal dashboard, minivan.dashboard + assert_equal dashboard, minivan.speedometer.dashboard + end end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 27aee400f9..71fd3fd836 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -340,6 +340,13 @@ class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::Test tags(:misc).create_tagging(:taggable => posts(:thinking)) assert_equal num_tagging + 1, Tagging.count end + + def test_build_and_then_save_parent_should_not_reload_target + client = Client.find(:first) + apple = client.build_firm(:name => "Apple") + client.save! + assert_no_queries { assert_equal apple, client.firm } + end end class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 7d9b1104cd..d290afc1dd 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -411,7 +411,6 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id @pirate.stubs(:id).returns('ABC1X') - @ship.stubs(:pirate_id).returns(@pirate.id) # prevents pirate from being reloaded due to non-matching foreign key @ship.pirate_attributes = { :id => @pirate.id, :catchphrase => 'Arr' } assert_equal 'Arr', @ship.pirate.catchphrase -- cgit v1.2.3