Commit 24803d71 authored by Thong Kuah's avatar Thong Kuah

Merge branch '339816-dont-detect-issues-in-factories' into 'master'

Ignore FactoryBot #create for cross-database modification detection

See merge request gitlab-org/gitlab!72941
parents 464293c8 efbb0d41
...@@ -61,6 +61,7 @@ module Database ...@@ -61,6 +61,7 @@ module Database
return unless cross_database_context[:enabled] return unless cross_database_context[:enabled]
return if connection.pool.instance_of?(ActiveRecord::ConnectionAdapters::NullPool) return if connection.pool.instance_of?(ActiveRecord::ConnectionAdapters::NullPool)
return if in_factory_bot_create?
database = connection.pool.db_config.name database = connection.pool.db_config.name
...@@ -113,6 +114,15 @@ module Database ...@@ -113,6 +114,15 @@ module Database
raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, message raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, message
end end
end end
# We ignore execution in the #create method from FactoryBot
# because it is not representative of real code we run in
# production. There are far too many false positives caused
# by instantiating objects in different `gitlab_schema` in a
# FactoryBot `create`.
def self.in_factory_bot_create?
caller_locations.any? { |l| l.path.end_with?('lib/factory_bot/evaluation.rb') && l.label == 'create' }
end
end end
end end
......
...@@ -127,6 +127,14 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do ...@@ -127,6 +127,14 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do
expect { run_queries }.to raise_error /Cross-database data modification/ expect { run_queries }.to raise_error /Cross-database data modification/
end end
end end
context 'when the modification is inside a factory save! call' do
let(:runner) { create(:ci_runner, :project, projects: [build(:project)]) }
it 'does not raise an error' do
runner
end
end
end end
end end
end end
...@@ -152,14 +160,6 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do ...@@ -152,14 +160,6 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do
end.not_to raise_error end.not_to raise_error
end end
it 'raises error when complex factories are built referencing both databases' do
expect do
ApplicationRecord.transaction do
create(:ci_pipeline)
end
end.to raise_error /Cross-database data modification/
end
it 'skips raising error on factory creation' do it 'skips raising error on factory creation' do
expect do expect do
Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'gitlab-issue') do Gitlab::Database.allow_cross_database_modification_within_transaction(url: 'gitlab-issue') do
......
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