Make Gitlab::CurrentSettings.current_application_settings return cached settings early if they exist without issuing any DB query
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
parent
1bcdd25ce2
commit
6afe6fa6bc
|
@ -32,8 +32,10 @@ module Gitlab
|
|||
|
||||
begin
|
||||
::ApplicationSetting.cached
|
||||
rescue ::Redis::BaseError, ::Errno::ENOENT, ::Errno::EADDRNOTAVAIL
|
||||
# In case Redis isn't running or the Redis UNIX socket file is not available
|
||||
rescue
|
||||
# In case Redis isn't running
|
||||
# or the Redis UNIX socket file is not available
|
||||
# or the DB is not running (we use migrations in the cache key)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -5,6 +5,13 @@ describe Gitlab::CurrentSettings do
|
|||
stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
|
||||
end
|
||||
|
||||
shared_context 'with settings in cache' do
|
||||
before do
|
||||
create(:application_setting)
|
||||
described_class.current_application_settings # warm the cache
|
||||
end
|
||||
end
|
||||
|
||||
describe '#current_application_settings', :use_clean_rails_memory_store_caching do
|
||||
it 'allows keys to be called directly' do
|
||||
db_settings = create(:application_setting,
|
||||
|
@ -31,17 +38,30 @@ describe Gitlab::CurrentSettings do
|
|||
end
|
||||
|
||||
context 'with DB unavailable' do
|
||||
context 'and settings in cache' do
|
||||
include_context 'with settings in cache'
|
||||
|
||||
it 'fetches the settings from cache without issuing any query' do
|
||||
expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
context 'and no settings in cache' do
|
||||
before do
|
||||
# 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)
|
||||
expect(ApplicationSetting).not_to receive(:current)
|
||||
end
|
||||
|
||||
it 'returns an in-memory ApplicationSetting object' do
|
||||
expect(ApplicationSetting).not_to receive(:current)
|
||||
|
||||
expect(described_class.current_application_settings).to be_a(Gitlab::FakeApplicationSettings)
|
||||
end
|
||||
|
||||
it 'does not issue any query' do
|
||||
expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with DB available' do
|
||||
|
@ -52,9 +72,21 @@ describe Gitlab::CurrentSettings do
|
|||
ar_wrapped_defaults.slice(*::ApplicationSetting.defaults.keys)
|
||||
end
|
||||
|
||||
before do
|
||||
context 'and settings in cache' do
|
||||
include_context 'with settings in cache'
|
||||
|
||||
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(ActiveRecord::Migrator).not_to receive(:needs_migration?)
|
||||
expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
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)
|
||||
end
|
||||
|
@ -123,4 +155,5 @@ describe Gitlab::CurrentSettings do
|
|||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue