Eager loading of dependent has_one associations won't delete the association...

Eager loading of dependent has_one associations won't delete the association #1212. Also, starting to refactor the tests to make them speedier, with optional support for transactional fixtures.


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1398 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/CHANGELOG

 *SVN*
 
+* Eager loading of dependent has_one associations won't delete the association #1212
+
 * Added a second parameter to the build and create method for has_one that controls whether the existing association should be replaced (which means nullifying its foreign key as well). By default this is true, but false can be passed to prevent it.
 
 * Using transactional fixtures now causes the data to be loaded only once.

activerecord/lib/active_record/associations.rb

@@ -719,7 +719,7 @@ def find_with_associations(options = {})
                   next unless row[primary_key_table[reflection.table_name]]
 
                   record.send(
-                    "#{reflection.name}=", 
+                    "set_#{reflection.name}_target", 
                     reflection.klass.send(:instantiate, extract_record(schema_abbreviations, reflection.table_name, row))
                   )
               end

activerecord/test/abstract_unit.rb

@@ -19,3 +19,6 @@ def create_fixtures(*table_names)
 end
 
 Test::Unit::TestCase.fixture_path = File.dirname(__FILE__) + "/fixtures/"
+#Test::Unit::TestCase.use_instantiated_fixtures = false
+#Test::Unit::TestCase.use_transactional_fixtures = (ENV['AR_TX_FIXTURES'] == "yes")
+

activerecord/test/associations_go_eager_test.rb

@@ -3,47 +3,49 @@
 require 'fixtures/comment'
 require 'fixtures/author'
 require 'fixtures/category'
+require 'fixtures/company'
 
 class EagerAssociationTest < Test::Unit::TestCase
-  fixtures :posts, :comments, :authors, :categories, :categories_posts
+  fixtures :posts, :comments, :authors, :categories, :categories_posts,
+            :companies, :accounts
 
   def test_loading_with_one_association
     posts = Post.find(:all, :include => :comments)
     assert_equal 2, posts.first.comments.size
-    assert posts.first.comments.include?(@greetings)
+    assert posts.first.comments.include?(comments(:greetings))
 
     post = Post.find(:first, :include => :comments, :conditions => "posts.title = 'Welcome to the weblog'")
     assert_equal 2, post.comments.size
-    assert post.comments.include?(@greetings)
+    assert post.comments.include?(comments(:greetings))
   end
 
   def test_with_ordering
     posts = Post.find(:all, :include => :comments, :order => "posts.id DESC")
-    assert_equal @authorless, posts[0]
-    assert_equal @thinking, posts[1]
-    assert_equal @welcome, posts[2]
+    assert_equal posts(:authorless), posts[0]
+    assert_equal posts(:thinking), posts[1]
+    assert_equal posts(:welcome), posts[2]
   end
   
   def test_loading_with_multiple_associations
     posts = Post.find(:all, :include => [ :comments, :author, :categories ], :order => "posts.id")
     assert_equal 2, posts.first.comments.size
     assert_equal 2, posts.first.categories.size
-    assert posts.first.comments.include?(@greetings)
+    assert posts.first.comments.include?(comments(:greetings))
   end
 
   def test_loading_from_an_association
-    posts = @david.posts.find(:all, :include => :comments, :order => "posts.id")
+    posts = authors(:david).posts.find(:all, :include => :comments, :order => "posts.id")
     assert_equal 2, posts.first.comments.size
   end
 
   def test_loading_with_no_associations
-    assert_nil Post.find(@authorless.id, :include => :author).author
+    assert_nil Post.find(posts(:authorless).id, :include => :author).author
   end
 
   def test_eager_association_loading_with_belongs_to
     comments = Comment.find(:all, :include => :post)
-    assert_equal @welcome.title, comments.first.post.title
-    assert_equal @thinking.title, comments.last.post.title
+    assert_equal posts(:welcome).title, comments.first.post.title
+    assert_equal posts(:thinking).title, comments.last.post.title
   end
 
   def test_eager_association_loading_with_habtm
@@ -51,12 +53,19 @@ def test_eager_association_loading_with_habtm
     assert_equal 2, posts[0].categories.size
     assert_equal 1, posts[1].categories.size
     assert_equal 0, posts[2].categories.size
