Merge branch 'master' into nested_has_many_through

Conflicts:
	activerecord/CHANGELOG
	activerecord/lib/active_record/association_preload.rb
	activerecord/lib/active_record/associations.rb
	activerecord/lib/active_record/associations/class_methods/join_dependency.rb
	activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
	activerecord/lib/active_record/associations/has_many_association.rb
	activerecord/lib/active_record/associations/has_many_through_association.rb
	activerecord/lib/active_record/associations/has_one_association.rb
	activerecord/lib/active_record/associations/has_one_through_association.rb
	activerecord/lib/active_record/associations/through_association_scope.rb
	activerecord/lib/active_record/reflection.rb
	activerecord/test/cases/associations/has_many_through_associations_test.rb
	activerecord/test/cases/associations/has_one_through_associations_test.rb
	activerecord/test/cases/reflection_test.rb
	activerecord/test/cases/relations_test.rb
	activerecord/test/fixtures/memberships.yml
	activerecord/test/models/categorization.rb
	activerecord/test/models/category.rb
	activerecord/test/models/member.rb
	activerecord/test/models/reference.rb
	activerecord/test/models/tagging.rb

1 files changed, 213 insertions, 93 deletions

diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index fbf7121468..8f55b7ebe6 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -21,19 +21,19 @@ require 'models/eye'
 
 class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
   def test_autosave_should_be_a_valid_option_for_has_one
-    assert base.valid_keys_for_has_one_association.include?(:autosave)
+    assert ActiveRecord::Associations::Builder::HasOne.valid_options.include?(:autosave)
   end
 
   def test_autosave_should_be_a_valid_option_for_belongs_to
-    assert base.valid_keys_for_belongs_to_association.include?(:autosave)
+    assert ActiveRecord::Associations::Builder::BelongsTo.valid_options.include?(:autosave)
   end
 
   def test_autosave_should_be_a_valid_option_for_has_many
-    assert base.valid_keys_for_has_many_association.include?(:autosave)
+    assert ActiveRecord::Associations::Builder::HasMany.valid_options.include?(:autosave)
   end
 
   def test_autosave_should_be_a_valid_option_for_has_and_belongs_to_many
-    assert base.valid_keys_for_has_and_belongs_to_many_association.include?(:autosave)
+    assert ActiveRecord::Associations::Builder::HasAndBelongsToMany.valid_options.include?(:autosave)
   end
 
   def test_should_not_add_the_same_callbacks_multiple_times_for_has_one
@@ -90,7 +90,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas
     firm = Firm.find(:first)
     assert firm.valid?
 
-    firm.account = Account.new
+    firm.build_account
 
     assert !firm.account.valid?
     assert !firm.valid?
@@ -102,7 +102,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas
     firm = Firm.find(:first)
     assert firm.valid?
 
-    firm.unvalidated_account = Account.new
+    firm.build_unvalidated_account
 
     assert !firm.unvalidated_account.valid?
     assert firm.valid?
@@ -112,7 +112,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas
   def test_build_before_child_saved
     firm = Firm.find(1)
 
-    account = firm.account.build("credit_limit" => 1000)
+    account = firm.build_account("credit_limit" => 1000)
     assert_equal account, firm.account
     assert !account.persisted?
     assert firm.save
@@ -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
@@ -565,7 +572,7 @@ class TestDefaultAutosaveAssociationOnNewRecord < ActiveRecord::TestCase
 end
 
 class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
@@ -578,7 +585,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     @pirate.ship.mark_for_destruction
 
     assert !@pirate.reload.marked_for_destruction?
-    assert !@pirate.ship.marked_for_destruction?
+    assert !@pirate.ship.reload.marked_for_destruction?
   end
 
   # has_one
@@ -667,119 +674,227 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
       end
     end
 
+    @ship.pirate.catchphrase = "Changed Catchphrase"
+
     assert_raise(RuntimeError) { assert !@ship.save }
     assert_not_nil @ship.reload.pirate
   end
 
-  # has_many & has_and_belongs_to
-  %w{ parrots birds }.each do |association_name|
-    define_method("test_should_destroy_#{association_name}_as_part_of_the_save_transaction_if_they_were_marked_for_destroyal") do
-      2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") }
+  def test_should_save_changed_child_objects_if_parent_is_saved
+    @pirate = @ship.create_pirate(:catchphrase => "Don' botharrr talkin' like one, savvy?")
+    @parrot = @pirate.parrots.create!(:name => 'Posideons Killer')
+    @parrot.name = "NewName"
+    @ship.save
+
+    assert_equal 'NewName', @parrot.reload.name
+  end
+
+  def test_should_destroy_has_many_as_part_of_the_save_transaction_if_they_were_marked_for_destruction
+    2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") }
 
-      assert !@pirate.send(association_name).any? { |child| child.marked_for_destruction? }
+    assert !@pirate.birds.any? { |child| child.marked_for_destruction? }
 
-      @pirate.send(association_name).each { |child| child.mark_for_destruction }
-      klass = @pirate.send(association_name).first.class
-      ids = @pirate.send(association_name).map(&:id)
+    @pirate.birds.each { |child| child.mark_for_destruction }
+    klass = @pirate.birds.first.class
+    ids = @pirate.birds.map(&:id)
 
-      assert @pirate.send(association_name).all? { |child| child.marked_for_destruction? }
-      ids.each { |id| assert klass.find_by_id(id) }
+    assert @pirate.birds.all? { |child| child.marked_for_destruction? }
+    ids.each { |id| assert klass.find_by_id(id) }
 
-      @pirate.save
-      assert @pirate.reload.send(association_name).empty?
-      ids.each { |id| assert_nil klass.find_by_id(id) }
+    @pirate.save
+    assert @pirate.reload.birds.empty?
+    ids.each { |id| assert_nil klass.find_by_id(id) }
+  end
+
+  def test_should_skip_validation_on_has_many_if_marked_for_destruction
+    2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") }
+
+    @pirate.birds.each { |bird| bird.name = '' }
+    assert !@pirate.valid?
+
+    @pirate.birds.each do |bird|
+      bird.mark_for_destruction
+      bird.expects(:valid?).never
     end
+    assert_difference("Bird.count", -2) { @pirate.save! }
+  end
 
-    define_method("test_should_skip_validation_on_the_#{association_name}_association_if_marked_for_destruction") do
-      2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") }
-      children = @pirate.send(association_name)
+  def test_should_skip_validation_on_has_many_if_destroyed
+    @pirate.birds.create!(:name => "birds_1")
 
-      children.each { |child| child.name = '' }
-      assert !@pirate.valid?
+    @pirate.birds.each { |bird| bird.name = '' }
+    assert !@pirate.valid?
 
-      children.each do |child|
-        child.mark_for_destruction
-        child.expects(:valid?).never
+    @pirate.birds.each { |bird| bird.destroy }
+    assert @pirate.valid?
+  end
+
+  def test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_has_many
+    @pirate.birds.create!(:name => "birds_1")
+
+    @pirate.birds.each { |bird| bird.mark_for_destruction }
+    assert @pirate.save
+
+    @pirate.birds.each { |bird| bird.expects(:destroy).never }
+    assert @pirate.save
+  end
+
+  def test_should_rollback_destructions_if_an_exception_occurred_while_saving_has_many
+    2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") }
+    before = @pirate.birds.map { |c| c.mark_for_destruction ; c }
+
+    # Stub the destroy method of the second child to raise an exception
+    class << before.last
+      def destroy(*args)
+        super
+        raise 'Oh noes!'
       end
-      assert_difference("#{association_name.classify}.count", -2) { @pirate.save! }
     end
 
-    define_method("test_should_skip_validation_on_the_#{association_name}_association_if_destroyed") do
-      @pirate.send(association_name).create!(:name => "#{association_name}_1")
-      children = @pirate.send(association_name)
+    assert_raise(RuntimeError) { assert !@pirate.save }
+    assert_equal before, @pirate.reload.birds
+  end
+
+  # Add and remove callbacks tests for association collections.
+  %w{ method proc }.each do |callback_type|
+    define_method("test_should_run_add_callback_#{callback_type}s_for_has_many") do
+      association_name_with_callbacks = "birds_with_#{callback_type}_callbacks"
+
+      pirate = Pirate.new(:catchphrase => "Arr")
+      pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed")
 
-      children.each { |child| child.name = '' }
-      assert !@pirate.valid?
+      expected = [
+        "before_adding_#{callback_type}_bird_<new>",
+        "after_adding_#{callback_type}_bird_<new>"
+      ]
 
-      children.each { |child| child.destroy }
-      assert @pirate.valid?
+      assert_equal expected, pirate.ship_log
     end
 
-    define_method("test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_#{association_name}") do
-      @pirate.send(association_name).create!(:name => "#{association_name}_1")
-      children = @pirate.send(association_name)
+    define_method("test_should_run_remove_callback_#{callback_type}s_for_has_many") do
+      association_name_with_callbacks = "birds_with_#{callback_type}_callbacks"
 
