Merge branch 'dont-persist-application-settings-in-test-env' into 'master'

Don't persist application settings in test env

See merge request !8573

12 files changed, 141 insertions, 104 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 8fab77cda0a..e33a58d3771 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -13,6 +13,49 @@ 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
@@ -163,46 +206,7 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def self.create_from_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,
-    )
+    create(DEFAULTS)
   end
 
   def home_page_url_column_exist
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 2ff27e46d64..c79e17b57ee 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -9,7 +9,9 @@ module Gitlab
     end
 
     def ensure_application_settings!
-      if connect_to_db?
+      return fake_application_settings unless connect_to_db?
+
+      unless ENV['IN_MEMORY_APPLICATION_SETTINGS'] == 'true'
         begin
           settings = ::ApplicationSetting.current
         # In case Redis isn't running or the Redis UNIX socket file is not available
@@ -20,43 +22,23 @@ module Gitlab
         settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration?
       end
 
-      settings || fake_application_settings
+      settings || in_memory_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(
-        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,
-      )
+      OpenStruct.new(ApplicationSetting::DEFAULTS)
     end
 
     private
diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb
index 3f635be22ba..a55adc9b1c8 100644
--- a/lib/gitlab/github_import/project_creator.rb
+++ b/lib/gitlab/github_import/project_creator.rb
@@ -1,6 +1,8 @@
 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')
@@ -34,7 +36,7 @@ module Gitlab
       end
 
       def visibility_level
-        repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility
+        repo.private ? Gitlab::VisibilityLevel::PRIVATE : current_application_settings.default_project_visibility
       end
 
       #
diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb
index 56ecf2bb644..cfe18dd4b6c 100644
--- a/spec/controllers/health_check_controller_spec.rb
+++ b/spec/controllers/health_check_controller_spec.rb
@@ -1,10 +1,16 @@
 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 66044b44495..e8e080ce3e2 100644
--- a/spec/features/admin/admin_disables_git_access_protocol_spec.rb
+++ b/spec/features/admin/admin_disables_git_access_protocol_spec.rb
@@ -1,10 +1,13 @@
 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 dec2dedf2b5..f7e49a56deb 100644
--- a/spec/features/admin/admin_health_check_spec.rb
+++ b/spec/features/admin/admin_health_check_spec.rb
@@ -1,9 +1,11 @@
 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
 
@@ -12,11 +14,12 @@ feature "Admin Health Check", feature: true do
       visit admin_health_check_path
     end
 
-    it { page.has_text? 'Health Check' }
-    it { page.has_text? 'Health information can be retrieved' }
-
     it 'has a health check access token' do
+      page.has_text? 'Health Check'
+      page.has_text? 'Health information can be retrieved'
+
       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 d92c66b689d..f05fbe3d062 100644
--- a/spec/features/admin/admin_runners_spec.rb
+++ b/spec/features/admin/admin_runners_spec.rb
@@ -1,7 +1,10 @@
 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 47fa2f14307..de42ab81fac 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -1,7 +1,10 @@
 require 'spec_helper'
 
 feature 'Admin updates settings', feature: true do
-  before(:each) do
+  include StubENV
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
     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 661fb761809..855247de2ea 100644
--- a/spec/features/admin/admin_uses_repository_checks_spec.rb
+++ b/spec/features/admin/admin_uses_repository_checks_spec.rb
@@ -1,7 +1,12 @@
 require 'rails_helper'
 
 feature 'Admin uses repository checks', feature: true do
-  before { login_as :admin }
+  include StubENV
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+    login_as :admin
+  end
 
   scenario 'to trigger a single check' do
     project = create(:empty_project)
@@ -29,7 +34,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 004341ffd02..b01c4805a34 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -1,36 +1,64 @@
 require 'spec_helper'
 
 describe Gitlab::CurrentSettings do
+  include StubENV
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
   describe '#current_application_settings' do
-    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)
+    context 'with DB available' do
+      before do
+        allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(true)
+      end
 
-      expect(current_application_settings).to be_a(ApplicationSetting)
-    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 '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)
+      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 eq fake_application_settings
+        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
+
+        expect(current_application_settings).to be_a(ApplicationSetting)
+      end
     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
+    context 'with DB unavailable' do
+      before do
+        allow_any_instance_of(Gitlab::CurrentSettings).to receive(:connect_to_db?).and_return(false)
+      end
 
-      expect(current_application_settings).to be_a(ApplicationSetting)
+      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
     end
 
-    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
+    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)
 
-      expect(current_application_settings).to be_a(ApplicationSetting)
+        expect(current_application_settings).to be_a(ApplicationSetting)
+        expect(current_application_settings).not_to be_persisted
+      end
     end
   end
 end
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index a3798c8cd6c..91202244227 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -337,8 +337,7 @@ describe API::Internal, api: true  do
 
     context 'ssh access has been disabled' do
       before do
-        settings = ::ApplicationSetting.create_from_defaults
-        settings.update_attribute(:enabled_git_access_protocol, 'http')
+        stub_application_setting(enabled_git_access_protocol: 'http')
       end
 
       it 'rejects the SSH push' do
@@ -360,8 +359,7 @@ describe API::Internal, api: true  do
 
     context 'http access has been disabled' do
       before do
-        settings = ::ApplicationSetting.create_from_defaults
-        settings.update_attribute(:enabled_git_access_protocol, 'ssh')
+        stub_application_setting(enabled_git_access_protocol: 'ssh')
       end
 
       it 'rejects the HTTP push' do
@@ -383,8 +381,7 @@ describe API::Internal, api: true  do
 
     context 'web actions are always allowed' do
       it 'allows WEB push' do
-        settings = ::ApplicationSetting.create_from_defaults
-        settings.update_attribute(:enabled_git_access_protocol, 'ssh')
+        stub_application_setting(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 6ee3307512d..f78899134d5 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -2,6 +2,7 @@ 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'