From 38fbfa6390c449dfc9a3db2975e65bd0fd665b18 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 30 Dec 2010 20:41:02 +0000
Subject: 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.

---
 activerecord/lib/active_record/associations.rb | 58 +++++++++-----------------
 activerecord/test/cases/lifecycle_test.rb      |  7 ++--
 2 files changed, 24 insertions(+), 41 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b49cf8de95..17889bebfd 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1617,11 +1617,13 @@ module ActiveRecord
         # Active Record codebase, is meant to allow plugins to define extra
         # finder conditions.
         def configure_dependency_for_has_many(reflection, extra_conditions = nil)
-          if reflection.options.include?(:dependent)
+          if reflection.options[:dependent]
+            method_name = "has_many_dependent_for_#{reflection.name}"
+
             case reflection.options[:dependent]
-              when :destroy
-                method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym
-                define_method(method_name) do
+            when :destroy, :delete_all, :nullify
+              define_method(method_name) do
+                if reflection.options[:dependent] == :destroy
                   send(reflection.name).each do |o|
                     # No point in executing the counter update since we're going to destroy the parent anyway
                     counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
@@ -1633,35 +1635,23 @@ module ActiveRecord
                     o.destroy
                   end
                 end
-                before_destroy method_name
-              when :delete_all
-                before_destroy do |record|
-                  self.class.send(:delete_all_has_many_dependencies,
-                  record,
-                  reflection.name,
-                  reflection.klass,
-                  reflection.dependent_conditions(record, self.class, extra_conditions))
-                end
-              when :nullify
-                before_destroy do |record|
-                  self.class.send(:nullify_has_many_dependencies,
-                  record,
-                  reflection.name,
-                  reflection.klass,
-                  reflection.primary_key_name,
-                  reflection.dependent_conditions(record, self.class, extra_conditions))
+
+                reflection.klass.send(:with_scope, :find => { :conditions => extra_conditions }) do
+                  # AssociationProxy#delete_all looks at the :dependent option and acts accordingly
+                  send(reflection.name).delete_all
                 end
-              when :restrict
-                method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym
-                define_method(method_name) do
-                  unless send(reflection.name).empty?
-                    raise DeleteRestrictionError.new(reflection)
-                  end
+              end
+            when :restrict
+              define_method(method_name) do
+                unless send(reflection.name).empty?
+                  raise DeleteRestrictionError.new(reflection)
                 end
-                before_destroy method_name
-              else
-                raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})"
+              end
+            else
+              raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})"
             end
+
+            before_destroy method_name
           end
         end
 
@@ -1724,14 +1714,6 @@ module ActiveRecord
           end
         end
 
-        def delete_all_has_many_dependencies(record, reflection_name, association_class, dependent_conditions)
-          association_class.delete_all(dependent_conditions)
-        end
-
-        def nullify_has_many_dependencies(record, reflection_name, association_class, primary_key_name, dependent_conditions)
-          association_class.update_all("#{primary_key_name} = NULL", dependent_conditions)
-        end
-
         mattr_accessor :valid_keys_for_has_many_association
         @@valid_keys_for_has_many_association = [
           :class_name, :table_name, :foreign_key, :primary_key,
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
-- 
cgit v1.2.3


From b0bb911fa1923d809b6e4fc120cc1adc0b8dc321 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 30 Dec 2010 20:42:30 +0000
Subject: Now we can drop-kick AssociationReflection#dependent_conditions into
 oblivion.

---
 activerecord/lib/active_record/reflection.rb | 11 -----------
 1 file changed, 11 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 0310e7050d..8c68a1bc00 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -298,17 +298,6 @@ module ActiveRecord
         !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many)
       end
 
-      def dependent_conditions(record, base_class, extra_conditions)
-        dependent_conditions = []
-        dependent_conditions << "#{primary_key_name} = #{record.send(name).send(:owner_quoted_id)}"
-        dependent_conditions << "#{options[:as]}_type = '#{base_class.name}'" if options[:as]
-        dependent_conditions << klass.send(:sanitize_sql, options[:conditions]) if options[:conditions]
-        dependent_conditions << extra_conditions if extra_conditions
-        dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ")
-        dependent_conditions = dependent_conditions.gsub('@', '\@')
-        dependent_conditions
-      end
-
       # Returns +true+ if +self+ is a +belongs_to+ reflection.
       def belongs_to?
         macro == :belongs_to
-- 
cgit v1.2.3


From 6db908515919a22c475bec2dd0ddc157e7933f3d Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 30 Dec 2010 20:43:55 +0000
Subject: And owner_quoted_id can go too

---
 activerecord/lib/active_record/associations/association_proxy.rb | 5 -----
 .../lib/active_record/associations/has_many_association.rb       | 8 --------
 .../lib/active_record/associations/has_one_association.rb        | 9 ---------
 3 files changed, 22 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index 6720d83199..97eab8a793 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -282,11 +282,6 @@ module ActiveRecord
           end
         end
 
