From 18ed6ecc6ef10006a773b7400ad1d55125c863e0 Mon Sep 17 00:00:00 2001 From: rhymes Date: Wed, 17 Jun 2020 14:01:42 +0200 Subject: [PATCH 1/2] Rename whitelist to safelist --- README.md | 10 +-- lib/bullet.rb | 67 ++++++++++++++++--- lib/bullet/detector/counter_cache.rb | 2 +- lib/bullet/detector/n_plus_one_query.rb | 2 +- lib/bullet/detector/unused_eager_loading.rb | 2 +- spec/bullet_spec.rb | 49 +++++++++++--- .../active_record/association_spec.rb | 12 ++-- spec/integration/counter_cache_spec.rb | 6 +- 8 files changed, 112 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index ac394a1e..a0f13cf0 100644 --- a/README.md +++ b/README.md @@ -111,15 +111,15 @@ Bullet.unused_eager_loading_enable = false Bullet.counter_cache_enable = false ``` -## Whitelist +## Safe list Sometimes Bullet may notify you of query problems you don't care to fix, or -which come from outside your code. You can whitelist these to ignore them: +which come from outside your code. You can add them to a safe list to ignore them: ```ruby -Bullet.add_whitelist :type => :n_plus_one_query, :class_name => "Post", :association => :comments -Bullet.add_whitelist :type => :unused_eager_loading, :class_name => "Post", :association => :comments -Bullet.add_whitelist :type => :counter_cache, :class_name => "Country", :association => :cities +Bullet.add_safelist :type => :n_plus_one_query, :class_name => "Post", :association => :comments +Bullet.add_safelist :type => :unused_eager_loading, :class_name => "Post", :association => :comments +Bullet.add_safelist :type => :counter_cache, :class_name => "Country", :association => :cities ``` If you want to skip bullet in some specific controller actions, you can diff --git a/lib/bullet.rb b/lib/bullet.rb index 39afbc1a..e800bf90 100644 --- a/lib/bullet.rb +++ b/lib/bullet.rb @@ -58,7 +58,7 @@ def enable=(enable) @enable = @n_plus_one_query_enable = @unused_eager_loading_enable = @counter_cache_enable = enable if enable? - reset_whitelist + reset_safelist unless orm_patches_applied self.orm_patches_applied = true Bullet::Mongoid.enable if mongoid? @@ -95,29 +95,74 @@ def stacktrace_excludes @stacktrace_excludes || [] end + def add_safelist(options) + reset_safelist + Thread.current[:safelist][options[:type]][options[:class_name]] ||= [] + Thread.current[:safelist][options[:type]][options[:class_name]] << options[:association].to_sym + end + + def delete_safelist(options) + reset_safelist + Thread.current[:safelist][options[:type]][options[:class_name]] ||= [] + Thread.current[:safelist][options[:type]][options[:class_name]].delete(options[:association].to_sym) + Thread.current[:safelist][options[:type]].delete_if { |_key, val| val.empty? } + end + + def get_safelist_associations(type, class_name) + Array(Thread.current[:safelist][type][class_name]) + end + + def reset_safelist + Thread.current[:safelist] ||= { n_plus_one_query: {}, unused_eager_loading: {}, counter_cache: {} } + end + + def clear_safelist + Thread.current[:safelist] = nil + end + def add_whitelist(options) - reset_whitelist - Thread.current[:whitelist][options[:type]][options[:class_name]] ||= [] - Thread.current[:whitelist][options[:type]][options[:class_name]] << options[:association].to_sym + ActiveSupport::Deprecation.warn(<<~WARN.strip + add_whitelist is deprecated in favor of add_safelist. It will be removed from the next major release. + WARN + ) + + add_safelist(options) end def delete_whitelist(options) - reset_whitelist - Thread.current[:whitelist][options[:type]][options[:class_name]] ||= [] - Thread.current[:whitelist][options[:type]][options[:class_name]].delete(options[:association].to_sym) - Thread.current[:whitelist][options[:type]].delete_if { |_key, val| val.empty? } + ActiveSupport::Deprecation.warn(<<~WARN.strip + delete_whitelist is deprecated in favor of delete_safelist. It will be removed from the next major release. + WARN + ) + + delete_safelist end def get_whitelist_associations(type, class_name) - Array(Thread.current[:whitelist][type][class_name]) + ActiveSupport::Deprecation.warn(<<~WARN.strip + get_whitelist_associations is deprecated in favor of get_safelist_associations. It will be removed from the next major release. + WARN + ) + + get_safelist_associations(type, class_name) end def reset_whitelist - Thread.current[:whitelist] ||= { n_plus_one_query: {}, unused_eager_loading: {}, counter_cache: {} } + ActiveSupport::Deprecation.warn(<<~WARN.strip + reset_whitelist is deprecated in favor of reset_safelist. It will be removed from the next major release. + WARN + ) + + reset_safelist end def clear_whitelist - Thread.current[:whitelist] = nil + ActiveSupport::Deprecation.warn(<<~WARN.strip + clear_whitelist is deprecated in favor of clear_safelist. It will be removed from the next major release. + WARN + ) + + clear_safelist end def bullet_logger=(active) diff --git a/lib/bullet/detector/counter_cache.rb b/lib/bullet/detector/counter_cache.rb index 471fc025..c0cc59b5 100644 --- a/lib/bullet/detector/counter_cache.rb +++ b/lib/bullet/detector/counter_cache.rb @@ -54,7 +54,7 @@ def impossible_objects private def create_notification(klazz, associations) - notify_associations = Array(associations) - Bullet.get_whitelist_associations(:counter_cache, klazz) + notify_associations = Array(associations) - Bullet.get_safelist_associations(:counter_cache, klazz) if notify_associations.present? notice = Bullet::Notification::CounterCache.new klazz, notify_associations diff --git a/lib/bullet/detector/n_plus_one_query.rb b/lib/bullet/detector/n_plus_one_query.rb index acc8db43..e5da9629 100644 --- a/lib/bullet/detector/n_plus_one_query.rb +++ b/lib/bullet/detector/n_plus_one_query.rb @@ -95,7 +95,7 @@ def association?(object, associations) private def create_notification(callers, klazz, associations) - notify_associations = Array(associations) - Bullet.get_whitelist_associations(:n_plus_one_query, klazz) + notify_associations = Array(associations) - Bullet.get_safelist_associations(:n_plus_one_query, klazz) if notify_associations.present? notice = Bullet::Notification::NPlusOneQuery.new(callers, klazz, notify_associations) diff --git a/lib/bullet/detector/unused_eager_loading.rb b/lib/bullet/detector/unused_eager_loading.rb index 831c1272..b0eb2a71 100644 --- a/lib/bullet/detector/unused_eager_loading.rb +++ b/lib/bullet/detector/unused_eager_loading.rb @@ -65,7 +65,7 @@ def add_eager_loadings(objects, associations) private def create_notification(callers, klazz, associations) - notify_associations = Array(associations) - Bullet.get_whitelist_associations(:unused_eager_loading, klazz) + notify_associations = Array(associations) - Bullet.get_safelist_associations(:unused_eager_loading, klazz) if notify_associations.present? notice = Bullet::Notification::UnusedEagerLoading.new(callers, klazz, notify_associations) diff --git a/spec/bullet_spec.rb b/spec/bullet_spec.rb index ff252006..6f1237f4 100644 --- a/spec/bullet_spec.rb +++ b/spec/bullet_spec.rb @@ -74,31 +74,60 @@ end end + describe '#add_safelist' do + context "for 'special' class names" do + it 'is added to the safelist successfully' do + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to include :department + end + end + end + describe '#add_whitelist' do context "for 'special' class names" do - it 'is added to the whitelist successfully' do + it 'is added to the safelist successfully' do Bullet.add_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) - expect(Bullet.get_whitelist_associations(:n_plus_one_query, 'Klass')).to include :department + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to include :department + end + end + end + + describe '#delete_safelist' do + context "for 'special' class names" do + it 'is deleted from the safelist successfully' do + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + Bullet.delete_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + expect(Thread.current[:safelist][:n_plus_one_query]).to eq({}) + end + end + + context 'when exists multiple definitions' do + it 'is deleted from the safelist successfully' do + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :team) + Bullet.delete_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :team) + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to include :department + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to_not include :team end end end describe '#delete_whitelist' do context "for 'special' class names" do - it 'is deleted from the whitelist successfully' do - Bullet.add_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + it 'is deleted from the safelist successfully' do + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) Bullet.delete_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) - expect(Thread.current[:whitelist][:n_plus_one_query]).to eq({}) + expect(Thread.current[:safelist][:n_plus_one_query]).to eq({}) end end context 'when exists multiple definitions' do - it 'is deleted from the whitelist successfully' do - Bullet.add_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) - Bullet.add_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :team) + it 'is deleted from the safelist successfully' do + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :department) + Bullet.add_safelist(type: :n_plus_one_query, class_name: 'Klass', association: :team) Bullet.delete_whitelist(type: :n_plus_one_query, class_name: 'Klass', association: :team) - expect(Bullet.get_whitelist_associations(:n_plus_one_query, 'Klass')).to include :department - expect(Bullet.get_whitelist_associations(:n_plus_one_query, 'Klass')).to_not include :team + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to include :department + expect(Bullet.get_safelist_associations(:n_plus_one_query, 'Klass')).to_not include :team end end end diff --git a/spec/integration/active_record/association_spec.rb b/spec/integration/active_record/association_spec.rb index 49b2186b..0e32872f 100644 --- a/spec/integration/active_record/association_spec.rb +++ b/spec/integration/active_record/association_spec.rb @@ -693,9 +693,9 @@ end end - context 'whitelist n plus one query' do - before { Bullet.add_whitelist type: :n_plus_one_query, class_name: 'Post', association: :comments } - after { Bullet.clear_whitelist } + context 'add n plus one query to safelist' do + before { Bullet.add_safelist type: :n_plus_one_query, class_name: 'Post', association: :comments } + after { Bullet.clear_safelist } it 'should not detect n plus one query' do Post.all.each { |post| post.comments.map(&:name) } @@ -714,9 +714,9 @@ end end - context 'whitelist unused eager loading' do - before { Bullet.add_whitelist type: :unused_eager_loading, class_name: 'Post', association: :comments } - after { Bullet.clear_whitelist } + context 'add unused eager loading to safelist' do + before { Bullet.add_safelist type: :unused_eager_loading, class_name: 'Post', association: :comments } + after { Bullet.clear_safelist } it 'should not detect unused eager loading' do Post.includes(:comments).map(&:name) diff --git a/spec/integration/counter_cache_spec.rb b/spec/integration/counter_cache_spec.rb index eb2a4d33..7cc23a45 100644 --- a/spec/integration/counter_cache_spec.rb +++ b/spec/integration/counter_cache_spec.rb @@ -55,9 +55,9 @@ end end - context 'whitelist' do - before { Bullet.add_whitelist type: :counter_cache, class_name: 'Country', association: :cities } - after { Bullet.clear_whitelist } + context 'safelist' do + before { Bullet.add_safelist type: :counter_cache, class_name: 'Country', association: :cities } + after { Bullet.clear_safelist } it 'should not detect counter cache' do Country.all.each { |country| country.cities.size } From d3854f95567858292933ecdeaccd05d221cf9020 Mon Sep 17 00:00:00 2001 From: rhymes Date: Wed, 17 Jun 2020 14:24:20 +0200 Subject: [PATCH 2/2] Fix method call --- lib/bullet.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bullet.rb b/lib/bullet.rb index e800bf90..dcded22b 100644 --- a/lib/bullet.rb +++ b/lib/bullet.rb @@ -135,7 +135,7 @@ def delete_whitelist(options) WARN ) - delete_safelist + delete_safelist(options) end def get_whitelist_associations(type, class_name)