From b6f43fc958d0e367fae9270e412cfce678775810 Mon Sep 17 00:00:00 2001
From: Matt Kasa <mkasa@gitlab.com>
Date: Wed, 26 Jan 2022 19:25:29 -0800
Subject: [PATCH] Fix use of ActiveRecord::Base in CurrentSettings
Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/350651
---
.rubocop_todo/database/multiple_databases.yml | 2 --
lib/gitlab/current_settings.rb | 4 ++--
spec/lib/gitlab/current_settings_spec.rb | 12 ++++++------
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/.rubocop_todo/database/multiple_databases.yml b/.rubocop_todo/database/multiple_databases.yml
index 28dddef7c79b8..3e48215fd5c8b 100644
--- a/.rubocop_todo/database/multiple_databases.yml
+++ b/.rubocop_todo/database/multiple_databases.yml
@@ -11,7 +11,6 @@ Database/MultipleDatabases:
- ee/spec/services/ee/merge_requests/update_service_spec.rb
- lib/backup/database.rb
- lib/backup/manager.rb
- - lib/gitlab/current_settings.rb
- lib/gitlab/database/load_balancing/load_balancer.rb
- lib/gitlab/database/load_balancing.rb
- lib/gitlab/database/load_balancing/sticking.rb
@@ -29,7 +28,6 @@ Database/MultipleDatabases:
- spec/db/schema_spec.rb
- spec/initializers/database_config_spec.rb
- spec/lib/backup/manager_spec.rb
- - spec/lib/gitlab/current_settings_spec.rb
- spec/lib/gitlab/database_spec.rb
- spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
- spec/lib/gitlab/profiler_spec.rb
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 2d2d8c412364d..0d6767ad56411 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -62,7 +62,7 @@ def uncached_application_settings
# need to be added to the application settings. To prevent Rake tasks
# and other callers from failing, use any loaded settings and return
# defaults for missing columns.
- if Gitlab::Runtime.rake? && ActiveRecord::Base.connection.migration_context.needs_migration?
+ if Gitlab::Runtime.rake? && ::ApplicationSetting.connection.migration_context.needs_migration?
db_attributes = current_settings&.attributes || {}
fake_application_settings(db_attributes)
elsif current_settings.present?
@@ -82,7 +82,7 @@ def in_memory_application_settings
def connect_to_db?
# When the DBMS is not available, an exception (e.g. PG::ConnectionBad) is raised
- active_db_connection = ActiveRecord::Base.connection.active? rescue false
+ active_db_connection = ::ApplicationSetting.connection.active? rescue false
active_db_connection &&
ApplicationSetting.database.cached_table_exists?
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index 46c33d7b7b286..73540a9b0f382 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -118,7 +118,7 @@
allow(Gitlab::Runtime).to receive(:rake?).and_return(true)
# For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(false)` causes issues
# during the initialization phase of the test suite, so instead let's mock the internals of it
- allow(ActiveRecord::Base.connection).to receive(:active?).and_return(false)
+ allow(ApplicationSetting.connection).to receive(:active?).and_return(false)
end
context 'and no settings in cache' do
@@ -150,8 +150,8 @@ def settings_from_defaults
it 'fetches the settings from cache' do
# For some reason, `allow(described_class).to receive(:connect_to_db?).and_return(true)` causes issues
# during the initialization phase of the test suite, so instead let's mock the internals of it
- expect(ActiveRecord::Base.connection).not_to receive(:active?)
- expect(ActiveRecord::Base.connection).not_to receive(:cached_table_exists?)
+ expect(ApplicationSetting.connection).not_to receive(:active?)
+ expect(ApplicationSetting.connection).not_to receive(:cached_table_exists?)
expect_any_instance_of(ActiveRecord::MigrationContext).not_to receive(:needs_migration?)
expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0)
end
@@ -159,8 +159,8 @@ def settings_from_defaults
context 'and no settings in cache' do
before do
- allow(ActiveRecord::Base.connection).to receive(:active?).and_return(true)
- allow(ActiveRecord::Base.connection).to receive(:cached_table_exists?).with('application_settings').and_return(true)
+ allow(ApplicationSetting.connection).to receive(:active?).and_return(true)
+ allow(ApplicationSetting.connection).to receive(:cached_table_exists?).with('application_settings').and_return(true)
end
context 'with RequestStore enabled', :request_store do
@@ -181,7 +181,7 @@ def settings_from_defaults
context 'when ApplicationSettings does not have a primary key' do
before do
- allow(ActiveRecord::Base.connection).to receive(:primary_key).with('application_settings').and_return(nil)
+ allow(ApplicationSetting.connection).to receive(:primary_key).with('application_settings').and_return(nil)
end
it 'raises an exception if ApplicationSettings does not have a primary key' do
--
GitLab