-      children.each { |child| child.mark_for_destruction }
-      assert @pirate.save
-      children.each { |child| child.expects(:destroy).never }
-      assert @pirate.save
+      @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed")
+      @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction }
+      child_id = @pirate.send(association_name_with_callbacks).first.id
+
+      @pirate.ship_log.clear
+      @pirate.save
+
+      expected = [
+        "before_removing_#{callback_type}_bird_#{child_id}",
+        "after_removing_#{callback_type}_bird_#{child_id}"
+      ]
+
+      assert_equal expected, @pirate.ship_log
     end
+  end
 
-    define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do
-      2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") }
-      before = @pirate.send(association_name).map { |c| c.mark_for_destruction ; c }
+  def test_should_destroy_habtm_as_part_of_the_save_transaction_if_they_were_marked_for_destruction
+    2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") }
 
-      # Stub the destroy method of the the second child to raise an exception
-      class << before.last
-        def destroy(*args)
-          super
-          raise 'Oh noes!'
-        end
-      end
+    assert !@pirate.parrots.any? { |parrot| parrot.marked_for_destruction? }
+    @pirate.parrots.each { |parrot| parrot.mark_for_destruction }
 
-      assert_raise(RuntimeError) { assert !@pirate.save }
-      assert_equal before, @pirate.reload.send(association_name)
+    assert_no_difference "Parrot.count" do
+      @pirate.save
     end
 
-    # Add and remove callbacks tests for association collections.
-    %w{ method proc }.each do |callback_type|
-      define_method("test_should_run_add_callback_#{callback_type}s_for_#{association_name}") do
-        association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks"
+    assert @pirate.reload.parrots.empty?
 
-        pirate = Pirate.new(:catchphrase => "Arr")
-        pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed")
+    join_records = Pirate.connection.select_all("SELECT * FROM parrots_pirates WHERE pirate_id = #{@pirate.id}")
+    assert join_records.empty?
+  end
 
-        expected = [
-          "before_adding_#{callback_type}_#{association_name.singularize}_<new>",
-          "after_adding_#{callback_type}_#{association_name.singularize}_<new>"
-        ]
+  def test_should_skip_validation_on_habtm_if_marked_for_destruction
+    2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") }
 
-        assert_equal expected, pirate.ship_log
-      end
+    @pirate.parrots.each { |parrot| parrot.name = '' }
+    assert !@pirate.valid?
+
+    @pirate.parrots.each do |parrot|
+      parrot.mark_for_destruction
+      parrot.expects(:valid?).never
+    end
+
+    @pirate.save!
+    assert @pirate.reload.parrots.empty?
+  end
+
+  def test_should_skip_validation_on_habtm_if_destroyed
+    @pirate.parrots.create!(:name => "parrots_1")
 
-      define_method("test_should_run_remove_callback_#{callback_type}s_for_#{association_name}") do
-        association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks"
+    @pirate.parrots.each { |parrot| parrot.name = '' }
+    assert !@pirate.valid?
 
-        @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed")
-        @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction }
-        child_id = @pirate.send(association_name_with_callbacks).first.id
+    @pirate.parrots.each { |parrot| parrot.destroy }
+    assert @pirate.valid?
+  end
 
-        @pirate.ship_log.clear
-        @pirate.save
+  def test_a_child_marked_for_destruction_should_not_be_destroyed_twice_while_saving_habtm
+    @pirate.parrots.create!(:name => "parrots_1")
+
+    @pirate.parrots.each { |parrot| parrot.mark_for_destruction }
+    assert @pirate.save
+
+    assert_no_queries do
+      assert @pirate.save
+    end
+  end
 
-        expected = [
-          "before_removing_#{callback_type}_#{association_name.singularize}_#{child_id}",
-          "after_removing_#{callback_type}_#{association_name.singularize}_#{child_id}"
-        ]
+  def test_should_rollback_destructions_if_an_exception_occurred_while_saving_habtm
+    2.times { |i| @pirate.parrots.create!(:name => "parrots_#{i}") }
+    before = @pirate.parrots.map { |c| c.mark_for_destruction ; c }
 
-        assert_equal expected, @pirate.ship_log
+    class << @pirate.parrots
+      def destroy(*args)
+        super
+        raise 'Oh noes!'
       end
     end
