Merge remote branch 'jonleighton/association_fixes' into fuuu

* jonleighton/association_fixes:
  Rename AssociationReflection#primary_key_name to foreign_key, since the options key which it relates to is :foreign_key
  Support for :counter_cache on polymorphic belongs_to
  Refactor BelongsToAssociation to allow BelongsToPolymorphicAssociation to inherit from it
  Specify the STI type condition using SQL IN rather than a whole load of ORs. Required a fix to ActiveRecord::Relation#merge for properly merging create_with_value. This also fixes a situation where the type condition was appearing twice in the resultant SQL query.
  Verify that when has_many associated objects are destroyed via :dependent => :destroy, when the parent is destroyed, the callbacks are run
  Get rid of extra_conditions param from configure_dependency_for_has_many. I can't see a particularly plausible argument for this being used by plugins, and if they really want they can just redefine the callback or whatever. Note also that before my recent commit the extra_conditions param was completely ignored for :dependent => :destroy.
  And owner_quoted_id can go too
  Now we can drop-kick AssociationReflection#dependent_conditions into oblivion.
  Refactor configure_dependency_for_has_many to use AssociationCollection#delete_all. It was necessary to change test_before_destroy in lifecycle_test.rb so that it checks topic.replies.size *before* doing the destroy, as afterwards it will now (correctly) be 0.

7 files changed, 90 insertions, 13 deletions

diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index cdde9a80d3..f97f89b6fe 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -198,16 +198,23 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_equal 1, Post.find(p.id).comments.size
   end
 
-  def test_belongs_to_with_primary_key_counter_with_assigning_nil
-    debate = Topic.create("title" => "debate")
-    reply  = Reply.create("title" => "blah!", "content" => "world around!", "parent_title" => "debate")
+  def test_belongs_to_with_primary_key_counter
+    debate  = Topic.create("title" => "debate")
+    debate2 = Topic.create("title" => "debate2")
+    reply   = Reply.create("title" => "blah!", "content" => "world around!", "parent_title" => "debate")
+
+    assert_equal 1, debate.reload.replies_count
+    assert_equal 0, debate2.reload.replies_count
 
-    assert_equal debate.title, reply.parent_title
-    assert_equal 1, Topic.find(debate.id).send(:read_attribute, "replies_count")
+    reply.topic_with_primary_key = debate2
+
+    assert_equal 0, debate.reload.replies_count
+    assert_equal 1, debate2.reload.replies_count
 
     reply.topic_with_primary_key = nil
 
-    assert_equal 0, Topic.find(debate.id).send(:read_attribute, "replies_count")
+    assert_equal 0, debate.reload.replies_count
+    assert_equal 0, debate2.reload.replies_count
   end
 
   def test_belongs_to_counter_with_reassigning
@@ -419,6 +426,18 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert_nil sponsor.sponsorable_id
   end
 
+  def test_assignment_updates_foreign_id_field_for_new_and_saved_records
+    client = Client.new
+    saved_firm = Firm.create :name => "Saved"
+    new_firm = Firm.new
+
+    client.firm = saved_firm
+    assert_equal saved_firm.id, client.client_of
+
+    client.firm = new_firm
+    assert_nil client.client_of
+  end
+
   def test_polymorphic_assignment_with_primary_key_updates_foreign_id_field_for_new_and_saved_records
     essay = Essay.new
     saved_writer = Author.create(:name => "David")
@@ -537,4 +556,27 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     assert proxy.stale_target?
     assert_equal companies(:first_firm), sponsor.sponsorable
   end
+
+  def test_reloading_association_with_key_change
+    client = companies(:second_client)
+    firm = client.firm # note this is a proxy object
+
+    client.firm = companies(:another_firm)
+    assert_equal companies(:another_firm), firm.reload
+
+    client.client_of = companies(:first_firm).id
+    assert_equal companies(:first_firm), firm.reload
+  end
+
+  def test_polymorphic_counter_cache
+    tagging = taggings(:welcome_general)
+    post    = posts(:welcome)
+    comment = comments(:greetings)
+
+    assert_difference 'post.reload.taggings_count', -1 do
+      assert_difference 'comment.reload.taggings_count', +1 do
+        tagging.taggable = comment
+      end
+    end
+  end
 end
diff --git a/activerecord/test/cases/associations/callbacks_test.rb b/activerecord/test/cases/associations/callbacks_test.rb
index 6a30e2905b..2d0d4541b4 100644
--- a/activerecord/test/cases/associations/callbacks_test.rb
+++ b/activerecord/test/cases/associations/callbacks_test.rb
@@ -3,6 +3,7 @@ require 'models/post'
 require 'models/author'
 require 'models/project'
 require 'models/developer'
+require 'models/company'
 
 class AssociationCallbacksTest < ActiveRecord::TestCase
   fixtures :posts, :authors, :projects, :developers
@@ -81,6 +82,14 @@ class AssociationCallbacksTest < ActiveRecord::TestCase
     assert_equal callback_log, jack.post_log
   end
 
+  def test_has_many_callbacks_for_destroy_on_parent
+    firm = Firm.create! :name => "Firm"
+    client = firm.clients.create! :name => "Client"
+    firm.destroy
+
+    assert_equal ["before_remove#{client.id}", "after_remove#{client.id}"], firm.log
+  end
+
   def test_has_and_belongs_to_many_add_callback
     david = developers(:david)
     ar = projects(:active_record)
diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb
index b8c3ffb9cb..0558deb71b 100644
--- a/activerecord/test/cases/lifecycle_test.rb
+++ b/activerecord/test/cases/lifecycle_test.rb
@@ -101,9 +101,10 @@ class LifecycleTest < ActiveRecord::TestCase
   fixtures :topics, :developers, :minimalistics
 
   def test_before_destroy
-    original_count = Topic.count
-    (topic_to_be_destroyed = Topic.find(1)).destroy
-    assert_equal original_count - (1 + topic_to_be_destroyed.replies.size), Topic.count
+    topic = Topic.find(1)
+    assert_difference 'Topic.count', -(1 + topic.replies.size) do
+      topic.destroy
+    end
   end
 
   def test_auto_observer
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 901c12b26c..e3db34520e 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -8,6 +8,8 @@ require 'models/ship'
 require 'models/pirate'
 require 'models/price_estimate'
 require 'models/tagging'
+require 'models/author'
+require 'models/post'
 
 class ReflectionTest < ActiveRecord::TestCase
   include ActiveRecord::Reflection
@@ -127,11 +129,11 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_belongs_to_inferred_foreign_key_from_assoc_name
     Company.belongs_to :foo
-    assert_equal "foo_id", Company.reflect_on_association(:foo).primary_key_name
+    assert_equal "foo_id", Company.reflect_on_association(:foo).foreign_key
     Company.belongs_to :bar, :class_name => "Xyzzy"
-    assert_equal "bar_id", Company.reflect_on_association(:bar).primary_key_name
+    assert_equal "bar_id", Company.reflect_on_association(:bar).foreign_key
     Company.belongs_to :baz, :class_name => "Xyzzy", :foreign_key => "xyzzy_id"
-    assert_equal "xyzzy_id", Company.reflect_on_association(:baz).primary_key_name
+    assert_equal "xyzzy_id", Company.reflect_on_association(:baz).foreign_key
   end
 
   def test_association_reflection_in_modules
diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb
index e8672515fc..7c6899d438 100644
--- a/activerecord/test/cases/relation_scoping_test.rb
+++ b/activerecord/test/cases/relation_scoping_test.rb
@@ -485,4 +485,11 @@ class DefaultScopingTest < ActiveRecord::TestCase
     posts_offset_limit = Post.offset(2).limit(3)
     assert_equal posts_limit_offset, posts_offset_limit
   end
+
+  def test_create_with_merge
+    aaron = (PoorDeveloperCalledJamis.create_with(:name => 'foo', :salary => 20) &
+             PoorDeveloperCalledJamis.create_with(:name => 'Aaron')).new
+    assert_equal 20, aaron.salary
+    assert_equal 'Aaron', aaron.name
+  end
 end
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index ee5f77b613..7af4dfe2c9 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -38,7 +38,9 @@ end
 class Firm < Company
   has_many :clients, :order => "id", :dependent => :destroy, :counter_sql =>
       "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " +
-      "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )"
+      "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )",
+      :before_remove => :log_before_remove,
+      :after_remove  => :log_after_remove
   has_many :unsorted_clients, :class_name => "Client"
   has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC"
   has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id"
@@ -88,6 +90,19 @@ class Firm < Company
   has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
   has_many :accounts
   has_many :unautosaved_accounts, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
+
+  def log
+    @log ||= []
+  end
+
+  private
+    def log_before_remove(record)
+      log << "before_remove#{record.id}"
+    end
+
+    def log_after_remove(record)
+      log << "after_remove#{record.id}"
+    end
 end
 
 class DependentFirm < Company
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 3dea7e1492..7f366b2c91 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -143,6 +143,7 @@ ActiveRecord::Schema.define do
       t.text    :body, :null => false
     end
     t.string  :type
+    t.integer :taggings_count, :default => 0
   end
 
   create_table :companies, :force => true do |t|