Used to_boolean for pages_storage_check

Used to_boolean for pages_storage_check

config/initializers/pages_storage_check.rb

@@ -2,21 +2,13 @@
 
 # This is to make sure at least one storage strategy for Pages is enabled.
 
-return if ::Feature.enabled?(:pages_update_legacy_storage, default_enabled: :yaml)
-
 pages = Settings.pages
 
-return unless pages['enabled']
-
-def check_boolean(val, attribute)
-  return if val.nil? || !!val == val
-
-  raise "Please set either true or false for pages:#{attribute}:enabled setting."
-end
+return unless pages['enabled'] && pages['local_store']
 
-check_boolean(pages['local_store']['enabled'], 'local_store')
-check_boolean(pages['object_store']['enabled'], 'object_store')
+local_store_enabled = Gitlab::Utils.to_boolean(pages['local_store']['enabled'])
+object_store_enabled = Gitlab::Utils.to_boolean(pages['object_store']['enabled'])
 
-if !pages['local_store']['enabled'] && !pages['object_store']['enabled']
-  raise "Please enable at least one of the two Pages storage strategy (local_store or object_store) in your config/gitlab.yml - set their 'enabled' attribute to true."
+if !local_store_enabled && !object_store_enabled
+  raise "Please enable at least one of the two Pages storage strategy (local_store or object_store) in your config/gitlab.yml."
 end

lib/gitlab/utils.rb

@@ -99,6 +99,8 @@ module Gitlab
     end
 
     def to_boolean(value, default: nil)
+      value = value.to_s if [0, 1].include?(value)
+
       return value if [true, false].include?(value)
       return true if value =~ /^(true|t|yes|y|1|on)$/i
       return false if value =~ /^(false|f|no|n|0|off)$/i

spec/initializers/pages_storage_check_spec.rb

@@ -3,95 +3,91 @@
 require 'spec_helper'
 
 RSpec.describe 'pages storage check' do
-  let(:main_error_message) { "Please enable at least one of the two Pages storage strategy (local_store or object_store) in your config/gitlab.yml - set their 'enabled' attribute to true." }
+  let(:main_error_message) { "Please enable at least one of the two Pages storage strategy (local_store or object_store) in your config/gitlab.yml." }
 
   subject(:initializer) { load Rails.root.join('config/initializers/pages_storage_check.rb') }
 
-  context 'when the pages_update_legacy_storage FF is turned on' do
+  context 'when local store does not exist yet' do
     before do
-      stub_feature_flags(pages_update_legacy_storage: true)
+      Settings.pages['local_store'] = nil
     end
 
     it { is_expected.to be_truthy }
   end
 
-  context 'when the pages_update_legacy_storage FF is turned false' do
+  context 'when pages is not enabled' do
     before do
-      stub_feature_flags(pages_update_legacy_storage: false)
+      Settings.pages['enabled'] = false
     end
 
-    context 'when pages is not enabled' do
-      before do
-        Settings.pages['enabled'] = false
-      end
+    it { is_expected.to be_truthy }
+  end
 
-      it { is_expected.to be_truthy }
+  context 'when pages is enabled' do
+    before do
+      Settings.pages['enabled'] = true
+      Settings.pages['local_store'] = Settingslogic.new({})
     end
 
-    context 'when pages is enabled' do
+    context 'when pages object storage is not enabled' do
       before do
-        Settings.pages['enabled'] = true
+        Settings.pages['object_store']['enabled'] = false
       end
 
-      context 'when pages object storage is not enabled' do
-        before do
-          Settings.pages['object_store']['enabled'] = false
-        end
-
-        context 'when pages local storage is not enabled' do
-          it 'raises an exception' do
-            Settings.pages['local_store']['enabled'] = false
+      context 'when pages local storage is not enabled' do
+        it 'raises an exception' do
+          Settings.pages['local_store']['enabled'] = false
 
-            expect { subject }.to raise_error(main_error_message)
-          end
+          expect { subject }.to raise_error(main_error_message)
         end
+      end
 
-        context 'when pages local storage is enabled' do
-          it 'is true' do
-            Settings.pages['local_store']['enabled'] = true
+      context 'when pages local storage is enabled' do
+        it 'is true' do
+          Settings.pages['local_store']['enabled'] = true
 
-            expect(subject).to be_truthy
-          end
+          expect(subject).to be_truthy
         end
       end
+    end
 
-      context 'when pages object storage is enabled' do
-        before do
-          Settings.pages['object_store']['enabled'] = true
-        end
+    context 'when pages object storage is enabled' do
+      before do
+        Settings.pages['object_store']['enabled'] = true
+      end
 
-        context 'when pages local storage is not enabled' do
-          it 'is true' do
-            Settings.pages['local_store']['enabled'] = false
+      context 'when pages local storage is not enabled' do
+        it 'is true' do
+          Settings.pages['local_store']['enabled'] = false
 
-            expect(subject).to be_truthy
-          end
+          expect(subject).to be_truthy
         end
+      end
 
-        context 'when pages local storage is enabled' do
-          it 'is true' do
-            Settings.pages['local_store']['enabled'] = true
+      context 'when pages local storage is enabled' do
+        it 'is true' do
+          Settings.pages['local_store']['enabled'] = true
 
-            expect(subject).to be_truthy
-          end
+          expect(subject).to be_truthy
         end
       end
+    end
 
-      context 'when enabled attributes are set with a character instead of a boolean' do
-        it 'raises an exception' do
-          Settings.pages['local_store']['enabled'] = 0
+    context 'when using integers instead of booleans' do
+      it 'is true' do
+        Settings.pages['local_store']['enabled'] = 1
+        Settings.pages['object_store']['enabled'] = 0
 
-          expect { subject }.to raise_error("Please set either true or false for pages:local_store:enabled setting.")
-        end
+        expect(subject).to be_truthy
       end
+    end
 
-      context 'when both enabled attributes are not set' do
-        it 'raises an exception' do
-          Settings.pages['local_store']['enabled'] = nil
-          Settings.pages['object_store']['enabled'] = nil
+    context 'when both enabled attributes are not set' do
+      it 'raises an exception' do
+        Settings.pages['local_store']['enabled'] = nil
+        Settings.pages['object_store']['enabled'] = nil
 
-          expect { subject }.to raise_error(main_error_message)
-        end
+        expect { subject }.to raise_error(main_error_message)
       end
     end
   end

spec/lib/gitlab/utils_spec.rb

@@ -192,6 +192,7 @@ RSpec.describe Gitlab::Utils do
       expect(to_boolean('YeS')).to be(true)
       expect(to_boolean('t')).to be(true)
       expect(to_boolean('1')).to be(true)
+      expect(to_boolean(1)).to be(true)
       expect(to_boolean('ON')).to be(true)
 
       expect(to_boolean('FaLse')).to be(false)
@@ -199,6 +200,7 @@ RSpec.describe Gitlab::Utils do
       expect(to_boolean('NO')).to be(false)
       expect(to_boolean('n')).to be(false)
       expect(to_boolean('0')).to be(false)
+      expect(to_boolean(0)).to be(false)
       expect(to_boolean('oFF')).to be(false)
     end