Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that entire collection has been loaded before short circuiting #37747

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/associations/preloader.rb
Expand Up @@ -184,7 +184,7 @@ def records_by_owner
# and attach it to a relation. The class returned implements a `run` method
# that accepts a preloader.
def preloader_for(reflection, owners)
if owners.first.association(reflection.name).loaded?
if owners.all? { |o| o.association(reflection.name).loaded? }
return AlreadyLoaded
end
reflection.check_preloadable!
Expand Down
15 changes: 15 additions & 0 deletions activerecord/test/cases/associations/eager_test.rb
Expand Up @@ -626,6 +626,21 @@ def test_eager_with_has_many_through_an_sti_join_model
assert_equal [comments(:does_it_hurt)], assert_no_queries { author.special_post_comments }
end

def test_preloading_with_has_one_through_an_sti_with_after_initialize
author_a = Author.create!(name: "A")
author_b = Author.create!(name: "B")
post_a = StiPost.create!(author: author_a, title: "TITLE", body: "BODY")
post_b = SpecialPost.create!(author: author_b, title: "TITLE", body: "BODY")
comment_a = SpecialComment.create!(post: post_a, body: "TEST")
comment_b = SpecialComment.create!(post: post_b, body: "TEST")
reset_callbacks(StiPost, :initialize) do
StiPost.after_initialize { author }
comments = SpecialComment.where(id: [comment_a.id, comment_b.id]).includes(:author).to_a
comments_with_author = comments.map { |c| [c.id, c.author.try(:id)] }
assert_equal comments_with_author.size, comments_with_author.map(&:second).compact.size
end
end

def test_preloading_has_many_through_with_implicit_source
authors = Author.includes(:very_special_comments).to_a
assert_no_queries do
Expand Down
20 changes: 3 additions & 17 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Expand Up @@ -2831,7 +2831,7 @@ def self.name
end

test "prevent double insertion of new object when the parent association loaded in the after save callback" do
reset_callbacks(:save, Bulb) do
reset_callbacks(Bulb, :save) do
Bulb.after_save { |record| record.car.bulbs.load }

car = Car.create!
Expand All @@ -2842,7 +2842,7 @@ def self.name
end

test "prevent double firing the before save callback of new object when the parent association saved in the callback" do
reset_callbacks(:save, Bulb) do
reset_callbacks(Bulb, :save) do
count = 0
Bulb.before_save { |record| record.car.save && count += 1 }

Expand Down Expand Up @@ -2961,7 +2961,7 @@ def test_ids_reader_memoization
end

def test_loading_association_in_validate_callback_doesnt_affect_persistence
reset_callbacks(:validation, Bulb) do
reset_callbacks(Bulb, :validation) do
Bulb.after_validation { |record| record.car.bulbs.load }

car = Car.create!(name: "Car")
Expand Down Expand Up @@ -2996,18 +2996,4 @@ def test_has_many_preloading_with_duplicate_records
def force_signal37_to_load_all_clients_of_firm
companies(:first_firm).clients_of_firm.load_target
end

def reset_callbacks(kind, klass)
old_callbacks = {}
old_callbacks[klass] = klass.send("_#{kind}_callbacks").dup
klass.subclasses.each do |subclass|
old_callbacks[subclass] = subclass.send("_#{kind}_callbacks").dup
end
yield
ensure
klass.send("_#{kind}_callbacks=", old_callbacks[klass])
klass.subclasses.each do |subclass|
subclass.send("_#{kind}_callbacks=", old_callbacks[subclass])
end
end
end
Expand Up @@ -545,13 +545,6 @@ def test_inverse_instance_should_be_set_before_initialize_callbacks_are_run
assert_predicate Man.joins(:interests).includes(:interests).first.interests, :any?
end
end

def reset_callbacks(target, type)
old_callbacks = target.send(:get_callbacks, type).deep_dup
yield
ensure
target.send(:set_callbacks, type, old_callbacks) if old_callbacks
end
end

class InverseBelongsToTests < ActiveRecord::TestCase
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/test_case.rb
Expand Up @@ -87,6 +87,20 @@ def with_has_many_inversing
ensure
ActiveRecord::Base.has_many_inversing = old
end

def reset_callbacks(klass, kind)
old_callbacks = {}
old_callbacks[klass] = klass.send("_#{kind}_callbacks").dup
klass.subclasses.each do |subclass|
old_callbacks[subclass] = subclass.send("_#{kind}_callbacks").dup
end
yield
ensure
klass.send("_#{kind}_callbacks=", old_callbacks[klass])
klass.subclasses.each do |subclass|
subclass.send("_#{kind}_callbacks=", old_callbacks[subclass])
end
end
end

class PostgreSQLTestCase < TestCase
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/comment.rb
Expand Up @@ -59,6 +59,7 @@ def to_s
end

class SpecialComment < Comment
has_one :author, through: :post
default_scope { where(deleted_at: nil) }

def self.what_are_you
Expand Down