-    assert posts[0].categories.include?(@technology)
-    assert posts[1].categories.include?(@general)
+    assert posts[0].categories.include?(categories(:technology))
+    assert posts[1].categories.include?(categories(:general))
   end
   
   def test_eager_with_inheritance
     posts = SpecialPost.find(:all, :include => [ :comments ])
   end  
+
+  def test_eager_with_has_one_dependent_does_not_destroy_dependent
+    assert_not_nil companies(:first_firm).account
+    f = Firm.find(:first, :include => :account,
+            :conditions => ["companies.name = ?", "37signals"])
+    assert_not_nil companies(:first_firm, :reload).account
+  end
 end
 

activerecord/test/associations_test.rb

@@ -17,10 +17,8 @@ class Car < ActiveRecord::Base; has_many :wheels, :name => "wheels"; end
 
 
 class AssociationsTest < Test::Unit::TestCase
-  def setup
-    create_fixtures "accounts", "companies", "developers", "projects", "developers_projects", "computers"
-    @signals37 = Firm.find(1)
-  end
+  fixtures :accounts, :companies, :developers, :projects, :developers_projects,
+           :computers
   
   def test_force_reload
     firm = Firm.new("name" => "A New Firm, Inc")
@@ -63,23 +61,20 @@ def test_storing_in_pstore
 end
 
 class HasOneAssociationsTest < Test::Unit::TestCase
-  def setup
-    create_fixtures "accounts", "companies", "developers", "projects", "developers_projects"
-    @signals37 = Firm.find(1)
-  end
+  fixtures :accounts, :companies, :developers, :projects, :developers_projects
   
   def test_has_one
-    assert_equal @signals37.account, Account.find(1)
-    assert_equal Account.find(1).credit_limit, @signals37.account.credit_limit
+    assert_equal companies(:first_firm).account, Account.find(1)
+    assert_equal Account.find(1).credit_limit, companies(:first_firm).account.credit_limit
   end
 
   def test_triple_equality
-    assert Account === @signals37.account
+    assert Account === companies(:first_firm).account
   end
 
   def test_type_mismatch
-    assert_raises(ActiveRecord::AssociationTypeMismatch) { @signals37.account = 1 }
-    assert_raises(ActiveRecord::AssociationTypeMismatch) { @signals37.account = Project.find(1) }
+    assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = 1 }
+    assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = Project.find(1) }
   end
 
   def test_natural_assignment
@@ -90,10 +85,10 @@ def test_natural_assignment
   end
   
   def test_natural_assignment_to_nil
-    old_account_id = @signals37.account.id
-    @signals37.account = nil
-    @signals37.save
-    assert_nil @signals37.account
+    old_account_id = companies(:first_firm).account.id
+    companies(:first_firm).account = nil
+    companies(:first_firm).save
+    assert_nil companies(:first_firm).account
     # account is dependent, therefore is destroyed when reference to owner is lost
     assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } 
   end
@@ -121,7 +116,7 @@ def test_assignment_without_replacement_on_create
     apple.account = citibank
     assert_equal apple.id, citibank.firm_id
     
-    hsbc = apple.create_account({ :name => "HSBC", :credit_limit => 10}, false)
+    hsbc = apple.create_account({:credit_limit => 10}, false)
     assert_equal apple.id, hsbc.firm_id
     hsbc.save
     assert_equal apple.id, citibank.firm_id
@@ -256,14 +251,11 @@ def test_assignment_before_either_saved
 
 
 class HasManyAssociationsTest < Test::Unit::TestCase
-  fixtures :accounts, :companies, :developers, :projects, :developers_projects, :topics
-  
-  def setup
-    @signals37 = Firm.find(1)
-  end
+  fixtures :accounts, :companies, :developers, :projects,
+           :developers_projects, :topics
   
   def force_signal37_to_load_all_clients_of_firm
-    @signals37.clients_of_firm.each {|f| }
+    companies(:first_firm).clients_of_firm.each {|f| }
   end
   
   def test_counting
@@ -366,30 +358,30 @@ def test_find_first_sanitized
   end
 
   def test_find_in_collection
-    assert_equal Client.find(2).name, @signals37.clients.find(2).name
-    assert_raises(ActiveRecord::RecordNotFound) { @signals37.clients.find(6) }
+    assert_equal Client.find(2).name, companies(:first_firm).clients.find(2).name
+    assert_raises(ActiveRecord::RecordNotFound) { companies(:first_firm).clients.find(6) }
   end
 
   def test_adding
     force_signal37_to_load_all_clients_of_firm
     natural = Client.new("name" => "Natural Company")
