If a has_many goes :through a belongs_to, and the foreign key of the belongs_to changes, then the has_many should be considered stale.

14 files changed, 124 insertions, 35 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index c6c41a7a12..0aea05b348 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1501,7 +1501,11 @@ module ActiveRecord
               association_instance_set(reflection.name, association)
             end
 
-            reflection.klass.uncached { association.reload } if force_reload
+            if force_reload
+              reflection.klass.uncached { association.reload }
+            elsif association.stale_target?
+              association.reload
+            end
 
             association
           end
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index c9abfe36e8..bbfe18f9fb 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -44,7 +44,10 @@ module ActiveRecord
 
       def stale_target?
         if @target && @target.persisted?
-          @target.send(@reflection.association_primary_key).to_i != @owner.send(@reflection.primary_key_name).to_i
+          target_id   = @target.send(@reflection.association_primary_key).to_s
+          foreign_key = @owner.send(@reflection.primary_key_name).to_s
+
+          target_id != foreign_key
         else
           false
         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 844ae94c3d..c580de7fbe 100644
--- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -25,8 +25,12 @@ module ActiveRecord
 
       def stale_target?
         if @target && @target.persisted?
-          @target.send(@reflection.association_primary_key).to_i != @owner.send(@reflection.primary_key_name).to_i ||
-          @target.class.base_class.name.to_s != @owner.send(@reflection.options[:foreign_type]).to_s
+          target_id    = @target.send(@reflection.association_primary_key).to_s
+          foreign_key  = @owner.send(@reflection.primary_key_name).to_s
+          target_type  = @target.class.base_class.name
+          foreign_type = @owner.send(@reflection.options[:foreign_type]).to_s
+
+          target_id != foreign_key || target_type != foreign_type
         else
           false
         end
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 f0bc6aedf2..5f4667b4d8 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -59,6 +59,7 @@ module ActiveRecord
 
         def find_target
           return [] unless target_reflection_has_associated_record?
+          update_stale_state
           scoped.all
         end
 
diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb
index e8cf73976b..eb17935d6a 100644
--- a/activerecord/lib/active_record/associations/has_one_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_through_association.rb
@@ -33,6 +33,7 @@ module ActiveRecord
 
     private
       def find_target
+        update_stale_state
         scoped.first
       end
     end
diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb
index c11fce5db0..e57de84f66 100644
--- a/activerecord/lib/active_record/associations/through_association_scope.rb
+++ b/activerecord/lib/active_record/associations/through_association_scope.rb
@@ -10,6 +10,17 @@ module ActiveRecord
         end
       end
 
+      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
+
+          previous_key != current_key
+        else
+          false
+        end
+      end
+
       protected
 
       def construct_find_scope
@@ -165,6 +176,14 @@ module ActiveRecord
       end
 
       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
+      end
     end
   end
 end
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index 1820f95261..cdde9a80d3 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -17,7 +17,7 @@ require 'models/essay'
 class BelongsToAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :developers, :projects, :topics,
            :developers_projects, :computers, :authors, :author_addresses,
-           :posts, :tags, :taggings, :comments
+           :posts, :tags, :taggings, :comments, :sponsors, :members
 
   def test_belongs_to
     Client.find(3).firm.name
@@ -488,39 +488,53 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_reassigning_the_parent_id_updates_the_object
-    original_parent = Firm.create! :name => "original"
-    updated_parent = Firm.create! :name => "updated"
+    client = companies(:second_client)
 
-    client = Client.new("client_of" => original_parent.id)
-    assert_equal original_parent, client.firm
-    assert_equal original_parent, client.firm_with_condition
-    assert_equal original_parent, client.firm_with_other_name
+    client.firm
+    client.firm_with_condition
+    firm_proxy                = client.send(:association_instance_get, :firm)
+    firm_with_condition_proxy = client.send(:association_instance_get, :firm_with_condition)
 
-    client.client_of = updated_parent.id
-    assert_equal updated_parent, client.firm
-    assert_equal updated_parent, client.firm_with_condition
-    assert_equal updated_parent, client.firm_with_other_name
+    assert !firm_proxy.stale_target?
+    assert !firm_with_condition_proxy.stale_target?
+    assert_equal companies(:first_firm), client.firm
+    assert_equal companies(:first_firm), client.firm_with_condition
+
+    client.client_of = companies(:another_firm).id
+
+    assert firm_proxy.stale_target?
+    assert firm_with_condition_proxy.stale_target?
+    assert_equal companies(:another_firm), client.firm
+    assert_equal companies(:another_firm), client.firm_with_condition
   end
 
   def test_polymorphic_reassignment_of_associated_id_updates_the_object
-    member1 = Member.create!
-    member2 = Member.create!
+    sponsor = sponsors(:moustache_club_sponsor_for_groucho)
+
+    sponsor.sponsorable
+    proxy = sponsor.send(:association_instance_get, :sponsorable)
+
+    assert !proxy.stale_target?
+    assert_equal members(:groucho), sponsor.sponsorable
 
-    sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id)
-    assert_equal member1, sponsor.sponsorable
+    sponsor.sponsorable_id = members(:some_other_guy).id
 
-    sponsor.sponsorable_id = member2.id
-    assert_equal member2, sponsor.sponsorable
+    assert proxy.stale_target?
+    assert_equal members(:some_other_guy), sponsor.sponsorable
   end
 
   def test_polymorphic_reassignment_of_associated_type_updates_the_object
-    member1 = Member.create!
+    sponsor = sponsors(:moustache_club_sponsor_for_groucho)
 
