Return an ApplicationSetting in CurrentSettings

This replaces the use of fake_application_settings with
`::ApplicationSetting.build`_from_defaults. The reason is that
`fake_application_settings` doesn't have the custom accessors that
`ApplicationSetting` has, e.g. `#commit_email_hostname`, thus this
can lead to unexpected `nil` values which comes from the database
column instead of `.default_commit_email_hostname` returned by
`ApplicationSetting#commit_email_hostname`.

Using `::ApplicationSetting.build_from_defaults` should be safe as it
doesn't try to `INSERT` a DB record, in contrary to
`::ApplicationSetting.create_from_defaults` which we used to use, and
which created issues that the introduction of
`fake_application_settings` tried to resolve (575dced5).

Signed-off-by: Rémy Coutable <remy@rymai.me>

1 files changed, 9 insertions, 1 deletions

diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 477f9101e98..e6c80bed5a5 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -50,7 +50,15 @@ module Gitlab
         # and other callers from failing, use any loaded settings and return
         # defaults for missing columns.
         if ActiveRecord::Migrator.needs_migration?
-          return fake_application_settings(current_settings&.attributes)
+          db_attributes = current_settings&.attributes || {}
+          column_names = ::ApplicationSetting.column_names
+          final_attributes = ::ApplicationSetting
+            .defaults
+            .merge(db_attributes)
+            .stringify_keys
+            .slice(*column_names)
+
+          return ::ApplicationSetting.new(final_attributes)
         end
 
         return current_settings if current_settings.present?