-    @signals37.clients_of_firm << natural
-    assert_equal 2, @signals37.clients_of_firm.size # checking via the collection
-    assert_equal 2, @signals37.clients_of_firm(true).size # checking using the db
-    assert_equal natural, @signals37.clients_of_firm.last
+    companies(:first_firm).clients_of_firm << natural
+    assert_equal 2, companies(:first_firm).clients_of_firm.size # checking via the collection
+    assert_equal 2, companies(:first_firm).clients_of_firm(true).size # checking using the db
+    assert_equal natural, companies(:first_firm).clients_of_firm.last
   end
   
   def test_adding_a_mismatch_class
-    assert_raises(ActiveRecord::AssociationTypeMismatch) { @signals37.clients_of_firm << nil }
-    assert_raises(ActiveRecord::AssociationTypeMismatch) { @signals37.clients_of_firm << 1 }
-    assert_raises(ActiveRecord::AssociationTypeMismatch) { @signals37.clients_of_firm << Topic.find(1) }
+    assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << nil }
+    assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << 1 }
+    assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << Topic.find(1) }
   end
   
   def test_adding_a_collection
     force_signal37_to_load_all_clients_of_firm
-    @signals37.clients_of_firm.concat([Client.new("name" => "Natural Company"), Client.new("name" => "Apple")])
-    assert_equal 3, @signals37.clients_of_firm.size
-    assert_equal 3, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.concat([Client.new("name" => "Natural Company"), Client.new("name" => "Apple")])
+    assert_equal 3, companies(:first_firm).clients_of_firm.size
+    assert_equal 3, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_adding_before_save
@@ -435,51 +427,51 @@ def test_invalid_adding_before_save
   end
 
   def test_build
-    new_client = @signals37.clients_of_firm.build("name" => "Another Client")
+    new_client = companies(:first_firm).clients_of_firm.build("name" => "Another Client")
     assert_equal "Another Client", new_client.name
     assert new_client.new_record?
-    assert_equal new_client, @signals37.clients_of_firm.last
-    assert @signals37.save
+    assert_equal new_client, companies(:first_firm).clients_of_firm.last
+    assert companies(:first_firm).save
     assert !new_client.new_record?
-    assert_equal 2, @signals37.clients_of_firm(true).size
+    assert_equal 2, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_build_many
-    new_clients = @signals37.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}])
+    new_clients = companies(:first_firm).clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}])
     assert_equal 2, new_clients.size
 
-    assert @signals37.save
-    assert_equal 3, @signals37.clients_of_firm(true).size
+    assert companies(:first_firm).save
+    assert_equal 3, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_invalid_build
-    new_client = @signals37.clients_of_firm.build
+    new_client = companies(:first_firm).clients_of_firm.build
     assert new_client.new_record?
     assert !new_client.valid?
-    assert_equal new_client, @signals37.clients_of_firm.last
-    assert !@signals37.save
+    assert_equal new_client, companies(:first_firm).clients_of_firm.last
+    assert !companies(:first_firm).save
     assert new_client.new_record?
-    assert_equal 1, @signals37.clients_of_firm(true).size
+    assert_equal 1, companies(:first_firm).clients_of_firm(true).size
   end
   
   def test_create
     force_signal37_to_load_all_clients_of_firm
-    new_client = @signals37.clients_of_firm.create("name" => "Another Client")
+    new_client = companies(:first_firm).clients_of_firm.create("name" => "Another Client")
     assert !new_client.new_record?
-    assert_equal new_client, @signals37.clients_of_firm.last
-    assert_equal new_client, @signals37.clients_of_firm(true).last
+    assert_equal new_client, companies(:first_firm).clients_of_firm.last
+    assert_equal new_client, companies(:first_firm).clients_of_firm(true).last
   end
   
   def test_create_many
-    @signals37.clients_of_firm.create([{"name" => "Another Client"}, {"name" => "Another Client II"}])
-    assert_equal 3, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.create([{"name" => "Another Client"}, {"name" => "Another Client II"}])
+    assert_equal 3, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_deleting
     force_signal37_to_load_all_clients_of_firm