-    sponsor = Sponsor.new("sponsorable_type" => "Member", "sponsorable_id" => member1.id)
-    assert_equal member1, sponsor.sponsorable
+    sponsor.sponsorable
+    proxy = sponsor.send(:association_instance_get, :sponsorable)
 
-    sponsor.sponsorable_type = "Firm"
-    assert_not_equal member1, sponsor.sponsorable
-  end
+    assert !proxy.stale_target?
+    assert_equal members(:groucho), sponsor.sponsorable
+
+    sponsor.sponsorable_type = 'Firm'
 
+    assert proxy.stale_target?
+    assert_equal companies(:first_firm), sponsor.sponsorable
+  end
 end
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index a417345780..e98d178ff5 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -19,6 +19,7 @@ require 'models/book'
 require 'models/subscription'
 require 'models/categorization'
 require 'models/category'
+require 'models/essay'
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   fixtures :posts, :readers, :people, :comments, :authors, :categories,
@@ -534,4 +535,19 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_equal 2, authors(:mary).categories.count
     assert_equal 1, authors(:mary).categories.general.count
   end
+
+  def test_has_many_through_belongs_to_should_update_when_the_through_foreign_key_changes
+    post = posts(:eager_other)
+
+    post.author_categorizations
+    proxy = post.send(:association_instance_get, :author_categorizations)
+
+    assert !proxy.stale_target?
+    assert_equal authors(:mary).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id)
+
+    post.author_id = authors(:david).id
+
+    assert proxy.stale_target?
+    assert_equal authors(:david).categorizations.sort_by(&:id), post.author_categorizations.sort_by(&:id)
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb
index f6365e0216..7d55d909a7 100644
--- a/activerecord/test/cases/associations/has_one_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb
@@ -251,4 +251,19 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
       members(:groucho).club_through_many
     end
   end
+
+  def test_has_one_through_belongs_to_should_update_when_the_through_foreign_key_changes
+    minivan = minivans(:cool_first)
+
+    minivan.dashboard
+    proxy = minivan.send(:association_instance_get, :dashboard)
+
+    assert !proxy.stale_target?
+    assert_equal dashboards(:cool_first), minivan.dashboard
+
+    minivan.speedometer_id = speedometers(:second).id
+
+    assert proxy.stale_target?
+    assert_equal dashboards(:second), minivan.dashboard
+  end
 end
diff --git a/activerecord/test/fixtures/dashboards.yml b/activerecord/test/fixtures/dashboards.yml
index e75bf46e6c..a4c7e0d309 100644
--- a/activerecord/test/fixtures/dashboards.yml
+++ b/activerecord/test/fixtures/dashboards.yml
@@ -1,3 +1,6 @@
 cool_first:
   dashboard_id: d1
-  name: my_dashboard
\ No newline at end of file
+  name: my_dashboard
+second:
+  dashboard_id: d2
+  name: second
diff --git a/activerecord/test/fixtures/members.yml b/activerecord/test/fixtures/members.yml
index 6db945e61d..824840b7e5 100644
--- a/activerecord/test/fixtures/members.yml
+++ b/activerecord/test/fixtures/members.yml
@@ -1,6 +1,8 @@
 groucho:
+  id: 1
   name: Groucho Marx
   member_type_id: 1
 some_other_guy:
+  id: 2
   name: Englebert Humperdink
   member_type_id: 2
diff --git a/activerecord/test/fixtures/memberships.yml b/activerecord/test/fixtures/memberships.yml
index b9722dbc8a..eed8b22af8 100644
--- a/activerecord/test/fixtures/memberships.yml
+++ b/activerecord/test/fixtures/memberships.yml
@@ -1,20 +1,20 @@
 membership_of_boring_club:
   joined_on: <%= 3.weeks.ago.to_s(:db) %>
   club: boring_club
-  member: groucho
+  member_id: 1
   favourite: false
   type: CurrentMembership
 
 membership_of_favourite_club:
   joined_on: <%= 3.weeks.ago.to_s(:db) %>
   club: moustache_club
-  member: groucho
+  member_id: 1
   favourite: true
   type: Membership
 
 other_guys_membership:
   joined_on: <%= 4.weeks.ago.to_s(:db) %>
   club: boring_club
-  member: some_other_guy
+  member_id: 2
   favourite: false
   type: CurrentMembership
diff --git a/activerecord/test/fixtures/speedometers.yml b/activerecord/test/fixtures/speedometers.yml
index 6a471aba0a..e12398f0c4 100644
--- a/activerecord/test/fixtures/speedometers.yml
+++ b/activerecord/test/fixtures/speedometers.yml
@@ -1,4 +1,8 @@
 cool_first:
   speedometer_id: s1
   name: my_speedometer
-  dashboard_id: d1
\ No newline at end of file
+  dashboard_id: d1
+second:
+  speedometer_id: s2
+  name: second
+  dashboard_id: d2
diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml
index 42df8957d1..bfc6b238b1 100644
--- a/activerecord/test/fixtures/sponsors.yml
+++ b/activerecord/test/fixtures/sponsors.yml
@@ -1,9 +1,12 @@
 moustache_club_sponsor_for_groucho:
   sponsor_club: moustache_club
-  sponsorable: groucho (Member)
+  sponsorable_id: 1
+  sponsorable_type: Member
 boring_club_sponsor_for_groucho:
   sponsor_club: boring_club
-  sponsorable: some_other_guy (Member)
+  sponsorable_id: 2
+  sponsorable_type: Member
 crazy_club_sponsor_for_groucho:
   sponsor_club: crazy_club
-  sponsorable: some_other_guy (Member)
\ No newline at end of file
+  sponsorable_id: 2
+  sponsorable_type: Member