Skip to content

Commit

Permalink
add check before N+1 notification publish to prevent poly association…
Browse files Browse the repository at this point in the history
… with nil type to be include
  • Loading branch information
wendy-clio committed Jun 10, 2024
1 parent 0876ead commit 275c00a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 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
17 changes: 5 additions & 12 deletions spec/lib/jit_preloader/preloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,11 @@

it "successfully load the rest of association values" 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 275c00a

Please sign in to comment.