Commit 20015839 authored by Stan Hu's avatar Stan Hu

Move application setting schema check into create_from_defaults

`ApplicationSetting#create_from_defaults` is also called from the API so
we should cover all the bases by moving the primary ID check here.
parent fc715de9
......@@ -427,6 +427,8 @@ class ApplicationSetting < ApplicationRecord
end
def self.create_from_defaults
check_schema!
transaction(requires_new: true) do
super
end
......@@ -435,6 +437,22 @@ class ApplicationSetting < ApplicationRecord
current_without_cache
end
# Due to the frequency with which settings are accessed, it is
# likely that during a backup restore a running GitLab process
# will insert a new `application_settings` row before the
# constraints have been added to the table. This would add an
# extra row with ID 1 and prevent the primary key constraint from
# being added, which made ActiveRecord throw a
# IrreversibleOrderError anytime the settings were accessed
# (https://gitlab.com/gitlab-org/gitlab/-/issues/36405). To
# prevent this from happening, we do a sanity check that the
# primary key constraint is present before inserting a new entry.
def self.check_schema!
return if ActiveRecord::Base.connection.primary_key(self.table_name).present?
raise "The `#{self.table_name}` table is missing a primary key constraint in the database schema"
end
# By default, the backend is Rails.cache, which uses
# ActiveSupport::Cache::RedisStore. Since loading ApplicationSetting
# can cause a significant amount of load on Redis, let's cache it in
......
......@@ -56,8 +56,6 @@ module Gitlab
elsif current_settings.present?
current_settings
else
check_application_settings_schema!
::ApplicationSetting.create_from_defaults
end
end
......@@ -70,22 +68,6 @@ module Gitlab
@in_memory_application_settings ||= ::ApplicationSetting.build_from_defaults
end
# Due to the frequency with which settings are accessed, it is
# likely that during a backup restore a running GitLab process
# will insert a new `application_settings` row before the
# constraints have been added to the table. This would add an
# extra row with ID 1 and prevent the primary key constraint from
# being added, which made ActiveRecord throw a
# IrreversibleOrderError anytime the settings were accessed
# (https://gitlab.com/gitlab-org/gitlab/-/issues/36405). To
# prevent this from happening, we do a sanity check that the
# primary key constraint is present before inserting a new entry.
def check_application_settings_schema!
return if ActiveRecord::Base.connection.primary_key(ApplicationSetting.table_name).present?
raise "The `application_settings` table is missing a primary key constraint in the database schema"
end
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
......
......@@ -650,6 +650,16 @@ RSpec.describe ApplicationSetting do
end
end
context 'when ApplicationSettings does not have a primary key' do
before do
allow(ActiveRecord::Base.connection).to receive(:primary_key).with(described_class.table_name).and_return(nil)
end
it 'raises an exception' do
expect { described_class.create_from_defaults }.to raise_error(/table is missing a primary key constraint/)
end
end
describe '#disabled_oauth_sign_in_sources=' do
before do
allow(Devise).to receive(:omniauth_providers).and_return([:github])
......
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