From e0c9576a26e0bde088b59526eaca2362ccc489d5 Mon Sep 17 00:00:00 2001 From: Quintin Muncaster <12143900+quintinm-dev@users.noreply.github.com> Date: Tue, 11 Apr 2023 10:41:26 -0700 Subject: [PATCH 1/2] detect counter cache for count method --- CHANGELOG.md | 1 + lib/bullet/active_record4.rb | 9 +++++++++ lib/bullet/active_record41.rb | 9 +++++++++ lib/bullet/active_record42.rb | 9 +++++++++ lib/bullet/active_record5.rb | 11 +++++++++++ lib/bullet/active_record52.rb | 11 +++++++++++ lib/bullet/active_record60.rb | 11 +++++++++++ lib/bullet/active_record61.rb | 11 +++++++++++ lib/bullet/active_record70.rb | 11 +++++++++++ spec/integration/counter_cache_spec.rb | 24 ++++++++++++++++++++++++ 10 files changed, 107 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b4bfe23..327df4b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Next Release * Added `always_append_html_body` option, so the html snippet is always included even if there are no notifications +* Added detection of n+1 count queries from `count` method ## 7.0.7 (03/01/2023) diff --git a/lib/bullet/active_record4.rb b/lib/bullet/active_record4.rb index e9729007..f9e1f95d 100644 --- a/lib/bullet/active_record4.rb +++ b/lib/bullet/active_record4.rb @@ -176,6 +176,15 @@ def has_cached_counter?(reflection = reflection()) result end end + + ::ActiveRecord::Associations::CollectionProxy.class_eval do + def count + if Bullet.start? + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end end end end diff --git a/lib/bullet/active_record41.rb b/lib/bullet/active_record41.rb index 74a263fa..a76fd5da 100644 --- a/lib/bullet/active_record41.rb +++ b/lib/bullet/active_record41.rb @@ -168,6 +168,15 @@ def count_records origin_count_records end end + + ::ActiveRecord::Associations::CollectionProxy.class_eval do + def count + if Bullet.start? + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end end end end diff --git a/lib/bullet/active_record42.rb b/lib/bullet/active_record42.rb index 61afbda2..c54249af 100644 --- a/lib/bullet/active_record42.rb +++ b/lib/bullet/active_record42.rb @@ -233,6 +233,15 @@ def count_records origin_count_records end end + + ::ActiveRecord::Associations::CollectionProxy.class_eval do + def count + if Bullet.start? + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end end end end diff --git a/lib/bullet/active_record5.rb b/lib/bullet/active_record5.rb index d4eac028..ff01792f 100644 --- a/lib/bullet/active_record5.rb +++ b/lib/bullet/active_record5.rb @@ -260,6 +260,17 @@ def count_records end end ) + + ::ActiveRecord::Associations::CollectionProxy.prepend( + Module.new do + def count + if Bullet.start? && !proxy_association.is_a?(::ActiveRecord::Associations::ThroughAssociation) + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end + ) end end end diff --git a/lib/bullet/active_record52.rb b/lib/bullet/active_record52.rb index 034a49de..5120cd83 100644 --- a/lib/bullet/active_record52.rb +++ b/lib/bullet/active_record52.rb @@ -242,6 +242,17 @@ def count_records end end ) + + ::ActiveRecord::Associations::CollectionProxy.prepend( + Module.new do + def count + if Bullet.start? && !proxy_association.is_a?(::ActiveRecord::Associations::ThroughAssociation) + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end + ) end end end diff --git a/lib/bullet/active_record60.rb b/lib/bullet/active_record60.rb index 1c0898a9..e20e7c82 100644 --- a/lib/bullet/active_record60.rb +++ b/lib/bullet/active_record60.rb @@ -269,6 +269,17 @@ def count_records end end ) + + ::ActiveRecord::Associations::CollectionProxy.prepend( + Module.new do + def count + if Bullet.start? && !proxy_association.is_a?(::ActiveRecord::Associations::ThroughAssociation) + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end + ) end end end diff --git a/lib/bullet/active_record61.rb b/lib/bullet/active_record61.rb index 05ca07e9..6c29b4ac 100644 --- a/lib/bullet/active_record61.rb +++ b/lib/bullet/active_record61.rb @@ -269,6 +269,17 @@ def count_records end end ) + + ::ActiveRecord::Associations::CollectionProxy.prepend( + Module.new do + def count + if Bullet.start? && !proxy_association.is_a?(::ActiveRecord::Associations::ThroughAssociation) + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end + ) end end end diff --git a/lib/bullet/active_record70.rb b/lib/bullet/active_record70.rb index 8785849c..b15fc828 100644 --- a/lib/bullet/active_record70.rb +++ b/lib/bullet/active_record70.rb @@ -279,6 +279,17 @@ def count_records end end ) + + ::ActiveRecord::Associations::CollectionProxy.prepend( + Module.new do + def count + if Bullet.start? && !proxy_association.is_a?(::ActiveRecord::Associations::ThroughAssociation) + Bullet::Detector::CounterCache.add_counter_cache(proxy_association.owner, proxy_association.reflection.name) + end + super + end + end + ) end end end diff --git a/spec/integration/counter_cache_spec.rb b/spec/integration/counter_cache_spec.rb index 82b081c3..4ea443c3 100644 --- a/spec/integration/counter_cache_spec.rb +++ b/spec/integration/counter_cache_spec.rb @@ -64,5 +64,29 @@ expect(Bullet.collected_counter_cache_notifications).to be_empty end end + + describe 'with count' do + it 'should need counter cache' do + Country.all.each { |country| country.cities.count } + expect(Bullet.collected_counter_cache_notifications).not_to be_empty + end + + it 'should notify even with counter cache' do + Person.all.each { |person| person.pets.count } + expect(Bullet.collected_counter_cache_notifications).not_to be_empty + end + + if ActiveRecord::VERSION::MAJOR > 4 + it 'should not need counter cache for has_many through' do + Client.all.each { |client| client.firms.count } + expect(Bullet.collected_counter_cache_notifications).to be_empty + end + else + it 'should need counter cache for has_many through' do + Client.all.each { |client| client.firms.count } + expect(Bullet.collected_counter_cache_notifications).not_to be_empty + end + end + end end end From b5634f9117c91adf01b1b5f1731b4e5242adb2e6 Mon Sep 17 00:00:00 2001 From: Quintin Muncaster <12143900+quintinm-dev@users.noreply.github.com> Date: Tue, 11 Apr 2023 10:42:01 -0700 Subject: [PATCH 2/2] recommend size method in counter cache notification --- CHANGELOG.md | 1 + lib/bullet/notification/counter_cache.rb | 2 +- spec/bullet/notification/counter_cache_spec.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 327df4b3..7f64b386 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Added `always_append_html_body` option, so the html snippet is always included even if there are no notifications * Added detection of n+1 count queries from `count` method +* Changed the counter cache notification title to recommend using `size` ## 7.0.7 (03/01/2023) diff --git a/lib/bullet/notification/counter_cache.rb b/lib/bullet/notification/counter_cache.rb index aec4f459..1007c61c 100644 --- a/lib/bullet/notification/counter_cache.rb +++ b/lib/bullet/notification/counter_cache.rb @@ -8,7 +8,7 @@ def body end def title - 'Need Counter Cache' + 'Need Counter Cache with Active Record size' end end end diff --git a/spec/bullet/notification/counter_cache_spec.rb b/spec/bullet/notification/counter_cache_spec.rb index 13eae9a0..213bc5e2 100644 --- a/spec/bullet/notification/counter_cache_spec.rb +++ b/spec/bullet/notification/counter_cache_spec.rb @@ -8,7 +8,7 @@ module Notification subject { CounterCache.new(Post, %i[comments votes]) } it { expect(subject.body).to eq(' Post => [:comments, :votes]') } - it { expect(subject.title).to eq('Need Counter Cache') } + it { expect(subject.title).to eq('Need Counter Cache with Active Record size') } end end end