Skip to content

Commit

Permalink
Merge pull request #70 from clio/fix_poly_n_plus_one_false_alarm
Browse files Browse the repository at this point in the history
Fix polymorphic assocation with nil type N+1 false alarm issue
  • Loading branch information
wendy-clio authored Jun 11, 2024
2 parents 0876ead + 94c257e commit 92d50ad
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@ def load_target
# always an N+1 query.
record.jit_n_plus_one_tracking ||= owner.jit_n_plus_one_tracking if record

if !jit_loaded && owner.jit_n_plus_one_tracking
if !jit_loaded && owner.jit_n_plus_one_tracking && !is_polymorphic_association_without_type
ActiveSupport::Notifications.publish("n_plus_one_query",
source: owner, association: reflection.name)
end
end
end
end

private def is_polymorphic_association_without_type
self.is_a?(ActiveRecord::Associations::BelongsToPolymorphicAssociation) && self.klass.nil?
end
end
end

Expand Down
19 changes: 6 additions & 13 deletions spec/lib/jit_preloader/preloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,13 @@
contact1.update!(contact_owner_type: nil, contact_owner_id: nil)
end

it "successfully load the rest of association values" do
it "successfully load the rest of association values and does not publish a n+1 notification" do
contacts = Contact.jit_preload.to_a
expect(contacts.first.contact_owner).to eq(nil)
ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do
expect(contacts.first.contact_owner).to eq(nil)
end

expect(source_map).to eql({})

expect do
contacts.first.contact_owner
Expand All @@ -184,17 +188,6 @@
expect(contacts.second.contact_owner).to eq(ContactOwner.first)
expect(contacts.third.contact_owner).to eq(Address.first)
end

it "publish N+1 notification due to polymorphic nil type" do
contacts = Contact.jit_preload.to_a

ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do
contacts.first.contact_owner
end

expect_source_map = { contacts.first => [:contact_owner] }
expect(source_map).to eql(expect_source_map)
end
end
end
end
Expand Down

0 comments on commit 92d50ad

Please sign in to comment.