-    @signals37.clients_of_firm.delete(@signals37.clients_of_firm.first)
-    assert_equal 0, @signals37.clients_of_firm.size
-    assert_equal 0, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.delete(companies(:first_firm).clients_of_firm.first)
+    assert_equal 0, companies(:first_firm).clients_of_firm.size
+    assert_equal 0, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_deleting_before_save
@@ -492,29 +484,29 @@ def test_deleting_before_save
 
   def test_deleting_a_collection
     force_signal37_to_load_all_clients_of_firm
-    @signals37.clients_of_firm.create("name" => "Another Client")
-    assert_equal 2, @signals37.clients_of_firm.size
-    #@signals37.clients_of_firm.clear
-    @signals37.clients_of_firm.delete([@signals37.clients_of_firm[0], @signals37.clients_of_firm[1]])
-    assert_equal 0, @signals37.clients_of_firm.size
-    assert_equal 0, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.create("name" => "Another Client")
+    assert_equal 2, companies(:first_firm).clients_of_firm.size
+    #companies(:first_firm).clients_of_firm.clear
+    companies(:first_firm).clients_of_firm.delete([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1]])
+    assert_equal 0, companies(:first_firm).clients_of_firm.size
+    assert_equal 0, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_deleting_a_association_collection
     force_signal37_to_load_all_clients_of_firm
-    @signals37.clients_of_firm.create("name" => "Another Client")
-    assert_equal 2, @signals37.clients_of_firm.size
-    @signals37.clients_of_firm.clear
-    assert_equal 0, @signals37.clients_of_firm.size
-    assert_equal 0, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.create("name" => "Another Client")
+    assert_equal 2, companies(:first_firm).clients_of_firm.size
+    companies(:first_firm).clients_of_firm.clear
+    assert_equal 0, companies(:first_firm).clients_of_firm.size
+    assert_equal 0, companies(:first_firm).clients_of_firm(true).size
   end
 
   def test_deleting_a_item_which_is_not_in_the_collection
     force_signal37_to_load_all_clients_of_firm
     summit = Client.find_first("name = 'Summit'")
-    @signals37.clients_of_firm.delete(summit)
-    assert_equal 1, @signals37.clients_of_firm.size
-    assert_equal 1, @signals37.clients_of_firm(true).size
+    companies(:first_firm).clients_of_firm.delete(summit)
+    assert_equal 1, companies(:first_firm).clients_of_firm.size
+    assert_equal 1, companies(:first_firm).clients_of_firm(true).size
     assert_equal 2, summit.client_of
   end
 
@@ -532,10 +524,10 @@ def test_deleting_self_type_mismatch
 
   def test_destroy_all
     force_signal37_to_load_all_clients_of_firm
-    assert !@signals37.clients_of_firm.empty?, "37signals has clients after load"
-    @signals37.clients_of_firm.destroy_all
-    assert @signals37.clients_of_firm.empty?, "37signals has no clients after destroy all"
-    assert @signals37.clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh"
+    assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load"
+    companies(:first_firm).clients_of_firm.destroy_all
+    assert companies(:first_firm).clients_of_firm.empty?, "37signals has no clients after destroy all"
+    assert companies(:first_firm).clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh"
   end
 
   def test_dependence
@@ -577,12 +569,12 @@ def test_dependence_with_transaction_support_on_failure
 
   def test_dependence_on_account
     assert_equal 2, Account.find_all.length
-    @signals37.destroy
+    companies(:first_firm).destroy
     assert_equal 1, Account.find_all.length
   end
 
   def test_included_in_collection
-    assert @signals37.clients.include?(Client.find(2))
+    assert companies(:first_firm).clients.include?(Client.find(2))
   end
 
   def test_adding_array_and_collection
@@ -591,16 +583,12 @@ def test_adding_array_and_collection
 end
 
 class BelongsToAssociationsTest < Test::Unit::TestCase
-  fixtures :accounts, :companies, :developers, :projects, :topics
+  fixtures :accounts, :companies, :developers, :projects, :topics,
+           :developers_projects
   
-  def setup
-    create_fixtures "developers_projects"
-    @signals37 = Firm.find(1)
-  end
-
   def test_belongs_to
     Client.find(3).firm.name
-    assert_equal @signals37.name, Client.find(3).firm.name
+    assert_equal companies(:first_firm).name, Client.find(3).firm.name
     assert !Client.find(3).firm.nil?, "Microsoft should have a firm"   
   end
 