-        # Returns the ID of the owner, quoted if needed.
-        def owner_quoted_id
-          @owner.quoted_id
-        end
-
         # Can be redefined by subclasses, notably polymorphic belongs_to
         # The record parameter is necessary to support polymorphic inverses as we must check for
         # the association in the specific class of the record.
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index c3360463cd..a0e2e8b479 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -7,14 +7,6 @@ module ActiveRecord
     # is provided by its child HasManyThroughAssociation.
     class HasManyAssociation < AssociationCollection #:nodoc:
       protected
-        def owner_quoted_id
-          if @reflection.options[:primary_key]
-            @owner.class.quote_value(@owner.send(@reflection.options[:primary_key]))
-          else
-            @owner.quoted_id
-          end
-        end
-
         # Returns the number of records in this collection.
         #
         # If the association has a counter cache it gets that value. Otherwise
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index c32aaf986e..04cb16b909 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -65,15 +65,6 @@ module ActiveRecord
         end
       end
 
-      protected
-        def owner_quoted_id
-          if @reflection.options[:primary_key]
-            @owner.class.quote_value(@owner.send(@reflection.options[:primary_key]))
-          else
-            @owner.quoted_id
-          end
-        end
-
       private
         def find_target
           options = @reflection.options.dup.slice(:select, :order, :include, :readonly)
-- 
cgit v1.2.3


From f7a15d0e03dc173ab5f88773a97a3bf26d7b957e Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 30 Dec 2010 20:47:31 +0000
Subject: 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.

---
 activerecord/lib/active_record/associations.rb | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 17889bebfd..b82651ebce 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1612,11 +1612,7 @@ module ActiveRecord
         #  - set the foreign key to NULL if the option is set to :nullify
         #  - do not delete the parent record if there is any child record if the
         #    option is set to :restrict
-        #
-        # The +extra_conditions+ parameter, which is not used within the main
-        # Active Record codebase, is meant to allow plugins to define extra
-        # finder conditions.
-        def configure_dependency_for_has_many(reflection, extra_conditions = nil)
+        def configure_dependency_for_has_many(reflection)
           if reflection.options[:dependent]
             method_name = "has_many_dependent_for_#{reflection.name}"
 
@@ -1636,10 +1632,8 @@ module ActiveRecord
                   end
                 end
 
-                reflection.klass.send(:with_scope, :find => { :conditions => extra_conditions }) do
-                  # AssociationProxy#delete_all looks at the :dependent option and acts accordingly
-                  send(reflection.name).delete_all
-                end
+                # AssociationProxy#delete_all looks at the :dependent option and acts accordingly
+                send(reflection.name).delete_all
               end
             when :restrict
               define_method(method_name) do
-- 
cgit v1.2.3


From 2bf31868033c50d71d6d68c1ddad67147908adc4 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 30 Dec 2010 21:44:29 +0000
Subject: Verify that when has_many associated objects are destroyed via
 :dependent => :destroy, when the parent is destroyed, the callbacks are run

---
 activerecord/test/cases/associations/callbacks_test.rb |  9 +++++++++
 activerecord/test/models/company.rb                    | 17 ++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

(limited to 'activerecord')

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/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
-- 
cgit v1.2.3


From 62b084f80759300f10a4e5c4235bf1d13693a7d3 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 10:43:42 +0000
Subject: 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.

---
 .../lib/active_record/associations/association_collection.rb |  2 +-
 activerecord/lib/active_record/base.rb                       | 12 ++++++++----
 activerecord/lib/active_record/relation/spawn_methods.rb     |  6 +++++-
 activerecord/test/cases/relation_scoping_test.rb             |  7 +++++++
 4 files changed, 21 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 108e316672..64e10f56e2 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -488,7 +488,7 @@ module ActiveRecord
           ensure_owner_is_persisted!
 
           transaction do
-            with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do
+            with_scope(:create => @scope[:create].merge(scoped.scope_for_create)) do
               build_record(attrs, &block)
             end
           end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 396ab226a9..0a2e436ecc 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -874,7 +874,12 @@ module ActiveRecord #:nodoc:
 
         def relation #:nodoc:
           @relation ||= Relation.new(self, arel_table)
-          finder_needs_type_condition? ? @relation.where(type_condition) : @relation
+
+          if finder_needs_type_condition?
+            @relation.where(type_condition).create_with(inheritance_column.to_sym => sti_name)
+          else
+            @relation
+          end
         end
 
         # Finder methods must instantiate through this method to work with the
@@ -914,10 +919,9 @@ module ActiveRecord #:nodoc:
 
         def type_condition
           sti_column = arel_table[inheritance_column.to_sym]
-          condition = sti_column.eq(sti_name)
-          descendants.each { |subclass| condition = condition.or(sti_column.eq(subclass.sti_name)) }
+          sti_names  = ([self] + descendants).map { |model| model.sti_name }
 
-          condition
+          sti_column.in(sti_names)
         end
 
         # Guesses the table name, but does not decorate it with prefix and suffix information.
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 5acf3ec83a..e1f7fa2949 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -46,13 +46,17 @@ module ActiveRecord
 
       merged_relation.where_values = merged_wheres
 
-      (Relation::SINGLE_VALUE_METHODS - [:lock]).each do |method|
+      (Relation::SINGLE_VALUE_METHODS - [:lock, :create_with]).each do |method|
         value = r.send(:"#{method}_value")
         merged_relation.send(:"#{method}_value=", value) unless value.nil?
       end
 
       merged_relation.lock_value = r.lock_value unless merged_relation.lock_value
 
+      if r.create_with_value
+        merged_relation.create_with_value = (merged_relation.create_with_value || {}).merge(r.create_with_value)
+      end
+
       # Apply scope extension modules
       merged_relation.send :apply_modules, r.extensions
 
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
-- 
cgit v1.2.3


From bea4065d3c8c8f845ddda45b3ec98e3fb308d913 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 18:08:44 +0000
Subject: Refactor BelongsToAssociation to allow
 BelongsToPolymorphicAssociation to inherit from it

---
 .../associations/association_collection.rb         |   4 -
 .../associations/association_proxy.rb              |  31 ++++--
 .../associations/belongs_to_association.rb         | 104 +++++++++++----------
 .../belongs_to_polymorphic_association.rb          |  64 +++----------
 .../associations/through_association.rb            |   2 -
 .../associations/belongs_to_associations_test.rb   |  42 +++++++--
 6 files changed, 127 insertions(+), 120 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 64e10f56e2..0959ea2c5a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -31,10 +31,6 @@ module ActiveRecord
         end
       end
 
-      def scoped
-        with_scope(@scope) { @reflection.klass.scoped }
-      end
-
       def find(*args)
         options = args.extract_options!
 
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index 97eab8a793..7e68241a2c 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -8,7 +8,7 @@ module ActiveRecord
     #
     #   AssociationProxy
     #     BelongsToAssociation
-    #     BelongsToPolymorphicAssociation
+    #       BelongsToPolymorphicAssociation
     #     AssociationCollection + HasAssociation
     #       HasAndBelongsToManyAssociation
     #       HasManyAssociation
@@ -116,6 +116,7 @@ module ActiveRecord
       # Reloads the \target and returns +self+ on success.
       def reload
         reset
+        construct_scope
         load_target
         self unless @target.nil?
       end
@@ -166,6 +167,10 @@ module ActiveRecord
         end
       end
 
+      def scoped
+        with_scope(@scope) { target_klass.scoped }
+      end
+
       protected
         def interpolate_sql(sql, record = nil)
           @owner.send(:interpolate_sql, sql, record)
@@ -192,15 +197,19 @@ module ActiveRecord
 
         # Forwards +with_scope+ to the reflection.
         def with_scope(*args, &block)
-          @reflection.klass.send :with_scope, *args, &block
+          target_klass.send :with_scope, *args, &block
         end
 
         # Construct the scope used for find/create queries on the target
         def construct_scope
-          @scope = {
-            :find   => construct_find_scope,
-            :create => construct_create_scope
-          }
+          if target_klass
+            @scope = {
+              :find   => construct_find_scope,
+              :create => construct_create_scope
+            }
+          else
+            @scope = nil
+          end
         end
 
         # Implemented by subclasses
@@ -214,7 +223,7 @@ module ActiveRecord
         end
 
         def aliased_table
-          @reflection.klass.arel_table
+          target_klass.arel_table
         end
 
         # Set the inverse association, if possible
@@ -224,6 +233,12 @@ module ActiveRecord
           end
         end
 
+        # This class of the target. belongs_to polymorphic overrides this to look at the
+        # polymorphic_type field on the owner.
+        def target_klass
+          @reflection.klass
+        end
+
       private
         # Forwards any missing method call to the \target.
         def method_missing(method, *args)
@@ -254,7 +269,7 @@ module ActiveRecord
         def load_target
           return nil unless defined?(@loaded)
 
-          if !loaded? && (!@owner.new_record? || foreign_key_present)
+          if !loaded? && (!@owner.new_record? || foreign_key_present) && @scope
             @target = find_target
           end
 
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index 98c1c13524..6b29e3ef92 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -11,29 +11,16 @@ module ActiveRecord
       end
 
       def replace(record)
-        counter_cache_name = @reflection.counter_cache_column
-
-        if record.nil?
-          if counter_cache_name && @owner.persisted?
-            @reflection.klass.decrement_counter(counter_cache_name, previous_record_id) if @owner[@reflection.primary_key_name]
-          end
-
-          @target = @owner[@reflection.primary_key_name] = nil
-        else
-          raise_on_type_mismatch(record)
-
-          if counter_cache_name && @owner.persisted? && record.id != @owner[@reflection.primary_key_name]
-            @reflection.klass.increment_counter(counter_cache_name, record.id)
-            @reflection.klass.decrement_counter(counter_cache_name, @owner[@reflection.primary_key_name]) if @owner[@reflection.primary_key_name]
-          end
-
-          @target = (AssociationProxy === record ? record.target : record)
-          @owner[@reflection.primary_key_name] = record_id(record) if record.persisted?
-          @updated = true
-        end
+        record = record.target if AssociationProxy === record
+        raise_on_type_mismatch(record) unless record.nil?
 
