Commit f1d137ec authored by Achilleas Pipinellis's avatar Achilleas Pipinellis

Merge branch '219967-replace-denylisted-language' into 'master'

Remove blacklist/whitelist language in bulk-insert-safe

Closes #219967

See merge request gitlab-org/gitlab!34825
parents 9953e287 a6ae49d3
...@@ -37,7 +37,7 @@ module BulkInsertSafe ...@@ -37,7 +37,7 @@ module BulkInsertSafe
# These are the callbacks we think safe when used on models that are # These are the callbacks we think safe when used on models that are
# written to the database in bulk # written to the database in bulk
CALLBACK_NAME_WHITELIST = Set[ ALLOWED_CALLBACKS = Set[
:initialize, :initialize,
:validate, :validate,
:validation, :validation,
...@@ -179,16 +179,12 @@ module BulkInsertSafe ...@@ -179,16 +179,12 @@ module BulkInsertSafe
end end
def _bulk_insert_callback_allowed?(name, args) def _bulk_insert_callback_allowed?(name, args)
_bulk_insert_whitelisted?(name) || _bulk_insert_saved_from_belongs_to?(name, args) ALLOWED_CALLBACKS.include?(name) || _bulk_insert_saved_from_belongs_to?(name, args)
end end
# belongs_to associations will install a before_save hook during class loading # belongs_to associations will install a before_save hook during class loading
def _bulk_insert_saved_from_belongs_to?(name, args) def _bulk_insert_saved_from_belongs_to?(name, args)
args.first == :before && args.second.to_s.start_with?('autosave_associated_records_for_') args.first == :before && args.second.to_s.start_with?('autosave_associated_records_for_')
end end
def _bulk_insert_whitelisted?(name)
CALLBACK_NAME_WHITELIST.include?(name)
end
end end
end end
...@@ -121,10 +121,10 @@ These callbacks cannot be used with bulk insertions, since they are meant to be ...@@ -121,10 +121,10 @@ These callbacks cannot be used with bulk insertions, since they are meant to be
every instance that is saved or created. Since these events do not fire when every instance that is saved or created. Since these events do not fire when
records are inserted in bulk, we currently disallow their use. records are inserted in bulk, we currently disallow their use.
The specifics around which callbacks are disallowed are defined in The specifics around which callbacks are explicitly allowed are defined in
[`BulkInsertSafe`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/concerns/bulk_insert_safe.rb). [`BulkInsertSafe`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/concerns/bulk_insert_safe.rb).
Consult the module source code for details. If your class uses any of the blacklisted Consult the module source code for details. If your class uses callbacks that are not explicitly designated
functionality, and you `include BulkInsertSafe`, the application will fail with an error. safe and you `include BulkInsertSafe` the application will fail with an error.
### `BulkInsertSafe` versus `InsertAll` ### `BulkInsertSafe` versus `InsertAll`
......
...@@ -7,17 +7,17 @@ RSpec.shared_examples 'a BulkInsertSafe model' do |klass| ...@@ -7,17 +7,17 @@ RSpec.shared_examples 'a BulkInsertSafe model' do |klass|
let(:target_class) { klass.dup } let(:target_class) { klass.dup }
# We consider all callbacks unsafe for bulk insertions unless we have explicitly # We consider all callbacks unsafe for bulk insertions unless we have explicitly
# whitelisted them (esp. anything related to :save, :create, :commit etc.) # allowed them (especially anything related to :save, :create, :commit, etc.)
let(:callback_method_blacklist) do let(:unsafe_callbacks) do
ActiveRecord::Callbacks::CALLBACKS.reject do |callback| ActiveRecord::Callbacks::CALLBACKS.reject do |callback|
cb_name = callback.to_s.gsub(/(before_|after_|around_)/, '').to_sym cb_name = callback.to_s.gsub(/(before_|after_|around_)/, '').to_sym
BulkInsertSafe::CALLBACK_NAME_WHITELIST.include?(cb_name) BulkInsertSafe::ALLOWED_CALLBACKS.include?(cb_name)
end.to_set end.to_set
end end
context 'when calling class methods directly' do context 'when calling class methods directly' do
it 'raises an error when method is not bulk-insert safe' do it 'raises an error when method is not bulk-insert safe' do
callback_method_blacklist.each do |m| unsafe_callbacks.each do |m|
expect { target_class.send(m, nil) }.to( expect { target_class.send(m, nil) }.to(
raise_error(BulkInsertSafe::MethodNotAllowedError), raise_error(BulkInsertSafe::MethodNotAllowedError),
"Expected call to #{m} to raise an error, but it didn't" "Expected call to #{m} to raise an error, but it didn't"
...@@ -26,7 +26,7 @@ RSpec.shared_examples 'a BulkInsertSafe model' do |klass| ...@@ -26,7 +26,7 @@ RSpec.shared_examples 'a BulkInsertSafe model' do |klass|
end end
it 'does not raise an error when method is bulk-insert safe' do it 'does not raise an error when method is bulk-insert safe' do
BulkInsertSafe::CALLBACK_NAME_WHITELIST.each do |name| BulkInsertSafe::ALLOWED_CALLBACKS.each do |name|
expect { target_class.set_callback(name) {} }.not_to raise_error expect { target_class.set_callback(name) {} }.not_to raise_error
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment