1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Ensure reload re-applies preload values for strict loading

If you have an application that has strict_loading set and then call
`reload` that would cause the preload values to get lost and
applications would start throwing a stict loading violation error.

In order to fix this we capture the association cache and re-apply that
to the reloaded records to avoid the strict loading error. Of course if
you never preloaded your records, this will still raise a strict loading
violation.

This change also removes the `reload` definition from associations.rb
because we can get the same behavior when we reassign the association
cache.

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
This commit is contained in:
eileencodes 2021-01-12 11:03:44 -05:00
parent ba1ec19013
commit 3328cd1d69
No known key found for this signature in database
GPG key ID: BA5C575120BBE8DF
3 changed files with 57 additions and 7 deletions

View file

@ -326,11 +326,6 @@ module ActiveRecord
super super
end end
def reload(*) # :nodoc:
clear_association_cache
super
end
private private
# Clears out the association cache. # Clears out the association cache.
def clear_association_cache def clear_association_cache

View file

@ -835,6 +835,7 @@ module ActiveRecord
self.class.unscoped { _find_record(options) } self.class.unscoped { _find_record(options) }
end end
@association_cache = fresh_object.instance_variable_get(:@association_cache)
@attributes = fresh_object.instance_variable_get(:@attributes) @attributes = fresh_object.instance_variable_get(:@attributes)
@new_record = false @new_record = false
@previously_new_record = false @previously_new_record = false
@ -893,11 +894,17 @@ module ActiveRecord
end end
private private
def strict_loaded_associations
@association_cache.find_all do |_, assoc|
assoc.owner.strict_loading? && !assoc.owner.strict_loading_n_plus_one_only?
end.map(&:first)
end
def _find_record(options) def _find_record(options)
if options && options[:lock] if options && options[:lock]
self.class.lock(options[:lock]).find(id) self.class.preload(strict_loaded_associations).lock(options[:lock]).find(id)
else else
self.class.find(id) self.class.preload(strict_loaded_associations).find(id)
end end
end end

View file

@ -204,6 +204,54 @@ class StrictLoadingTest < ActiveRecord::TestCase
end end
end end
def test_strict_loading_has_one_reload
with_strict_loading_by_default(Developer) do
ship = Ship.create!(developer: Developer.first, name: "The Great Ship")
developer = Developer.preload(:ship).first
assert_predicate developer, :strict_loading?
assert_equal ship, developer.ship
developer.reload
assert_nothing_raised do
assert_equal ship, developer.ship
end
end
end
def test_strict_loading_with_has_many
with_strict_loading_by_default(Developer) do
devs = Developer.preload(:audit_logs).all
assert_nothing_raised do
devs.map(&:audit_logs).to_a
end
devs.reload
assert_nothing_raised do
devs.map(&:audit_logs).to_a
end
end
end
def test_strict_loading_with_has_many_singular_association_and_reload
with_strict_loading_by_default(Developer) do
dev = Developer.preload(:audit_logs).first
assert_nothing_raised do
dev.audit_logs.to_a
end
dev.reload
assert_nothing_raised do
dev.audit_logs.to_a
end
end
end
def test_preload_audit_logs_are_strict_loading_because_parent_is_strict_loading def test_preload_audit_logs_are_strict_loading_because_parent_is_strict_loading
developer = Developer.first developer = Developer.first