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

app/models/application_setting.rb

@@ -194,7 +194,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)

changelogs/unreleased/fix-application-setting-nil-cache.yml

+---
+title: Fix application setting to cache nil object
+merge_request:
+author:
+type: fixed

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

spec/models/application_setting_spec.rb

@@ -193,6 +193,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 'restricted signup domains' do