mirror of
https://github.com/github/rails.git
synced 2026-01-09 14:48:01 -05:00
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.
This commit is contained in:
committed by
Aaron Patterson
parent
2d9626fc74
commit
1c07b84df9
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -59,6 +59,7 @@ module ActiveRecord
|
||||
|
||||
def find_target
|
||||
return [] unless target_reflection_has_associated_record?
|
||||
update_stale_state
|
||||
scoped.all
|
||||
end
|
||||
|
||||
|
||||
@@ -33,6 +33,7 @@ module ActiveRecord
|
||||
|
||||
private
|
||||
def find_target
|
||||
update_stale_state
|
||||
scoped.first
|
||||
end
|
||||
end
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 = 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_id = member2.id
|
||||
assert_equal member2, sponsor.sponsorable
|
||||
assert !proxy.stale_target?
|
||||
assert_equal members(:groucho), sponsor.sponsorable
|
||||
|
||||
sponsor.sponsorable_id = members(:some_other_guy).id
|
||||
|
||||
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
|
||||
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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
5
activerecord/test/fixtures/dashboards.yml
vendored
5
activerecord/test/fixtures/dashboards.yml
vendored
@@ -1,3 +1,6 @@
|
||||
cool_first:
|
||||
dashboard_id: d1
|
||||
name: my_dashboard
|
||||
name: my_dashboard
|
||||
second:
|
||||
dashboard_id: d2
|
||||
name: second
|
||||
|
||||
2
activerecord/test/fixtures/members.yml
vendored
2
activerecord/test/fixtures/members.yml
vendored
@@ -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
|
||||
|
||||
6
activerecord/test/fixtures/memberships.yml
vendored
6
activerecord/test/fixtures/memberships.yml
vendored
@@ -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
|
||||
|
||||
6
activerecord/test/fixtures/speedometers.yml
vendored
6
activerecord/test/fixtures/speedometers.yml
vendored
@@ -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
|
||||
|
||||
9
activerecord/test/fixtures/sponsors.yml
vendored
9
activerecord/test/fixtures/sponsors.yml
vendored
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user