@@ -854,16 +842,16 @@ def test_create
   end
   
   def test_uniq_after_the_fact
-    @developers["jamis"].find.projects << @projects["active_record"].find
-    @developers["jamis"].find.projects << @projects["active_record"].find
-    assert_equal 3, @developers["jamis"].find.projects.size
-    assert_equal 1, @developers["jamis"].find.projects.uniq.size
+    developers(:jamis).projects << projects(:active_record)
+    developers(:jamis).projects << projects(:active_record)
+    assert_equal 3, developers(:jamis).projects.size
+    assert_equal 1, developers(:jamis).projects.uniq.size
   end
 
   def test_uniq_before_the_fact
-    @projects["active_record"].find.developers << @developers["jamis"].find
-    @projects["active_record"].find.developers << @developers["david"].find
-    assert_equal 2, @projects["active_record"].find.developers.size
+    projects(:active_record).developers << developers(:jamis)
+    projects(:active_record).developers << developers(:david)
+    assert_equal 2, projects(:active_record).developers.size
   end
   
   def test_deleting
@@ -934,35 +922,35 @@ def test_destroy_all
   end
 
   def test_rich_association
-    @jamis = @developers["jamis"].find
-    @jamis.projects.push_with_attributes(@projects["action_controller"].find, :joined_on => Date.today)
-    assert_equal Date.today.to_s, @jamis.projects.select { |p| p.name == @projects["action_controller"]["name"] }.first.joined_on.to_s
-    assert_equal Date.today.to_s, @developers["jamis"].find.projects.select { |p| p.name == @projects["action_controller"]["name"] }.first.joined_on.to_s
+    jamis = developers(:jamis)
+    jamis.projects.push_with_attributes(projects(:action_controller), :joined_on => Date.today)
+    assert_equal Date.today.to_s, jamis.projects.select { |p| p.name == projects(:action_controller).name }.first.joined_on.to_s
+    assert_equal Date.today.to_s, developers(:jamis).projects.select { |p| p.name == projects(:action_controller).name }.first.joined_on.to_s
   end
 
   def test_associations_with_conditions
-    assert_equal 2, @projects["active_record"].find.developers.size
-    assert_equal 1, @projects["active_record"].find.developers_named_david.size
+    assert_equal 2, projects(:active_record).developers.size
+    assert_equal 1, projects(:active_record).developers_named_david.size
     
-    @projects["active_record"].find.developers_named_david.clear
-    assert_equal 1, @projects["active_record"].find.developers.size
+    projects(:active_record).developers_named_david.clear
+    assert_equal 1, projects(:active_record).developers.size
   end
   
   def test_find_in_association
     # Using sql
-    assert_equal @developers["david"].find, @projects["active_record"].find.developers.find(@developers["david"]["id"]), "SQL find"
+    assert_equal developers(:david), projects(:active_record).developers.find(developers(:david).id), "SQL find"
     
     # Using ruby
-    @active_record = @projects["active_record"].find
-    @active_record.developers.reload
-    assert_equal @developers["david"].find, @active_record.developers.find(@developers["david"]["id"]), "Ruby find"
+    active_record = projects(:active_record)
+    active_record.developers.reload
+    assert_equal developers(:david), active_record.developers.find(developers(:david).id), "Ruby find"
   end
 
   def xtest_find_in_association_with_options
-    developers = @active_record.developers.find(:all)
+    developers = projects(:active_record).developers.find(:all)
     assert_equal 2, developers.size
     
-    assert_equal @david, @active_record.developers.find(:first, :conditions => "salary < 10000")
-    assert_equal @jamis, @active_record.developers.find(:first, :order => "salary DESC")
+    assert_equal developers(:david), projects(:active_record).developers.find(:first, :conditions => "salary < 10000")
+    assert_equal developers(:jamis), projects(:active_record).developers.find(:first, :order => "salary DESC")
   end
-end
\ No newline at end of file
+end

activerecord/test/fixtures/company.rb

@@ -21,7 +21,7 @@ class Firm < Company
            :finder_sql  => 'SELECT * FROM companies WHERE client_of = 1000',
            :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000'
 
-  has_one :account, :dependent => true
+  has_one :account, :foreign_key => "firm_id", :dependent => true
 end
 
 class Client < Company