diff options
author | Winnie Hellmann <winnie@gitlab.com> | 2017-10-20 19:20:29 +0000 |
---|---|---|
committer | Winnie Hellmann <winnie@gitlab.com> | 2017-10-20 19:20:29 +0000 |
commit | 81175d2c37d7bb9768ee21b13207ef57d11ad3ea (patch) | |
tree | 901b7ca83dbaeab2c3eec5ff013dd47e9ddf8fdd | |
parent | cc8af554fdfa6d54315ae50465c42603a44780b1 (diff) | |
parent | beeed14f0470e912a2add7a7b94cb1cc1c6adb07 (diff) | |
download | gitlab-ce-81175d2c37d7bb9768ee21b13207ef57d11ad3ea.tar.gz |
Merge branch 'fix-application-setting-nil-cache' into 'master'
Prevent ApplicationSetting to cache nil value
Closes #39275
See merge request gitlab-org/gitlab-ce!14952
-rw-r--r-- | app/models/application_setting.rb | 5 | ||||
-rw-r--r-- | changelogs/unreleased/fix-application-setting-nil-cache.yml | 5 | ||||
-rw-r--r-- | spec/lib/gitlab/current_settings_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/application_setting_spec.rb | 15 |
4 files changed, 25 insertions, 2 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index d3b8debb0fd..4dda276bb41 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -203,7 +203,10 @@ class ApplicationSetting < ActiveRecord::Base ensure_cache_setup Rails.cache.fetch(CACHE_KEY) do - ApplicationSetting.last + ApplicationSetting.last.tap do |settings| + # do not cache nils + raise 'missing settings' unless settings + end end rescue # Fall back to an uncached value if there are any problems (e.g. redis down) diff --git a/changelogs/unreleased/fix-application-setting-nil-cache.yml b/changelogs/unreleased/fix-application-setting-nil-cache.yml new file mode 100644 index 00000000000..a5f028e3d69 --- /dev/null +++ b/changelogs/unreleased/fix-application-setting-nil-cache.yml @@ -0,0 +1,5 @@ +--- +title: Fix application setting to cache nil object +merge_request: +author: +type: fixed diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index d57ffcae8e1..492659a82b0 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::CurrentSettings do it 'falls back to DB if Redis returns an empty value' do expect(ApplicationSetting).to receive(:cached).and_return(nil) - expect(ApplicationSetting).to receive(:last).and_call_original + expect(ApplicationSetting).to receive(:last).and_call_original.twice expect(current_application_settings).to be_a(ApplicationSetting) end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 6945c90cb9b..30495fd4f5e 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -220,6 +220,21 @@ describe ApplicationSetting do expect(described_class.current).to eq(:last) end end + + context 'when an ApplicationSetting is not yet present' do + it 'does not cache nil object' do + # when missing settings a nil object is returned, but not cached + allow(described_class).to receive(:last).and_return(nil).twice + expect(described_class.current).to be_nil + + # when the settings are set the method returns a valid object + allow(described_class).to receive(:last).and_return(:last) + expect(described_class.current).to eq(:last) + + # subsequent calls get everything from cache + expect(described_class.current).to eq(:last) + end + end end context 'restrict creating duplicates' do |