+
+    assert_raise(RuntimeError) { assert !@pirate.save }
+    assert_equal before, @pirate.reload.parrots
+  end
+
+  # Add and remove callbacks tests for association collections.
+  %w{ method proc }.each do |callback_type|
+    define_method("test_should_run_add_callback_#{callback_type}s_for_habtm") do
+      association_name_with_callbacks = "parrots_with_#{callback_type}_callbacks"
+
+      pirate = Pirate.new(:catchphrase => "Arr")
+      pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed")
+
+      expected = [
+        "before_adding_#{callback_type}_parrot_<new>",
+        "after_adding_#{callback_type}_parrot_<new>"
+      ]
+
+      assert_equal expected, pirate.ship_log
+    end
+
+    define_method("test_should_run_remove_callback_#{callback_type}s_for_habtm") do
+      association_name_with_callbacks = "parrots_with_#{callback_type}_callbacks"
+
+      @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed")
+      @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction }
+      child_id = @pirate.send(association_name_with_callbacks).first.id
+
+      @pirate.ship_log.clear
+      @pirate.save
+
+      expected = [
+        "before_removing_#{callback_type}_parrot_#{child_id}",
+        "after_removing_#{callback_type}_parrot_#{child_id}"
+      ]
+
+      assert_equal expected, @pirate.ship_log
+    end
   end
 end
 
 class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
@@ -850,7 +965,10 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
     values = [@pirate.reload.catchphrase, @pirate.ship.name, *@pirate.ship.parts.map(&:name)]
     # Oracle saves empty string as NULL
     if current_adapter?(:OracleAdapter)
-      assert_equal [nil, nil, nil, nil], values
+      expected = ActiveRecord::IdentityMap.enabled? ?
+        [nil, nil, '', ''] :
+        [nil, nil, nil, nil]
+      assert_equal expected, values
     else
       assert_equal ['', '', '', ''], values
     end
@@ -899,7 +1017,7 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
 end
 
 class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @ship = Ship.create(:name => 'Nights Dirty Lightning')
@@ -945,7 +1063,8 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
     @ship.save(:validate => false)
     # Oracle saves empty string as NULL
     if current_adapter?(:OracleAdapter)
-      assert_equal [nil, nil], [@ship.reload.name, @ship.pirate.catchphrase]
+      expected = ActiveRecord::IdentityMap.enabled? ?  [nil, ''] : [nil, nil]
+      assert_equal expected, [@ship.reload.name, @ship.pirate.catchphrase]
     else
       assert_equal ['', ''], [@ship.reload.name, @ship.pirate.catchphrase]
     end
@@ -1146,7 +1265,7 @@ module AutosaveAssociationOnACollectionAssociationTests
 end
 
 class TestAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @association_name = :birds
@@ -1160,7 +1279,7 @@ class TestAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCase
 end
 
 class TestAutosaveAssociationOnAHasAndBelongsToManyAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @association_name = :parrots
@@ -1175,7 +1294,7 @@ class TestAutosaveAssociationOnAHasAndBelongsToManyAssociation < ActiveRecord::T
 end
 
 class TestAutosaveAssociationValidationsOnAHasManyAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
@@ -1191,11 +1310,12 @@ class TestAutosaveAssociationValidationsOnAHasManyAssociation < ActiveRecord::Te
 end
 
 class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
     @pirate.create_ship(:name => 'titanic')
+    super
   end
 
   test "should automatically validate associations with :validate => true" do
@@ -1204,7 +1324,7 @@ class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::Tes
     assert !@pirate.valid?
   end
 
-  test "should not automatically validate associations without :validate => true" do
+  test "should not automatically asd validate associations without :validate => true" do
     assert @pirate.valid?
     @pirate.non_validated_ship.name = ''
     assert @pirate.valid?
@@ -1212,7 +1332,7 @@ class TestAutosaveAssociationValidationsOnAHasOneAssociation < ActiveRecord::Tes
 end
 
 class TestAutosaveAssociationValidationsOnABelongsToAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
@@ -1232,7 +1352,7 @@ class TestAutosaveAssociationValidationsOnABelongsToAssociation < ActiveRecord::
 end
 
 class TestAutosaveAssociationValidationsOnAHABTMAssociation < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?")
@@ -1254,7 +1374,7 @@ class TestAutosaveAssociationValidationsOnAHABTMAssociation < ActiveRecord::Test
 end
 
 class TestAutosaveAssociationValidationMethodsGeneration < ActiveRecord::TestCase
-  self.use_transactional_fixtures = false
+  self.use_transactional_fixtures = false unless supports_savepoints?
 
   def setup
     @pirate = Pirate.new