Merge branch 'dz-refactor-object-store-settings' into 'master'

Create class responsible for default object store settings

See merge request gitlab-org/gitlab-ce!20785

3 files changed, 48 insertions, 23 deletions

diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index c3122827a6b..3230d9c0a5b 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -1,4 +1,5 @@
 require_relative '../settings'
+require_relative '../object_store_settings'
 
 # Default settings
 Settings['ldap'] ||= Settingslogic.new({})
@@ -179,14 +180,7 @@ Settings.artifacts['storage_path'] = Settings.absolute(Settings.artifacts.values
 # Settings.artifact['path'] is deprecated, use `storage_path` instead
 Settings.artifacts['path']         = Settings.artifacts['storage_path']
 Settings.artifacts['max_size'] ||= 100 # in megabytes
-Settings.artifacts['object_store'] ||= Settingslogic.new({})
-Settings.artifacts['object_store']['enabled'] = false if Settings.artifacts['object_store']['enabled'].nil?
-Settings.artifacts['object_store']['remote_directory'] ||= nil
-Settings.artifacts['object_store']['direct_upload'] = false if Settings.artifacts['object_store']['direct_upload'].nil?
-Settings.artifacts['object_store']['background_upload'] = true if Settings.artifacts['object_store']['background_upload'].nil?
-Settings.artifacts['object_store']['proxy_download'] = false if Settings.artifacts['object_store']['proxy_download'].nil?
-# Convert upload connection settings to use string keys, to make Fog happy
-Settings.artifacts['object_store']['connection']&.deep_stringify_keys!
+Settings.artifacts['object_store'] = ObjectStoreSettings.parse(Settings.artifacts['object_store'])
 
 #
 # Registry
@@ -225,14 +219,7 @@ Settings.pages.admin['certificate'] ||= ''
 Settings['lfs'] ||= Settingslogic.new({})
 Settings.lfs['enabled']      = true if Settings.lfs['enabled'].nil?
 Settings.lfs['storage_path'] = Settings.absolute(Settings.lfs['storage_path'] || File.join(Settings.shared['path'], "lfs-objects"))
-Settings.lfs['object_store'] ||= Settingslogic.new({})
-Settings.lfs['object_store']['enabled'] = false if Settings.lfs['object_store']['enabled'].nil?
-Settings.lfs['object_store']['remote_directory'] ||= nil
-Settings.lfs['object_store']['direct_upload'] = false if Settings.lfs['object_store']['direct_upload'].nil?
-Settings.lfs['object_store']['background_upload'] = true if Settings.lfs['object_store']['background_upload'].nil?
-Settings.lfs['object_store']['proxy_download'] = false if Settings.lfs['object_store']['proxy_download'].nil?
-# Convert upload connection settings to use string keys, to make Fog happy
-Settings.lfs['object_store']['connection']&.deep_stringify_keys!
+Settings.lfs['object_store'] = ObjectStoreSettings.parse(Settings.lfs['object_store'])
 
 #
 # Uploads
@@ -240,14 +227,8 @@ Settings.lfs['object_store']['connection']&.deep_stringify_keys!
 Settings['uploads'] ||= Settingslogic.new({})
 Settings.uploads['storage_path'] = Settings.absolute(Settings.uploads['storage_path'] || 'public')
 Settings.uploads['base_dir'] = Settings.uploads['base_dir'] || 'uploads/-/system'
-Settings.uploads['object_store'] ||= Settingslogic.new({})
-Settings.uploads['object_store']['enabled'] = false if Settings.uploads['object_store']['enabled'].nil?
+Settings.uploads['object_store'] = ObjectStoreSettings.parse(Settings.uploads['object_store'])
 Settings.uploads['object_store']['remote_directory'] ||= 'uploads'
-Settings.uploads['object_store']['direct_upload'] = false if Settings.uploads['object_store']['direct_upload'].nil?
-Settings.uploads['object_store']['background_upload'] = true if Settings.uploads['object_store']['background_upload'].nil?
-Settings.uploads['object_store']['proxy_download'] = false if Settings.uploads['object_store']['proxy_download'].nil?
-# Convert upload connection settings to use string keys, to make Fog happy
-Settings.uploads['object_store']['connection']&.deep_stringify_keys!
 
 #
 # Mattermost
diff --git a/config/object_store_settings.rb b/config/object_store_settings.rb
new file mode 100644
index 00000000000..d85ff394dcc
--- /dev/null
+++ b/config/object_store_settings.rb
@@ -0,0 +1,15 @@
+# Set default values for object_store settings
+class ObjectStoreSettings
+  def self.parse(object_store)
+    object_store ||= Settingslogic.new({})
+    object_store['enabled'] = false if object_store['enabled'].nil?
+    object_store['remote_directory'] ||= nil
+    object_store['direct_upload'] = false if object_store['direct_upload'].nil?
+    object_store['background_upload'] = true if object_store['background_upload'].nil?
+    object_store['proxy_download'] = false if object_store['proxy_download'].nil?
+
+    # Convert upload connection settings to use string keys, to make Fog happy
+    object_store['connection']&.deep_stringify_keys!
+    object_store
+  end
+end
diff --git a/spec/config/object_store_settings_spec.rb b/spec/config/object_store_settings_spec.rb
new file mode 100644
index 00000000000..b1ada3c99db
--- /dev/null
+++ b/spec/config/object_store_settings_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+require Rails.root.join('config', 'object_store_settings.rb')
+
+describe ObjectStoreSettings do
+  describe '.parse' do
+    it 'should set correct default values' do
+      settings = described_class.parse(nil)
+
+      expect(settings['enabled']).to be false
+      expect(settings['direct_upload']).to be false
+      expect(settings['background_upload']).to be true
+      expect(settings['remote_directory']).to be nil
+    end
+
+    it 'respects original values' do
+      original_settings = Settingslogic.new({
+        'enabled' => true,
+        'remote_directory' => 'artifacts'
+      })
+
+      settings = described_class.parse(original_settings)
+
+      expect(settings['enabled']).to be true
+      expect(settings['direct_upload']).to be false
+      expect(settings['background_upload']).to be true
+      expect(settings['remote_directory']).to eq 'artifacts'
+    end
+  end
+end