+        update_counters(record)
+        replace_keys(record)
         set_inverse_instance(record)
 
+        @target  = record
+        @updated = true if record
+
         loaded
         record
       end
@@ -44,8 +31,8 @@ module ActiveRecord
 
       def stale_target?
         if @target && @target.persisted?
-          target_id   = @target.send(@reflection.association_primary_key).to_s
-          foreign_key = @owner.send(@reflection.primary_key_name).to_s
+          target_id   = @target[@reflection.association_primary_key].to_s
+          foreign_key = @owner[@reflection.primary_key_name].to_s
 
           target_id != foreign_key
         else
@@ -54,27 +41,51 @@ module ActiveRecord
       end
 
       private
+        def update_counters(record)
+          counter_cache_name = @reflection.counter_cache_column
+
+          if counter_cache_name && @owner.persisted? && different_target?(record)
+            if record
+              target_klass.increment_counter(counter_cache_name, record.id)
+            end
+
+            if foreign_key_present
+              target_klass.decrement_counter(counter_cache_name, target_id)
+            end
+          end
+        end
+
+        # Checks whether record is different to the current target, without loading it
+        def different_target?(record)
+          record.nil? && @owner[@reflection.primary_key_name] ||
+          record.id   != @owner[@reflection.primary_key_name]
+        end
+
+        def replace_keys(record)
+          @owner[@reflection.primary_key_name] = record && record[@reflection.association_primary_key]
+        end
+
         def find_target
-          find_method = if @reflection.options[:primary_key]
-                          "find_by_#{@reflection.options[:primary_key]}"
-                        else
-                          "find"
-                        end
-
-          options = @reflection.options.dup.slice(:select, :include, :readonly)
-
-          the_target = with_scope(:find => @scope[:find]) do
-            @reflection.klass.send(find_method,
-              @owner[@reflection.primary_key_name],
-              options
-            ) if @owner[@reflection.primary_key_name]
+          if foreign_key_present
+            scoped.first.tap { |record| set_inverse_instance(record) }
           end
-          set_inverse_instance(the_target)
-          the_target
         end
 
         def construct_find_scope
-          { :conditions => conditions }
+          {
+            :conditions => construct_conditions,
+            :select     => @reflection.options[:select],
+            :include    => @reflection.options[:include],
+            :readonly   => @reflection.options[:readonly]
+          }
+        end
+
+        def construct_conditions
+          conditions = aliased_table[@reflection.association_primary_key].
+                       eq(@owner[@reflection.primary_key_name])
+
+          conditions = conditions.and(Arel.sql(sql_conditions)) if sql_conditions
+          conditions
         end
 
         def foreign_key_present
@@ -88,17 +99,12 @@ module ActiveRecord
           inverse && inverse.macro == :has_one
         end
 
-        def record_id(record)
-          record.send(@reflection.options[:primary_key] || :id)
-        end
-
-        def previous_record_id
-          @previous_record_id ||= if @reflection.options[:primary_key]
-                                    previous_record = @owner.send(@reflection.name)
-                                    previous_record.nil? ? nil : previous_record.id
-                                  else
-                                    @owner[@reflection.primary_key_name]
-                                  end
+        def target_id
+          if @reflection.options[:primary_key]
+            @owner.send(@reflection.name).try(:id)
+          else
+            @owner[@reflection.primary_key_name]
+          end
         end
     end
   end
diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
index 90eff7399c..6293c4ca42 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -1,28 +1,7 @@
 module ActiveRecord
   # = Active Record Belongs To Polymorphic Association
   module Associations
-    class BelongsToPolymorphicAssociation < AssociationProxy #:nodoc:
-      def replace(record)
-        if record.nil?
-          @target = @owner[@reflection.primary_key_name] = @owner[@reflection.options[:foreign_type]] = nil
-        else
-          @target = (AssociationProxy === record ? record.target : record)
-
-          @owner[@reflection.primary_key_name] = record_id(record)
-          @owner[@reflection.options[:foreign_type]] = record.class.base_class.name.to_s
-
-          @updated = true
-        end
-
-        set_inverse_instance(record)
-        loaded
-        record
-      end
-
-      def updated?
-        @updated
-      end
-
+    class BelongsToPolymorphicAssociation < BelongsToAssociation #:nodoc:
       def stale_target?
         if @target && @target.persisted?
           target_id    = @target.send(@reflection.association_primary_key).to_s
@@ -38,43 +17,26 @@ module ActiveRecord
 
       private
 
-        def inverse_reflection_for(record)
-          @reflection.polymorphic_inverse_of(record.class)
+        def replace_keys(record)
+          super
+          @owner[@reflection.options[:foreign_type]] = record && record.class.base_class.name
         end
 
-        def invertible_for?(record)
-          inverse = inverse_reflection_for(record)
-          inverse && inverse.macro == :has_one
+        def different_target?(record)
+          super || record.class != target_klass
         end
 
-        def construct_find_scope
-          { :conditions => conditions }
-        end
-
-        def find_target
-          return nil if association_class.nil?
-
-          target = association_class.send(:with_scope, :find => @scope[:find]) do
-            association_class.find(
-              @owner[@reflection.primary_key_name],
-              :select  => @reflection.options[:select],
-              :include => @reflection.options[:include]
-            )
-          end
-          set_inverse_instance(target)
-          target
-        end
-
-        def foreign_key_present
-          !@owner[@reflection.primary_key_name].nil?
+        def inverse_reflection_for(record)
+          @reflection.polymorphic_inverse_of(record.class)
         end
 
-        def record_id(record)
-          record.send(@reflection.options[:primary_key] || :id)
+        def target_klass
+          type = @owner[@reflection.options[:foreign_type]]
+          type && type.constantize
         end
 
-        def association_class
-          @owner[@reflection.options[:foreign_type]] ? @owner[@reflection.options[:foreign_type]].constantize : nil
+        def raise_on_type_mismatch(record)
+          # A polymorphic association cannot have a type mismatch, by definition
         end
     end
   end
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index 57718285f8..65ca4b1282 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -158,8 +158,6 @@ module ActiveRecord
       alias_method :sql_conditions, :conditions
 
       def update_stale_state
-        construct_scope if stale_target?
-
         if @reflection.through_reflection.macro == :belongs_to
           @through_foreign_key = @owner.send(@reflection.through_reflection.primary_key_name)
         end
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index cdde9a80d3..6dcbbc7e34 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
+
+    reply.topic_with_primary_key = debate2
 
-    assert_equal debate.title, reply.parent_title
-    assert_equal 1, Topic.find(debate.id).send(:read_attribute, "replies_count")
+    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,15 @@ 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
 end
-- 
cgit v1.2.3


From 3c400627eb9cfac380d716ccf1182d61db4a45a6 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 18:36:02 +0000
Subject: Support for :counter_cache on polymorphic belongs_to

---
 .../lib/active_record/associations/belongs_to_association.rb |  2 +-
 .../test/cases/associations/belongs_to_associations_test.rb  | 12 ++++++++++++
 activerecord/test/schema/schema.rb                           |  1 +
 3 files changed, 14 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index 6b29e3ef92..e4758f4369 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -46,7 +46,7 @@ module ActiveRecord
 
           if counter_cache_name && @owner.persisted? && different_target?(record)
             if record
-              target_klass.increment_counter(counter_cache_name, record.id)
+              record.class.increment_counter(counter_cache_name, record.id)
             end
 
             if foreign_key_present
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 6dcbbc7e34..f97f89b6fe 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -567,4 +567,16 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     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/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|
-- 
cgit v1.2.3


From 12675988813e82ac30f7c0e0008c12c4cf5d8cdc Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 20:00:24 +0000
Subject: Rename AssociationReflection#primary_key_name to foreign_key, since
 the options key which it relates to is :foreign_key

---
 .../lib/active_record/association_preload.rb       | 22 +++++++++++-----------
 activerecord/lib/active_record/associations.rb     |  4 ++--
 .../associations/belongs_to_association.rb         | 14 +++++++-------
 .../belongs_to_polymorphic_association.rb          |  2 +-
 .../join_dependency/join_association.rb            |  8 ++++----
 .../has_and_belongs_to_many_association.rb         |  4 ++--
 .../active_record/associations/has_association.rb  |  4 ++--
 .../associations/has_many_association.rb           |  2 +-
 .../associations/has_many_through_association.rb   |  2 +-
 .../associations/has_one_association.rb            |  6 +++---
 .../associations/through_association.rb            | 10 +++++-----
 .../lib/active_record/autosave_association.rb      |  6 +++---
 activerecord/lib/active_record/fixtures.rb         |  2 +-
 activerecord/lib/active_record/reflection.rb       | 14 +++++++-------
 .../lib/active_record/relation/calculations.rb     |  2 +-
 activerecord/test/cases/reflection_test.rb         |  8 +++++---
 16 files changed, 56 insertions(+), 54 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index ecf7b6c210..638897a86b 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -205,7 +205,7 @@ module ActiveRecord
           # FIXME: options[:select] is always nil in the tests.  Do we really
           # need it?
           options[:select] || left[Arel.star],
-          right[reflection.primary_key_name].as(
+          right[reflection.foreign_key].as(
             Arel.sql('the_parent_record_id'))
         ]
 
@@ -220,7 +220,7 @@ module ActiveRecord
 
         all_associated_records = associated_records(ids) do |some_ids|
           method     = in_or_equal(some_ids)
-          conditions = right[reflection.primary_key_name].send(*method)
+          conditions = right[reflection.foreign_key].send(*method)
           conditions = custom_conditions.inject(conditions) do |ast, cond|
             ast.and cond
           end
@@ -241,7 +241,7 @@ module ActiveRecord
 
           unless through_records.empty?
             through_reflection = reflections[options[:through]]
-            through_primary_key = through_reflection.primary_key_name
+            through_primary_key = through_reflection.foreign_key
             source = reflection.source_reflection.name
             through_records.first.class.preload_associations(through_records, source)
             if through_reflection.macro == :belongs_to
