From baeaa97ef49558a303b0bd7b6cc9ce4ccfc13d1d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 21 Jan 2017 22:55:35 +0000 Subject: Revert "Merge branch 'dont-persist-application-settings-in-test-env' into 'master'" This reverts merge request !8573 --- app/models/application_setting.rb | 84 +++++++++++----------- lib/gitlab/current_settings.rb | 44 ++++++++---- lib/gitlab/github_import/project_creator.rb | 4 +- spec/controllers/health_check_controller_spec.rb | 6 -- .../admin_disables_git_access_protocol_spec.rb | 3 - spec/features/admin/admin_health_check_spec.rb | 9 +-- spec/features/admin/admin_runners_spec.rb | 3 - spec/features/admin/admin_settings_spec.rb | 5 +- .../admin/admin_uses_repository_checks_spec.rb | 9 +-- spec/lib/gitlab/current_settings_spec.rb | 68 ++++++------------ spec/requests/api/internal_spec.rb | 9 ++- spec/spec_helper.rb | 1 - 12 files changed, 104 insertions(+), 141 deletions(-) diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index e33a58d3771..8fab77cda0a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -13,49 +13,6 @@ class ApplicationSetting < ActiveRecord::Base [\r\n] # any number of newline characters }x - DEFAULTS_CE = { - after_sign_up_text: nil, - akismet_enabled: false, - container_registry_token_expire_delay: 5, - default_branch_protection: Settings.gitlab['default_branch_protection'], - default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], - default_projects_limit: Settings.gitlab['default_projects_limit'], - default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], - disabled_oauth_sign_in_sources: [], - domain_whitelist: Settings.gitlab['domain_whitelist'], - gravatar_enabled: Settings.gravatar['enabled'], - help_page_text: nil, - housekeeping_bitmaps_enabled: true, - housekeeping_enabled: true, - housekeeping_full_repack_period: 50, - housekeeping_gc_period: 200, - housekeeping_incremental_repack_period: 10, - import_sources: Gitlab::ImportSources.values, - koding_enabled: false, - koding_url: nil, - max_artifacts_size: Settings.artifacts['max_size'], - max_attachment_size: Settings.gitlab['max_attachment_size'], - plantuml_enabled: false, - plantuml_url: nil, - recaptcha_enabled: false, - repository_checks_enabled: true, - repository_storages: ['default'], - require_two_factor_authentication: false, - restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], - session_expire_delay: Settings.gitlab['session_expire_delay'], - send_user_confirmation_email: false, - shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], - shared_runners_text: nil, - sidekiq_throttling_enabled: false, - sign_in_text: nil, - signin_enabled: Settings.gitlab['signin_enabled'], - signup_enabled: Settings.gitlab['signup_enabled'], - two_factor_grace_period: 48, - user_default_external: false - } - - DEFAULTS = DEFAULTS_CE - serialize :restricted_visibility_levels serialize :import_sources serialize :disabled_oauth_sign_in_sources, Array @@ -206,7 +163,46 @@ class ApplicationSetting < ActiveRecord::Base end def self.create_from_defaults - create(DEFAULTS) + create( + default_projects_limit: Settings.gitlab['default_projects_limit'], + default_branch_protection: Settings.gitlab['default_branch_protection'], + signup_enabled: Settings.gitlab['signup_enabled'], + signin_enabled: Settings.gitlab['signin_enabled'], + gravatar_enabled: Settings.gravatar['enabled'], + sign_in_text: nil, + after_sign_up_text: nil, + help_page_text: nil, + shared_runners_text: nil, + restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], + max_attachment_size: Settings.gitlab['max_attachment_size'], + session_expire_delay: Settings.gitlab['session_expire_delay'], + default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + domain_whitelist: Settings.gitlab['domain_whitelist'], + import_sources: Gitlab::ImportSources.values, + shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], + max_artifacts_size: Settings.artifacts['max_size'], + require_two_factor_authentication: false, + two_factor_grace_period: 48, + recaptcha_enabled: false, + akismet_enabled: false, + koding_enabled: false, + koding_url: nil, + plantuml_enabled: false, + plantuml_url: nil, + repository_checks_enabled: true, + disabled_oauth_sign_in_sources: [], + send_user_confirmation_email: false, + container_registry_token_expire_delay: 5, + repository_storages: ['default'], + user_default_external: false, + sidekiq_throttling_enabled: false, + housekeeping_enabled: true, + housekeeping_bitmaps_enabled: true, + housekeeping_incremental_repack_period: 10, + housekeeping_full_repack_period: 50, + housekeeping_gc_period: 200, + ) end def home_page_url_column_exist diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index c79e17b57ee..2ff27e46d64 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -9,9 +9,7 @@ module Gitlab end def ensure_application_settings! - return fake_application_settings unless connect_to_db? - - unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true' + if connect_to_db? begin settings = ::ApplicationSetting.current # In case Redis isn't running or the Redis UNIX socket file is not available @@ -22,23 +20,43 @@ module Gitlab settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? end - settings || in_memory_application_settings + settings || fake_application_settings end def sidekiq_throttling_enabled? current_application_settings.sidekiq_throttling_enabled? end - def in_memory_application_settings - @in_memory_application_settings ||= ApplicationSetting.new(ApplicationSetting::DEFAULTS) - # In case migrations the application_settings table is not created yet, - # we fallback to a simple OpenStruct - rescue ActiveRecord::StatementInvalid - fake_application_settings - end - def fake_application_settings - OpenStruct.new(ApplicationSetting::DEFAULTS) + OpenStruct.new( + default_projects_limit: Settings.gitlab['default_projects_limit'], + default_branch_protection: Settings.gitlab['default_branch_protection'], + signup_enabled: Settings.gitlab['signup_enabled'], + signin_enabled: Settings.gitlab['signin_enabled'], + gravatar_enabled: Settings.gravatar['enabled'], + koding_enabled: false, + plantuml_enabled: false, + sign_in_text: nil, + after_sign_up_text: nil, + help_page_text: nil, + shared_runners_text: nil, + restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'], + max_attachment_size: Settings.gitlab['max_attachment_size'], + session_expire_delay: Settings.gitlab['session_expire_delay'], + default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], + default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'], + domain_whitelist: Settings.gitlab['domain_whitelist'], + import_sources: %w[gitea github bitbucket gitlab google_code fogbugz git gitlab_project], + shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'], + max_artifacts_size: Settings.artifacts['max_size'], + require_two_factor_authentication: false, + two_factor_grace_period: 48, + akismet_enabled: false, + repository_checks_enabled: true, + container_registry_token_expire_delay: 5, + user_default_external: false, + sidekiq_throttling_enabled: false, + ) end private diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index a55adc9b1c8..3f635be22ba 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -1,8 +1,6 @@ module Gitlab module GithubImport class ProjectCreator - include Gitlab::CurrentSettings - attr_reader :repo, :name, :namespace, :current_user, :session_data, :type def initialize(repo, name, namespace, current_user, session_data, type: 'github') @@ -36,7 +34,7 @@ module Gitlab end def visibility_level - repo.private ? Gitlab::VisibilityLevel::PRIVATE : current_application_settings.default_project_visibility + repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility end # diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index cfe18dd4b6c..56ecf2bb644 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -1,16 +1,10 @@ require 'spec_helper' describe HealthCheckController do - include StubENV - let(:token) { current_application_settings.health_check_access_token } let(:json_response) { JSON.parse(response.body) } let(:xml_response) { Hash.from_xml(response.body)['hash'] } - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - end - describe 'GET #index' do context 'when services are up but NO access token' do it 'returns a not found page' do diff --git a/spec/features/admin/admin_disables_git_access_protocol_spec.rb b/spec/features/admin/admin_disables_git_access_protocol_spec.rb index e8e080ce3e2..66044b44495 100644 --- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb +++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb @@ -1,13 +1,10 @@ require 'rails_helper' feature 'Admin disables Git access protocol', feature: true do - include StubENV - let(:project) { create(:empty_project, :empty_repo) } let(:admin) { create(:admin) } background do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as(admin) end diff --git a/spec/features/admin/admin_health_check_spec.rb b/spec/features/admin/admin_health_check_spec.rb index f7e49a56deb..dec2dedf2b5 100644 --- a/spec/features/admin/admin_health_check_spec.rb +++ b/spec/features/admin/admin_health_check_spec.rb @@ -1,11 +1,9 @@ require 'spec_helper' feature "Admin Health Check", feature: true do - include StubENV include WaitForAjax before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as :admin end @@ -14,12 +12,11 @@ feature "Admin Health Check", feature: true do visit admin_health_check_path end - it 'has a health check access token' do - page.has_text? 'Health Check' - page.has_text? 'Health information can be retrieved' + it { page.has_text? 'Health Check' } + it { page.has_text? 'Health information can be retrieved' } + it 'has a health check access token' do token = current_application_settings.health_check_access_token - expect(page).to have_content("Access token is #{token}") expect(page).to have_selector('#health-check-token', text: token) end diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index f05fbe3d062..d92c66b689d 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -1,10 +1,7 @@ require 'spec_helper' describe "Admin Runners" do - include StubENV - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') login_as :admin end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index de42ab81fac..47fa2f14307 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -1,10 +1,7 @@ require 'spec_helper' feature 'Admin updates settings', feature: true do - include StubENV - - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + before(:each) do login_as :admin visit admin_application_settings_path end diff --git a/spec/features/admin/admin_uses_repository_checks_spec.rb b/spec/features/admin/admin_uses_repository_checks_spec.rb index 855247de2ea..661fb761809 100644 --- a/spec/features/admin/admin_uses_repository_checks_spec.rb +++ b/spec/features/admin/admin_uses_repository_checks_spec.rb @@ -1,12 +1,7 @@ require 'rails_helper' feature 'Admin uses repository checks', feature: true do - include StubENV - - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - login_as :admin - end + before { login_as :admin } scenario 'to trigger a single check' do project = create(:empty_project) @@ -34,7 +29,7 @@ feature 'Admin uses repository checks', feature: true do scenario 'to clear all repository checks', js: true do visit admin_application_settings_path - + expect(RepositoryCheck::ClearWorker).to receive(:perform_async) click_link 'Clear all repository checks' diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb index b01c4805a34..004341ffd02 100644 --- a/spec/lib/gitlab/current_settings_spec.rb +++ b/spec/lib/gitlab/current_settings_spec.rb @@ -1,64 +1,36 @@ require 'spec_helper' describe Gitlab::CurrentSettings do - include StubENV - - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - end - describe '#current_application_settings' do - context 'with DB available' do - before do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) - end - - it 'attempts to use cached values first' do - expect(ApplicationSetting).to receive(:current) - expect(ApplicationSetting).not_to receive(:last) - - expect(current_application_settings).to be_a(ApplicationSetting) - end + it 'attempts to use cached values first' do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) + expect(ApplicationSetting).to receive(:current).and_return(::ApplicationSetting.create_from_defaults) + expect(ApplicationSetting).not_to receive(:last) - it 'falls back to DB if Redis returns an empty value' do - expect(ApplicationSetting).to receive(:last).and_call_original - - expect(current_application_settings).to be_a(ApplicationSetting) - end + expect(current_application_settings).to be_a(ApplicationSetting) + end - it 'falls back to DB if Redis fails' do - expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError) - expect(ApplicationSetting).to receive(:last).and_call_original + it 'does not attempt to connect to DB or Redis' do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false) + expect(ApplicationSetting).not_to receive(:current) + expect(ApplicationSetting).not_to receive(:last) - expect(current_application_settings).to be_a(ApplicationSetting) - end + expect(current_application_settings).to eq fake_application_settings end - context 'with DB unavailable' do - before do - allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false) - end + it 'falls back to DB if Redis returns an empty value' do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) + expect(ApplicationSetting).to receive(:last).and_call_original - it 'returns an in-memory ApplicationSetting object' do - expect(ApplicationSetting).not_to receive(:current) - expect(ApplicationSetting).not_to receive(:last) - - expect(current_application_settings).to be_a(OpenStruct) - end + expect(current_application_settings).to be_a(ApplicationSetting) end - context 'when ENV["IN_MEMORY_APPLICATION_SETTINGS"] is true' do - before do - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'true') - end - - it 'returns an in-memory ApplicationSetting object' do - expect(ApplicationSetting).not_to receive(:current) - expect(ApplicationSetting).not_to receive(:last) + it 'falls back to DB if Redis fails' do + allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true) + expect(ApplicationSetting).to receive(:current).and_raise(::Redis::BaseError) + expect(ApplicationSetting).to receive(:last).and_call_original - expect(current_application_settings).to be_a(ApplicationSetting) - expect(current_application_settings).not_to be_persisted - end + expect(current_application_settings).to be_a(ApplicationSetting) end end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 91202244227..a3798c8cd6c 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -337,7 +337,8 @@ describe API::Internal, api: true do context 'ssh access has been disabled' do before do - stub_application_setting(enabled_git_access_protocol: 'http') + settings = ::ApplicationSetting.create_from_defaults + settings.update_attribute(:enabled_git_access_protocol, 'http') end it 'rejects the SSH push' do @@ -359,7 +360,8 @@ describe API::Internal, api: true do context 'http access has been disabled' do before do - stub_application_setting(enabled_git_access_protocol: 'ssh') + settings = ::ApplicationSetting.create_from_defaults + settings.update_attribute(:enabled_git_access_protocol, 'ssh') end it 'rejects the HTTP push' do @@ -381,7 +383,8 @@ describe API::Internal, api: true do context 'web actions are always allowed' do it 'allows WEB push' do - stub_application_setting(enabled_git_access_protocol: 'ssh') + settings = ::ApplicationSetting.create_from_defaults + settings.update_attribute(:enabled_git_access_protocol, 'ssh') project.team << [user, :developer] push(key, project, 'web') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f78899134d5..6ee3307512d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,6 @@ require './spec/simplecov_env' SimpleCovEnv.start! ENV["RAILS_ENV"] ||= 'test' -ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' -- cgit v1.2.1