Skip to content
Browse files

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.
  • Loading branch information...
1 parent 2d9626f commit 1c07b84df95e932d50376c1d0a13585b2e2ef868 @jonleighton jonleighton committed with tenderlove Dec 23, 2010
View
6 activerecord/lib/active_record/associations.rb
@@ -1501,7 +1501,11 @@ def collection_reader_method(reflection, association_proxy_class)
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
View
5 activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -44,7 +44,10 @@ def updated?
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
View
8 activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb
@@ -25,8 +25,12 @@ def updated?
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
View
1 activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -59,6 +59,7 @@ def delete_records(records)
def find_target
return [] unless target_reflection_has_associated_record?
+ update_stale_state
scoped.all
end
View
1 activerecord/lib/active_record/associations/has_one_through_association.rb
@@ -33,6 +33,7 @@ def create_through_record(new_value) #nodoc:
private
def find_target
+ update_stale_state
scoped.first
end
end
View
19 activerecord/lib/active_record/associations/through_association_scope.rb
@@ -10,6 +10,17 @@ def scoped
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 @@ def build_sti_condition
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
View
60 activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -17,7 +17,7 @@
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 @@ def test_attributes_are_being_set_when_initialized_from_belongs_to_association_w
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
View
16 activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -19,6 +19,7 @@
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 @@ def test_count_has_many_through_with_named_scope
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
View
15 activerecord/test/cases/associations/has_one_through_associations_test.rb
@@ -251,4 +251,19 @@ def test_has_one_through_many_raises_exception
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
View
5 activerecord/test/fixtures/dashboards.yml
@@ -1,3 +1,6 @@
cool_first:
dashboard_id: d1
- name: my_dashboard
+ name: my_dashboard
+second:
+ dashboard_id: d2
+ name: second
View
2 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
View
6 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
View
6 activerecord/test/fixtures/speedometers.yml
@@ -1,4 +1,8 @@
cool_first:
speedometer_id: s1
name: my_speedometer
- dashboard_id: d1
+ dashboard_id: d1
+second:
+ speedometer_id: s2
+ name: second
+ dashboard_id: d2
View
9 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)
+ sponsorable_id: 2
+ sponsorable_type: Member

0 comments on commit 1c07b84

Please sign in to comment.
Something went wrong with that request. Please try again.