diff --git a/config/initializers/active_record_lifecycle.rb b/config/initializers/active_record_lifecycle.rb index 8d4b6d61abe34b16d5f262af3cd2949b2c27c48c..92cc1d8161790076d8d7cd3a203e59da16480ad8 100644 --- a/config/initializers/active_record_lifecycle.rb +++ b/config/initializers/active_record_lifecycle.rb @@ -5,7 +5,7 @@ if defined?(ActiveRecord::Base) && !Gitlab::Runtime.sidekiq? Gitlab::Cluster::LifecycleEvents.on_worker_start do ActiveSupport.on_load(:active_record) do - ActiveRecord::Base.establish_connection + ActiveRecord::Base.establish_connection # rubocop: disable Database/EstablishConnection Gitlab::AppLogger.debug("ActiveRecord connection established") end diff --git a/config/initializers/database_config.rb b/config/initializers/database_config.rb index a3172fae027c0eae7983351f35433eeba55667b5..050ab1d9b3ec5b53a61f39f952618a122dcd3c30 100644 --- a/config/initializers/database_config.rb +++ b/config/initializers/database_config.rb @@ -13,6 +13,6 @@ # The Geo::TrackingBase model does not yet use connects_to. So, # this will not properly support geo: from config/databse.yml # file yet. This is ACK of the current state and will be fixed. - Geo::TrackingBase.establish_connection(Gitlab::Database.geo_db_config_with_default_pool_size) + Geo::TrackingBase.establish_connection(Gitlab::Database.geo_db_config_with_default_pool_size) # rubocop: disable Database/EstablishConnection end end diff --git a/ee/lib/gitlab/geo/database_tasks.rb b/ee/lib/gitlab/geo/database_tasks.rb index 31d8e97d10f58c05661a7142e679de61fc3c4efb..7a4b7a5154b5b0ebf6c449a9af47b411f2b5386c 100644 --- a/ee/lib/gitlab/geo/database_tasks.rb +++ b/ee/lib/gitlab/geo/database_tasks.rb @@ -132,7 +132,7 @@ def load ActiveRecord::Tasks::DatabaseTasks.load_schema(ActiveRecord::Base.configurations.configs_for(env_name: 'test').first, :sql, ENV['SCHEMA']) ensure if should_reconnect - ActiveRecord::Base.establish_connection(Gitlab::Geo::DatabaseTasks.db_config) + ActiveRecord::Base.establish_connection(Gitlab::Geo::DatabaseTasks.db_config) # rubocop: disable Database/EstablishConnection end end end @@ -204,7 +204,7 @@ def set_db_env(settings) ActiveRecord::Migrator.migrations_paths = ActiveRecord::Tasks::DatabaseTasks.migrations_paths Gitlab::Database::SchemaMigrations::Context.default_schema_migrations_path = settings[:schema_migrations_path] - ActiveRecord::Base.establish_connection(db_config) + ActiveRecord::Base.establish_connection(db_config) # rubocop: disable Database/EstablishConnection end class SeedLoader diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 9e733fc3a0fc59565e12a375972f3d1201d1b4b4..efb0e1ef1e1ae91f5cb4fa8d221b3effcf0bf294 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -170,7 +170,7 @@ namespace :gitlab do # the `ActiveRecord::Base.connection` might be switched to another one # This is due to `if should_reconnect`: # https://github.com/rails/rails/blob/a81aeb63a007ede2fe606c50539417dada9030c7/activerecord/lib/active_record/railties/databases.rake#L622 - ActiveRecord::Base.establish_connection :main + ActiveRecord::Base.establish_connection :main # rubocop: disable Database/EstablishConnection Rake::Task['gitlab:db:create_dynamic_partitions'].invoke end diff --git a/rubocop/cop/database/establish_connection.rb b/rubocop/cop/database/establish_connection.rb new file mode 100644 index 0000000000000000000000000000000000000000..20454887ce2d8238cb77d30167c0aab5b7183e58 --- /dev/null +++ b/rubocop/cop/database/establish_connection.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Database + class EstablishConnection < RuboCop::Cop::Cop + MSG = "Don't establish new database connections, as this slows down " \ + 'tests and may result in new connections using an incorrect configuration' + + def_node_matcher :establish_connection?, <<~PATTERN + (send (const ...) :establish_connection ...) + PATTERN + + def on_send(node) + add_offense(node, location: :expression) if establish_connection?(node) + end + end + end + end +end diff --git a/scripts/insert-rspec-profiling-data b/scripts/insert-rspec-profiling-data index b20118585588cd57a3d6d3f7bbf2a02eb637204a..be25972644c84e5e072e74eb10ccbda98b9fa153 100755 --- a/scripts/insert-rspec-profiling-data +++ b/scripts/insert-rspec-profiling-data @@ -12,7 +12,7 @@ module RspecProfiling # This disables the automatic creation of the database and # table. In the future, we may want a way to specify the host of # the database to connect so that we can call #install. - Result.establish_connection(results_url) + Result.establish_connection(results_url) # rubocop: disable Database/EstablishConnection end def results_url diff --git a/spec/lib/gitlab/database/bulk_update_spec.rb b/spec/lib/gitlab/database/bulk_update_spec.rb index 9a6463c99fa060c10acae9c8eec7fce10e75ff0b..08b4d50f83b5d97ce09d8148e9d087e61018569b 100644 --- a/spec/lib/gitlab/database/bulk_update_spec.rb +++ b/spec/lib/gitlab/database/bulk_update_spec.rb @@ -101,7 +101,7 @@ before do configuration_hash = ActiveRecord::Base.connection_db_config.configuration_hash - ActiveRecord::Base.establish_connection( + ActiveRecord::Base.establish_connection( # rubocop: disable Database/EstablishConnection configuration_hash.merge(prepared_statements: prepared_statements) ) end diff --git a/spec/rubocop/cop/database/establish_connection_spec.rb b/spec/rubocop/cop/database/establish_connection_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..a3c27d33cb0e34203a61037ed56a2d6c7b82c88f --- /dev/null +++ b/spec/rubocop/cop/database/establish_connection_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../../../rubocop/cop/database/establish_connection' + +RSpec.describe RuboCop::Cop::Database::EstablishConnection do + subject(:cop) { described_class.new } + + it 'flags the use of ActiveRecord::Base.establish_connection' do + expect_offense(<<~CODE) + ActiveRecord::Base.establish_connection + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end + + it 'flags the use of ActiveRecord::Base.establish_connection with arguments' do + expect_offense(<<~CODE) + ActiveRecord::Base.establish_connection(:foo) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end + + it 'flags the use of SomeModel.establish_connection' do + expect_offense(<<~CODE) + SomeModel.establish_connection + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't establish new database [...] + CODE + end +end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 316d645f99ffcea86333db9e372bad9a0886ba4d..fb70f82ef875003409cb31ceee246353f9253270 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -67,7 +67,7 @@ def recreate_all_databases! # Migrate each database individually with_reestablished_active_record_base do all_connection_classes.each do |connection_class| - ActiveRecord::Base.establish_connection(connection_class.connection_db_config) + ActiveRecord::Base.establish_connection(connection_class.connection_db_config) # rubocop: disable Database/EstablishConnection ActiveRecord::Tasks::DatabaseTasks.migrate end diff --git a/spec/support_specs/database/multiple_databases_spec.rb b/spec/support_specs/database/multiple_databases_spec.rb index 660563594587969bb3fd1735efd996ee541b32bf..b4cfa253813140a0a3f377dbb4cd986e328eb355 100644 --- a/spec/support_specs/database/multiple_databases_spec.rb +++ b/spec/support_specs/database/multiple_databases_spec.rb @@ -7,13 +7,13 @@ context 'when doing establish_connection' do context 'on ActiveRecord::Base' do it 'raises exception' do - expect { ActiveRecord::Base.establish_connection(:main) }.to raise_error /Cannot re-establish/ + expect { ActiveRecord::Base.establish_connection(:main) }.to raise_error /Cannot re-establish/ # rubocop: disable Database/EstablishConnection end context 'when using with_reestablished_active_record_base' do it 'does not raise exception' do with_reestablished_active_record_base do - expect { ActiveRecord::Base.establish_connection(:main) }.not_to raise_error + expect { ActiveRecord::Base.establish_connection(:main) }.not_to raise_error # rubocop: disable Database/EstablishConnection end end end @@ -25,13 +25,13 @@ end it 'raises exception' do - expect { Ci::ApplicationRecord.establish_connection(:ci) }.to raise_error /Cannot re-establish/ + expect { Ci::ApplicationRecord.establish_connection(:ci) }.to raise_error /Cannot re-establish/ # rubocop: disable Database/EstablishConnection end context 'when using with_reestablished_active_record_base' do it 'does not raise exception' do with_reestablished_active_record_base do - expect { Ci::ApplicationRecord.establish_connection(:main) }.not_to raise_error + expect { Ci::ApplicationRecord.establish_connection(:main) }.not_to raise_error # rubocop: disable Database/EstablishConnection end end end