Merge branch 'mk-fake-app-settings-rework' into 'master'

Drop OpenStruct use from FakeApplicationSettings

See merge request gitlab-org/gitlab!67855

lib/gitlab/fake_application_settings.rb

 # frozen_string_literal: true
 
-# This class extends an OpenStruct object by adding predicate methods to mimic
+# Fakes ActiveRecord attribute storage by adding predicate methods to mimic
 # ActiveRecord access. We rely on the initial values being true or false to
 # determine whether to define a predicate method because for a newly-added
 # column that has not been migrated yet, there is no way to determine the
 # column type without parsing db/structure.sql.
 module Gitlab
-  class FakeApplicationSettings < OpenStruct
-    include ApplicationSettingImplementation
-
-    # Mimic ActiveRecord predicate methods for boolean values
-    def self.define_predicate_methods(options)
-      options.each do |key, value|
-        next if key.to_s.end_with?('?')
-        next unless [true, false].include?(value)
-
-        define_method "#{key}?" do
-          actual_key = key.to_s.chomp('?')
-          self[actual_key]
+  class FakeApplicationSettings
+    prepend ApplicationSettingImplementation
+
+    def self.define_properties(settings)
+      settings.each do |key, value|
+        define_method key do
+          read_attribute(key)
+        end
+
+        if [true, false].include?(value)
+          define_method "#{key}?" do
+            read_attribute(key)
+          end
+        end
+
+        define_method "#{key}=" do |v|
+          @table[key.to_sym] = v
         end
       end
     end
 
-    def initialize(options = {})
-      super
+    def initialize(settings = {})
+      @table = settings.dup
 
-      FakeApplicationSettings.define_predicate_methods(options)
+      FakeApplicationSettings.define_properties(settings)
     end
 
-    alias_method :read_attribute, :[]
-    alias_method :has_attribute?, :[]
+    def read_attribute(key)
+      @table[key.to_sym]
+    end
+
+    def has_attribute?(key)
+      @table.key?(key.to_sym)
+    end
+
+    # Mimic behavior of OpenStruct, which absorbs any calls into undefined
+    # properties to return `nil`.
+    def method_missing(*)
+      nil
+    end
   end
 end
 

spec/lib/gitlab/fake_application_settings_spec.rb

@@ -6,27 +6,35 @@ RSpec.describe Gitlab::FakeApplicationSettings do
   let(:defaults) do
     described_class.defaults.merge(
       foobar: 'asdf',
-      'test?' => 123
+      'test?'.to_sym => 123,
+      # these two settings have no default in ApplicationSettingImplementation,
+      # so we need to set one here
+      domain_denylist: [],
+      archive_builds_in_seconds: nil
     )
   end
 
   let(:setting) { described_class.new(defaults) }
 
-  it 'wraps OpenStruct variables properly' do
+  it 'defines methods for default attributes' do
     expect(setting.password_authentication_enabled_for_web).to be_truthy
     expect(setting.signup_enabled).to be_truthy
     expect(setting.foobar).to eq('asdf')
   end
 
-  it 'defines predicate methods' do
+  it 'defines predicate methods for boolean properties' do
     expect(setting.password_authentication_enabled_for_web?).to be_truthy
     expect(setting.signup_enabled?).to be_truthy
   end
 
-  it 'does not define a predicate method' do
+  it 'does not define a predicate method for non-boolean properties' do
     expect(setting.foobar?).to be_nil
   end
 
+  it 'returns nil for undefined attributes' do
+    expect(setting.does_not_exist).to be_nil
+  end
+
   it 'does not override an existing predicate method' do
     expect(setting.test?).to eq(123)
   end