@@ -255,7 +255,7 @@ module ActiveRecord
             end
           end
         else
-          set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.primary_key_name)
+          set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.foreign_key)
         end
       end
 
@@ -263,8 +263,8 @@ module ActiveRecord
         return if records.first.send(reflection.name).loaded?
         options = reflection.options
 
-        primary_key_name = reflection.through_reflection_primary_key_name
-        id_to_record_map, ids = construct_id_map(records, primary_key_name || reflection.options[:primary_key])
+        foreign_key = reflection.through_reflection_foreign_key
+        id_to_record_map, ids = construct_id_map(records, foreign_key || reflection.options[:primary_key])
         records.each {|record| record.send(reflection.name).loaded}
 
         if options[:through]
@@ -281,7 +281,7 @@ module ActiveRecord
 
         else
           set_association_collection_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options),
-                                             reflection.primary_key_name)
+                                             reflection.foreign_key)
         end
       end
 
@@ -319,7 +319,7 @@ module ActiveRecord
       def preload_belongs_to_association(records, reflection, preload_options={})
         return if records.first.send("loaded_#{reflection.name}?")
         options = reflection.options
-        primary_key_name = reflection.primary_key_name
+        foreign_key = reflection.foreign_key
 
         klasses_and_ids = {}
 
@@ -330,7 +330,7 @@ module ActiveRecord
           # to their parent_records
           records.each do |record|
             if klass = record.send(polymorph_type)
-              klass_id = record.send(primary_key_name)
+              klass_id = record.send(foreign_key)
               if klass_id
                 id_map = klasses_and_ids[klass.constantize] ||= {}
                 (id_map[klass_id.to_s] ||= []) << record
@@ -339,7 +339,7 @@ module ActiveRecord
           end
         else
           id_map = records.group_by do |record|
-            key = record.send(primary_key_name)
+            key = record.send(foreign_key)
             key && key.to_s
           end
           id_map.delete nil
@@ -369,7 +369,7 @@ module ActiveRecord
 
         conditions = []
 
-        key = reflection.primary_key_name
+        key = reflection.foreign_key
 
         if interface = reflection.options[:as]
           key = "#{interface}_id"
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b82651ebce..9a4f6d4dfd 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1670,7 +1670,7 @@ module ActiveRecord
                 class_eval <<-eoruby, __FILE__, __LINE__ + 1
                   def #{method_name}
                     association = #{reflection.name}
