Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename whitelist to safelist #511

Merged
merged 3 commits into from Aug 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
Expand Up @@ -121,15 +121,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
Expand Down
69 changes: 57 additions & 12 deletions lib/bullet.rb
Expand Up @@ -35,7 +35,7 @@ class << self
:stacktrace_includes,
:stacktrace_excludes,
:skip_html_injection
attr_reader :whitelist
attr_reader :safelist
attr_accessor :add_footer, :orm_patches_applied, :skip_http_headers

available_notifiers =
Expand All @@ -57,7 +57,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?
Expand Down Expand Up @@ -95,29 +95,74 @@ def stacktrace_excludes
@stacktrace_excludes ||= []
end

def add_safelist(options)
reset_safelist
@safelist[options[:type]][options[:class_name]] ||= []
@safelist[options[:type]][options[:class_name]] << options[:association].to_sym
end

def delete_safelist(options)
reset_safelist
@safelist[options[:type]][options[:class_name]] ||= []
@safelist[options[:type]][options[:class_name]].delete(options[:association].to_sym)
@safelist[options[:type]].delete_if { |_key, val| val.empty? }
end

def get_safelist_associations(type, class_name)
Array(@safelist[type][class_name])
end

def reset_safelist
@safelist ||= { n_plus_one_query: {}, unused_eager_loading: {}, counter_cache: {} }
end

def clear_safelist
@safelist = nil
end

def add_whitelist(options)
reset_whitelist
@whitelist[options[:type]][options[:class_name]] ||= []
@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
@whitelist[options[:type]][options[:class_name]] ||= []
@whitelist[options[:type]][options[:class_name]].delete(options[:association].to_sym)
@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(options)
end

def get_whitelist_associations(type, class_name)
Array(@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
@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
@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)
Expand Down
2 changes: 1 addition & 1 deletion lib/bullet/detector/counter_cache.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/bullet/detector/n_plus_one_query.rb
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/bullet/detector/unused_eager_loading.rb
Expand Up @@ -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)
Expand Down
49 changes: 39 additions & 10 deletions spec/bullet_spec.rb
Expand Up @@ -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(Bullet.whitelist[:n_plus_one_query]).to eq({})
expect(Bullet.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
Expand Down
12 changes: 6 additions & 6 deletions spec/integration/active_record/association_spec.rb
Expand Up @@ -738,9 +738,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) }
Expand All @@ -759,9 +759,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)
Expand Down
6 changes: 3 additions & 3 deletions spec/integration/counter_cache_spec.rb
Expand Up @@ -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 }
Expand Down