Skip to content

Commit

Permalink
Auto include through associations when 'middle' association was loade…
Browse files Browse the repository at this point in the history
…d but not modified (#138) (#145)

When an association goes through another association, e.g. `has_many :tags, through: :post_tags`,
loading the `post_tags` association was enough, to not auto include the `tags` association when
it is first accessed.
This is correct for when the `post_tags` association was modified and auto include could lead to
weird results. When nothing was modified it is totally fine to auto include the through association.

Code author: Felix Tscheulin https://github.com/Flixt
  • Loading branch information
viktorianer authored May 30, 2024
1 parent 87e0ab7 commit 1595eb6
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
15 changes: 14 additions & 1 deletion lib/goldiloader/active_record_patches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,20 @@ module ThroughAssociationPatch
def auto_include?
# Only auto include through associations if the target association is auto-loadable
through_association = owner.association(through_reflection.name)
through_association.auto_include? && super
auto_include_self = super

# If the current association cannot be auto-included there is nothing we can do
return false unless auto_include_self

# If the through association can just be auto-included we're good
return true if through_association.auto_include?

# If the through association was already loaded and does not contain new, changed, or destroyed records
# we are also able to auto-include the association. It means it has only already been read or changes are
# already persisted.
through_association.loaded? && Array.wrap(through_association.target).none? do |record|
record.new_record? || record.changed? || record.destroyed?
end
end
end
::ActiveRecord::Associations::HasManyThroughAssociation.prepend(::Goldiloader::ThroughAssociationPatch)
Expand Down
42 changes: 42 additions & 0 deletions spec/goldiloader/goldiloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,48 @@
end
end

context "when the 'middle' part of a has_many through association was already loaded to memory and not modified" do
let!(:posts) { Post.order(:title).to_a }
let(:post) { posts.first }
let(:other_post) { posts.last }

before do
post.post_tags.to_a
end

it "auto eager loads the association when accessing a peer" do
other_post.tags.to_a
expect(post.association(:tags)).to be_loaded
end

it "doesn't reload the already loaded models when accessing a peer" do
object_ids = post.post_tags.map(&:object_id)
other_post.tags.to_a
expect(post.post_tags.map(&:object_id)).to eq(object_ids)
end
end

context "when the 'middle' part of a has_many through association was already loaded to memory and modified" do
let!(:posts) { Post.order(:title).to_a }
let(:post) { posts.first }
let(:other_post) { posts.last }

before do
post.post_tags.to_a.first.tag = child_tag3
end

it "doesn't auto eager loads the association when accessing a peer" do
other_post.tags.to_a
expect(post.association(:tags)).not_to be_loaded
end

it "doesn't reload the already loaded models when accessing a peer" do
object_ids = post.post_tags.map(&:object_id)
other_post.tags.to_a
expect(post.post_tags.map(&:object_id)).to eq(object_ids)
end
end

context "when a has_many through association has in-memory changes" do
let!(:posts) { Post.order(:title).to_a }
let(:post) { posts.first }
Expand Down

0 comments on commit 1595eb6

Please sign in to comment.