From 275c00aa247bfd50c56cc070c2ec6a8bbdb37738 Mon Sep 17 00:00:00 2001 From: Wendy Chen Date: Wed, 5 Jun 2024 10:34:18 -0400 Subject: [PATCH 1/2] add check before N+1 notification publish to prevent poly association with nil type to be include --- .../associations/singular_association.rb | 6 +++++- spec/lib/jit_preloader/preloader_spec.rb | 17 +++++------------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/jit_preloader/active_record/associations/singular_association.rb b/lib/jit_preloader/active_record/associations/singular_association.rb index b603f31..6bcecf0 100644 --- a/lib/jit_preloader/active_record/associations/singular_association.rb +++ b/lib/jit_preloader/active_record/associations/singular_association.rb @@ -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 diff --git a/spec/lib/jit_preloader/preloader_spec.rb b/spec/lib/jit_preloader/preloader_spec.rb index 69f7038..a932f6f 100644 --- a/spec/lib/jit_preloader/preloader_spec.rb +++ b/spec/lib/jit_preloader/preloader_spec.rb @@ -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 @@ -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 From 94c257ed34b0edd37fc6664ec387e692a200cb99 Mon Sep 17 00:00:00 2001 From: Wendy Chen <139145049+wendy-clio@users.noreply.github.com> Date: Mon, 10 Jun 2024 14:49:08 -0400 Subject: [PATCH 2/2] Update spec/lib/jit_preloader/preloader_spec.rb Co-authored-by: Olivia Werre <38996622+oliviawerre@users.noreply.github.com> --- spec/lib/jit_preloader/preloader_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/jit_preloader/preloader_spec.rb b/spec/lib/jit_preloader/preloader_spec.rb index a932f6f..92e5c11 100644 --- a/spec/lib/jit_preloader/preloader_spec.rb +++ b/spec/lib/jit_preloader/preloader_spec.rb @@ -171,7 +171,7 @@ 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 ActiveSupport::Notifications.subscribed(callback, "n_plus_one_query") do expect(contacts.first.contact_owner).to eq(nil)