From f37f28b962376843e7682dad15067edb131ecdd8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Mon, 18 Mar 2019 14:32:26 +0100
Subject: [PATCH] Fix race cond. in
ApplicationSettingImplementation.create_from_defaults
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: R茅my Coutable <remy@rymai.me>
---
app/models/application_setting.rb | 7 +++++++
db/fixtures/development/02_application_settings.rb | 10 ++++++++++
db/fixtures/development/02_settings.rb | 8 --------
db/fixtures/development/08_settings.rb | 7 -------
db/fixtures/production/001_application_settings.rb | 2 ++
spec/models/application_setting_spec.rb | 9 ++++-----
6 files changed, 23 insertions(+), 20 deletions(-)
create mode 100644 db/fixtures/development/02_application_settings.rb
delete mode 100644 db/fixtures/development/02_settings.rb
delete mode 100644 db/fixtures/development/08_settings.rb
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9e91e4ab4b9..7ec8505b33a 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -223,4 +223,11 @@ class ApplicationSetting < ApplicationRecord
reset_memoized_terms
end
after_commit :expire_performance_bar_allowed_user_ids_cache, if: -> { previous_changes.key?('performance_bar_allowed_group_id') }
+
+ def self.create_from_defaults
+ super
+ rescue ActiveRecord::RecordNotUnique
+ # We already have an ApplicationSetting record, so just return it.
+ current_without_cache
+ end
end
diff --git a/db/fixtures/development/02_application_settings.rb b/db/fixtures/development/02_application_settings.rb
new file mode 100644
index 00000000000..d604f0be3cd
--- /dev/null
+++ b/db/fixtures/development/02_application_settings.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+puts "Creating the default ApplicationSetting record.".color(:green)
+Gitlab::CurrentSettings.current_application_settings
+
+# Details https://gitlab.com/gitlab-org/gitlab-ce/issues/46241
+puts "Enable hashed storage for every new projects.".color(:green)
+ApplicationSetting.current_without_cache.update!(hashed_storage_enabled: true)
+
+print '.'
diff --git a/db/fixtures/development/02_settings.rb b/db/fixtures/development/02_settings.rb
deleted file mode 100644
index 3a4a5d436bf..00000000000
--- a/db/fixtures/development/02_settings.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-
-# Enable hashed storage, in development mode, for all projects by default.
-Gitlab::Seeder.quiet do
- ApplicationSetting.create_from_defaults unless ApplicationSetting.current_without_cache
- ApplicationSetting.current_without_cache.update!(hashed_storage_enabled: true)
- print '.'
-end
diff --git a/db/fixtures/development/08_settings.rb b/db/fixtures/development/08_settings.rb
deleted file mode 100644
index 141465c06cf..00000000000
--- a/db/fixtures/development/08_settings.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-# We want to enable hashed storage for every new project in development
-# Details https://gitlab.com/gitlab-org/gitlab-ce/issues/46241
-Gitlab::Seeder.quiet do
- ApplicationSetting.create_from_defaults unless ApplicationSetting.current_without_cache
- ApplicationSetting.current_without_cache.update!(hashed_storage_enabled: true)
- print '.'
-end
diff --git a/db/fixtures/production/001_application_settings.rb b/db/fixtures/production/001_application_settings.rb
index ab15717e9a9..cf647650142 100644
--- a/db/fixtures/production/001_application_settings.rb
+++ b/db/fixtures/production/001_application_settings.rb
@@ -1,2 +1,4 @@
+# frozen_string_literal: true
+
puts "Creating the default ApplicationSetting record.".color(:green)
Gitlab::CurrentSettings.current_application_settings
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index c5579dafb4a..c81572d739e 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -6,6 +6,7 @@ describe ApplicationSetting do
let(:setting) { described_class.create_from_defaults }
it { include(CacheableAttributes) }
+ it { include(ApplicationSettingImplementation) }
it { expect(described_class.current_without_cache).to eq(described_class.last) }
it { expect(setting).to be_valid }
@@ -286,12 +287,10 @@ describe ApplicationSetting do
end
context 'restrict creating duplicates' do
- before do
- described_class.create_from_defaults
- end
+ let!(:current_settings) { described_class.create_from_defaults }
- it 'raises an record creation violation if already created' do
- expect { described_class.create_from_defaults }.to raise_error(ActiveRecord::RecordNotUnique)
+ it 'returns the current settings' do
+ expect(described_class.create_from_defaults).to eq(current_settings)
end
end
--
2.30.9