-                    association.update_attribute(#{reflection.primary_key_name.inspect}, nil) if association
+                    association.update_attribute(#{reflection.foreign_key.inspect}, nil) if association
                   end
                 eoruby
               when :restrict
@@ -1782,7 +1782,7 @@ module ActiveRecord
 
           reflection = create_reflection(:has_and_belongs_to_many, association_id, options, self)
 
-          if reflection.association_foreign_key == reflection.primary_key_name
+          if reflection.association_foreign_key == reflection.foreign_key
             raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection)
           end
 
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index e4758f4369..63eb38ab34 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -32,7 +32,7 @@ module ActiveRecord
       def stale_target?
         if @target && @target.persisted?
           target_id   = @target[@reflection.association_primary_key].to_s
-          foreign_key = @owner[@reflection.primary_key_name].to_s
+          foreign_key = @owner[@reflection.foreign_key].to_s
 
           target_id != foreign_key
         else
@@ -57,12 +57,12 @@ module ActiveRecord
 
         # Checks whether record is different to the current target, without loading it
         def different_target?(record)
-          record.nil? && @owner[@reflection.primary_key_name] ||
-          record.id   != @owner[@reflection.primary_key_name]
+          record.nil? && @owner[@reflection.foreign_key] ||
+          record.id   != @owner[@reflection.foreign_key]
         end
 
         def replace_keys(record)
-          @owner[@reflection.primary_key_name] = record && record[@reflection.association_primary_key]
+          @owner[@reflection.foreign_key] = record && record[@reflection.association_primary_key]
         end
 
         def find_target
@@ -82,14 +82,14 @@ module ActiveRecord
 
         def construct_conditions
           conditions = aliased_table[@reflection.association_primary_key].
-                       eq(@owner[@reflection.primary_key_name])
+                       eq(@owner[@reflection.foreign_key])
 
           conditions = conditions.and(Arel.sql(sql_conditions)) if sql_conditions
           conditions
         end
 
         def foreign_key_present
-          !@owner[@reflection.primary_key_name].nil?
+          !@owner[@reflection.foreign_key].nil?
         end
 
         # NOTE - for now, we're only supporting inverse setting from belongs_to back onto
@@ -103,7 +103,7 @@ module ActiveRecord
           if @reflection.options[:primary_key]
             @owner.send(@reflection.name).try(:id)
           else
-            @owner[@reflection.primary_key_name]
+            @owner[@reflection.foreign_key]
           end
         end
     end
diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
index 6293c4ca42..46adc048b8 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -5,7 +5,7 @@ module ActiveRecord
       def stale_target?
         if @target && @target.persisted?
           target_id    = @target.send(@reflection.association_primary_key).to_s
-          foreign_key  = @owner.send(@reflection.primary_key_name).to_s
+          foreign_key  = @owner.send(@reflection.foreign_key).to_s
           target_type  = @target.class.base_class.name
           foreign_type = @owner.send(@reflection.options[:foreign_type]).to_s
 
diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
index 02707dfae1..1e5149d80f 100644
--- a/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/class_methods/join_dependency/join_association.rb
@@ -193,11 +193,11 @@ module ActiveRecord
             first_key = second_key = nil
 
             if through_reflection.macro == :belongs_to
-              jt_primary_key = through_reflection.primary_key_name
+              jt_primary_key = through_reflection.foreign_key
               jt_foreign_key = through_reflection.association_primary_key
             else
               jt_primary_key = through_reflection.active_record_primary_key
-              jt_foreign_key = through_reflection.primary_key_name
+              jt_foreign_key = through_reflection.foreign_key
 
               if through_reflection.options[:as] # has_many :through against a polymorphic join
                 jt_conditions <<
@@ -231,7 +231,7 @@ module ActiveRecord
                 join_table[reflection.source_reflection.options[:foreign_type]].
                   eq(reflection.options[:source_type])
               else
-                second_key = source_reflection.primary_key_name
+                second_key = source_reflection.foreign_key
               end
             end
 
@@ -262,7 +262,7 @@ module ActiveRecord
           end
 
           def join_belongs_to_to(relation)
-            foreign_key = options[:foreign_key] || reflection.primary_key_name
+            foreign_key = options[:foreign_key] || reflection.foreign_key
             primary_key = options[:primary_key] || reflection.klass.primary_key
 
             join_target_table(
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index b1d454545f..5336b6cc28 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -40,7 +40,7 @@ module ActiveRecord
             attributes = columns.map do |column|
               name = column.name
               value = case name.to_s
-                when @reflection.primary_key_name.to_s
+                when @reflection.foreign_key.to_s
                   @owner.id
                 when @reflection.association_foreign_key.to_s
                   record.id
@@ -64,7 +64,7 @@ module ActiveRecord
             records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
           else
             relation = Arel::Table.new(@reflection.options[:join_table])
-            stmt = relation.where(relation[@reflection.primary_key_name].eq(@owner.id).
+            stmt = relation.where(relation[@reflection.foreign_key].eq(@owner.id).
               and(relation[@reflection.association_foreign_key].in(records.map { |x| x.id }.compact))
             ).compile_delete
             @owner.connection.delete stmt.to_sql
diff --git a/activerecord/lib/active_record/associations/has_association.rb b/activerecord/lib/active_record/associations/has_association.rb
index 0ecdb696ea..190fed77c2 100644
--- a/activerecord/lib/active_record/associations/has_association.rb
+++ b/activerecord/lib/active_record/associations/has_association.rb
@@ -14,9 +14,9 @@ module ActiveRecord
         def construct_owner_attributes(reflection = @reflection)
           attributes = {}
           if reflection.macro == :belongs_to
-            attributes[reflection.association_primary_key] = @owner.send(reflection.primary_key_name)
+            attributes[reflection.association_primary_key] = @owner.send(reflection.foreign_key)
           else
-            attributes[reflection.primary_key_name] = @owner.send(reflection.active_record_primary_key)
+            attributes[reflection.foreign_key] = @owner.send(reflection.active_record_primary_key)
 
             if reflection.options[:as]
               attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index a0e2e8b479..0d044b28e4 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -58,7 +58,7 @@ module ActiveRecord
             when :delete_all
               @reflection.klass.delete(records.map { |r| r.id })
             else
-              updates    = { @reflection.primary_key_name => nil }
+              updates    = { @reflection.foreign_key => nil }
               conditions = { @reflection.association_primary_key => records.map { |r| r.id } }
 
               with_scope(@scope) do
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index e2b008034e..2348ee099c 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -31,7 +31,7 @@ module ActiveRecord
 
       protected
         def target_reflection_has_associated_record?
-          if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.primary_key_name].blank?
+          if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank?
             false
           else
             true
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 04cb16b909..12ccb3af8a 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -38,11 +38,11 @@ module ActiveRecord
               @target.destroy if @target.persisted?
               @owner.clear_association_cache
             when :nullify
-              @target[@reflection.primary_key_name] = nil
+              @target[@reflection.foreign_key] = nil
               @target.save if @owner.persisted? && @target.persisted?
             end
           else
-            @target[@reflection.primary_key_name] = nil
+            @target[@reflection.foreign_key] = nil
             @target.save if @owner.persisted? && @target.persisted?
           end
         end
@@ -96,7 +96,7 @@ module ActiveRecord
           if replace_existing
             replace(record, true)
           else
-            record[@reflection.primary_key_name] = @owner.id if @owner.persisted?
+            record[@reflection.foreign_key] = @owner.id if @owner.persisted?
             self.target = record
             set_inverse_instance(record)
           end
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index 65ca4b1282..c78f5d969f 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -13,7 +13,7 @@ module ActiveRecord
       def stale_target?
         if @target && @reflection.through_reflection.macro == :belongs_to && defined?(@through_foreign_key)
           previous_key = @through_foreign_key.to_s
-          current_key  = @owner.send(@reflection.through_reflection.primary_key_name).to_s
+          current_key  = @owner.send(@reflection.through_reflection.foreign_key).to_s
 
           previous_key != current_key
         else
@@ -69,14 +69,14 @@ module ActiveRecord
         if @reflection.source_reflection.macro == :belongs_to
           reflection_primary_key = @reflection.source_reflection.options[:primary_key] ||
                                    @reflection.klass.primary_key
-          source_primary_key     = @reflection.source_reflection.primary_key_name
+          source_primary_key     = @reflection.source_reflection.foreign_key
           if @reflection.options[:source_type]
             column = @reflection.source_reflection.options[:foreign_type]
             conditions <<
               right[column].eq(@reflection.options[:source_type])
           end
         else
-          reflection_primary_key = @reflection.source_reflection.primary_key_name
+          reflection_primary_key = @reflection.source_reflection.foreign_key
           source_primary_key     = @reflection.source_reflection.options[:primary_key] ||
                                    @reflection.through_reflection.klass.primary_key
           if @reflection.source_reflection.options[:as]
@@ -100,7 +100,7 @@ module ActiveRecord
         raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro)
 
         join_attributes = {
-          @reflection.source_reflection.primary_key_name =>
+          @reflection.source_reflection.foreign_key =>
             associate.send(@reflection.source_reflection.association_primary_key)
         }
 
@@ -159,7 +159,7 @@ module ActiveRecord
 
       def update_stale_state
         if @reflection.through_reflection.macro == :belongs_to
-          @through_foreign_key = @owner.send(@reflection.through_reflection.primary_key_name)
+          @through_foreign_key = @owner.send(@reflection.through_reflection.foreign_key)
         end
       end
     end
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 4a18719551..c6c1dd8b87 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -343,8 +343,8 @@ module ActiveRecord
           association.destroy
         else
           key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
-          if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
-            association[reflection.primary_key_name] = key
+          if autosave != false && (new_record? || association.new_record? || association[reflection.foreign_key] != key || autosave)
+            association[reflection.foreign_key] = key
             saved = association.save(:validate => !autosave)
             raise ActiveRecord::Rollback if !saved && autosave
             saved
@@ -367,7 +367,7 @@ module ActiveRecord
 
           if association.updated?
             association_id = association.send(reflection.options[:primary_key] || :id)
-            self[reflection.primary_key_name] = association_id
+            self[reflection.foreign_key] = association_id
           end
 
           saved if autosave
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 6fb723f2f5..3ddd6687b9 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -634,7 +634,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash)
 
               targets.each do |target|
                 join_fixtures["#{label}_#{target}"] = Fixture.new(
-                  { association.primary_key_name => row[primary_key_name],
+                  { association.foreign_key             => row[primary_key_name],
                     association.association_foreign_key => Fixtures.identify(target) },
                   nil, @connection)
               end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 8c68a1bc00..81a95d4971 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -196,8 +196,8 @@ module ActiveRecord
         @quoted_table_name ||= klass.quoted_table_name
       end
 
-      def primary_key_name
-        @primary_key_name ||= options[:foreign_key] || derive_primary_key_name
+      def foreign_key
+        @foreign_key ||= options[:foreign_key] || derive_foreign_key
       end
 
       def primary_key_column
@@ -251,7 +251,7 @@ module ActiveRecord
         false
       end
 
-      def through_reflection_primary_key_name
+      def through_reflection_foreign_key
       end
 
       def source_reflection
@@ -310,7 +310,7 @@ module ActiveRecord
           class_name
         end
 
-        def derive_primary_key_name
+        def derive_foreign_key
           if belongs_to?
             "#{name}_id"
           elsif options[:as]
@@ -392,11 +392,11 @@ module ActiveRecord
       end
 
       def through_reflection_primary_key
-        through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.primary_key_name
+        through_reflection.belongs_to? ? through_reflection.klass.primary_key : through_reflection.foreign_key
       end
 
-      def through_reflection_primary_key_name
-        through_reflection.primary_key_name if through_reflection.belongs_to?
+      def through_reflection_foreign_key
+        through_reflection.foreign_key if through_reflection.belongs_to?
       end
 
       private
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index fd45bb24dd..139752b190 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -211,7 +211,7 @@ module ActiveRecord
       group_attr      = @group_values
       association     = @klass.reflect_on_association(group_attr.first.to_sym)
       associated      = group_attr.size == 1 && association && association.macro == :belongs_to # only count belongs_to associations
-      group_fields  = Array(associated ? association.primary_key_name : group_attr)
+      group_fields  = Array(associated ? association.foreign_key : group_attr)
       group_aliases = group_fields.map { |field| column_alias_for(field) }
       group_columns = group_aliases.zip(group_fields).map { |aliaz,field|
         [aliaz, column_for(field)]
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
-- 
cgit v1.2.3