diff options
| -rw-r--r-- | lib/gitlab/current_settings.rb | 49 | ||||
| -rw-r--r-- | lib/gitlab/fake_application_settings.rb | 27 | ||||
| -rw-r--r-- | spec/factories/application_settings.rb | 4 | ||||
| -rw-r--r-- | spec/lib/gitlab/current_settings_spec.rb | 31 | ||||
| -rw-r--r-- | spec/lib/gitlab/fake_application_settings_spec.rb | 32 |
5 files changed, 121 insertions, 22 deletions
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 48735fd197d..284e6ad55a5 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -10,43 +10,49 @@ module Gitlab delegate :sidekiq_throttling_enabled?, to: :current_application_settings - def fake_application_settings - OpenStruct.new(::ApplicationSetting.defaults) + def fake_application_settings(defaults = ApplicationSetting.defaults) + FakeApplicationSettings.new(defaults) end private def ensure_application_settings! - unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' - settings = retrieve_settings_from_database? - end + return in_memory_application_settings if ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' - settings || in_memory_application_settings + cached_application_settings || uncached_application_settings end - def retrieve_settings_from_database? - settings = retrieve_settings_from_database_cache? - return settings if settings.present? - - return fake_application_settings unless connect_to_db? - + def cached_application_settings begin - db_settings = ::ApplicationSetting.current - # In case Redis isn't running or the Redis UNIX socket file is not available + ApplicationSetting.cached rescue ::Redis::BaseError, ::Errno::ENOENT - db_settings = ::ApplicationSetting.last + # In case Redis isn't running or the Redis UNIX socket file is not available end - db_settings || ::ApplicationSetting.create_from_defaults end - def retrieve_settings_from_database_cache? + def uncached_application_settings + return fake_application_settings unless connect_to_db? + + # This loads from the database into the cache, so handle Redis errors begin - settings = ApplicationSetting.cached + db_settings = ApplicationSetting.current rescue ::Redis::BaseError, ::Errno::ENOENT # In case Redis isn't running or the Redis UNIX socket file is not available - settings = nil end - settings + + # If there are pending migrations, it's possible there are columns that + # need to be added to the application settings. To prevent Rake tasks + # and other callers from failing, use any loaded settings and return + # defaults for missing columns. + if ActiveRecord::Migrator.needs_migration? + defaults = ApplicationSetting.defaults + defaults.merge!(db_settings.attributes.symbolize_keys) if db_settings.present? + return fake_application_settings(defaults) + end + + return db_settings if db_settings.present? + + ApplicationSetting.create_from_defaults || in_memory_application_settings end def in_memory_application_settings @@ -62,8 +68,7 @@ module Gitlab active_db_connection = ActiveRecord::Base.connection.active? rescue false active_db_connection && - ActiveRecord::Base.connection.table_exists?('application_settings') && - !ActiveRecord::Migrator.needs_migration? + ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError false end diff --git a/lib/gitlab/fake_application_settings.rb b/lib/gitlab/fake_application_settings.rb new file mode 100644 index 00000000000..bb14a8cd9e7 --- /dev/null +++ b/lib/gitlab/fake_application_settings.rb @@ -0,0 +1,27 @@ +# This class extends an OpenStruct object 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/schema.rb. +module Gitlab + class FakeApplicationSettings < OpenStruct + def initialize(options = {}) + super + + FakeApplicationSettings.define_predicate_methods(options) + end + + # 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] + end + end + end + end +end diff --git a/spec/factories/application_settings.rb b/spec/factories/application_settings.rb new file mode 100644 index 00000000000..aef65e724c2 --- /dev/null +++ b/spec/factories/application_settings.rb @@ -0,0 +1,4 @@ +FactoryGirl.define do + factory :application_setting do + end +end diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index fda39d78610..a566f24f6a6 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -32,6 +32,37 @@ describe Gitlab::CurrentSettings do expect(current_application_settings).to be_a(ApplicationSetting) end + + context 'with migrations pending' do + before do + expect(ActiveRecord::Migrator).to receive(:needs_migration?).and_return(true) + end + + it 'returns an in-memory ApplicationSetting object' do + settings = current_application_settings + + expect(settings).to be_a(OpenStruct) + expect(settings.sign_in_enabled?).to eq(settings.sign_in_enabled) + expect(settings.sign_up_enabled?).to eq(settings.sign_up_enabled) + end + + it 'uses the existing database settings and falls back to defaults' do + db_settings = create(:application_setting, + home_page_url: 'http://mydomain.com', + signup_enabled: false) + settings = current_application_settings + app_defaults = ApplicationSetting.last + + expect(settings).to be_a(OpenStruct) + expect(settings.home_page_url).to eq(db_settings.home_page_url) + expect(settings.signup_enabled?).to be_falsey + expect(settings.signup_enabled).to be_falsey + + # Check that unspecified values use the defaults + settings.reject! { |key, _| [:home_page_url, :signup_enabled].include? key } + settings.each { |key, _| expect(settings[key]).to eq(app_defaults[key]) } + end + end end context 'with DB unavailable' do diff --git a/spec/lib/gitlab/fake_application_settings_spec.rb b/spec/lib/gitlab/fake_application_settings_spec.rb new file mode 100644 index 00000000000..b793176d84a --- /dev/null +++ b/spec/lib/gitlab/fake_application_settings_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::FakeApplicationSettings do + let(:defaults) { { signin_enabled: false, foobar: 'asdf', signup_enabled: true, 'test?' => 123 } } + + subject { described_class.new(defaults) } + + it 'wraps OpenStruct variables properly' do + expect(subject.signin_enabled).to be_falsey + expect(subject.signup_enabled).to be_truthy + expect(subject.foobar).to eq('asdf') + end + + it 'defines predicate methods' do + expect(subject.signin_enabled?).to be_falsey + expect(subject.signup_enabled?).to be_truthy + end + + it 'predicate method changes when value is updated' do + subject.signin_enabled = true + + expect(subject.signin_enabled?).to be_truthy + end + + it 'does not define a predicate method' do + expect(subject.foobar?).to be_nil + end + + it 'does not override an existing predicate method' do + expect(subject.test?).to eq(123) + end +end |