aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/referentials_controller.rb4
-rw-r--r--app/errors/table_lock_timeout_error.rb1
-rw-r--r--app/models/referential.rb53
-rw-r--r--spec/features/lines_spec.rb20
-rw-r--r--spec/features/networks_spec.rb14
-rw-r--r--spec/features/users/user_delete_spec.rb2
-rw-r--r--spec/models/referential/referential_lock_during_creation_spec.rb198
-rw-r--r--spec/models/referential_spec.rb8
-rw-r--r--spec/rails_helper.rb5
-rw-r--r--spec/spec_helper.rb10
-rw-r--r--spec/support/referential.rb2
11 files changed, 268 insertions, 49 deletions
diff --git a/app/controllers/referentials_controller.rb b/app/controllers/referentials_controller.rb
index 6f398b7f5..83e3bc56a 100644
--- a/app/controllers/referentials_controller.rb
+++ b/app/controllers/referentials_controller.rb
@@ -8,7 +8,7 @@ class ReferentialsController < ChouetteController
def new
new! do
- build_referenial
+ build_referential
end
end
@@ -137,7 +137,7 @@ class ReferentialsController < ChouetteController
super
end
- def build_referenial
+ def build_referential
if params[:from]
source_referential = Referential.find(params[:from])
@referential = Referential.new_from(source_referential, current_functional_scope)
diff --git a/app/errors/table_lock_timeout_error.rb b/app/errors/table_lock_timeout_error.rb
new file mode 100644
index 000000000..102f3a4a0
--- /dev/null
+++ b/app/errors/table_lock_timeout_error.rb
@@ -0,0 +1 @@
+class TableLockTimeoutError < ActiveRecord::StatementInvalid; end
diff --git a/app/models/referential.rb b/app/models/referential.rb
index 8db009ebd..1cdda9e6a 100644
--- a/app/models/referential.rb
+++ b/app/models/referential.rb
@@ -13,17 +13,17 @@ class Referential < ActiveRecord::Base
validates_uniqueness_of :slug
- validates_format_of :slug, :with => %r{\A[a-z][0-9a-z_]+\Z}
- validates_format_of :prefix, :with => %r{\A[0-9a-zA-Z_]+\Z}
- validates_format_of :upper_corner, :with => %r{\A-?[0-9]+\.?[0-9]*\,-?[0-9]+\.?[0-9]*\Z}
- validates_format_of :lower_corner, :with => %r{\A-?[0-9]+\.?[0-9]*\,-?[0-9]+\.?[0-9]*\Z}
+ validates_format_of :slug, with: %r{\A[a-z][0-9a-z_]+\Z}
+ validates_format_of :prefix, with: %r{\A[0-9a-zA-Z_]+\Z}
+ validates_format_of :upper_corner, with: %r{\A-?[0-9]+\.?[0-9]*\,-?[0-9]+\.?[0-9]*\Z}
+ validates_format_of :lower_corner, with: %r{\A-?[0-9]+\.?[0-9]*\,-?[0-9]+\.?[0-9]*\Z}
validate :slug_excluded_values
attr_accessor :upper_corner
attr_accessor :lower_corner
has_one :user
- has_many :api_keys, :class_name => 'Api::V1::ApiKey', :dependent => :destroy
+ has_many :api_keys, class_name: 'Api::V1::ApiKey', dependent: :destroy
belongs_to :organisation
validates_presence_of :organisation
@@ -62,6 +62,18 @@ class Referential < ActiveRecord::Base
scope :order_by_validity_period, ->(dir) { joins(:metadatas).order("unnest(periodes) #{dir}") }
scope :order_by_lines, ->(dir) { joins(:metadatas).group("referentials.id").order("sum(array_length(referential_metadata.line_ids,1)) #{dir}") }
+ def save_with_table_lock_timeout(options = {})
+ save_without_table_lock_timeout(options)
+ rescue ActiveRecord::StatementInvalid => e
+ if e.message.include?('PG::LockNotAvailable')
+ raise TableLockTimeoutError.new(e)
+ else
+ raise
+ end
+ end
+
+ alias_method_chain :save, :table_lock_timeout
+
def lines
if metadatas.blank?
workbench ? workbench.lines : associated_lines
@@ -79,7 +91,7 @@ class Referential < ActiveRecord::Base
errors.add(:slug,I18n.t("referentials.errors.public_excluded"))
end
if slug == self.class.connection_config[:username]
- errors.add(:slug,I18n.t("referentials.errors.user_excluded", :user => slug))
+ errors.add(:slug,I18n.t("referentials.errors.user_excluded", user: slug))
end
end
end
@@ -146,7 +158,7 @@ class Referential < ActiveRecord::Base
end
def self.new_from(from, functional_scope)
- Referential.new(
+ Referential.new(
name: I18n.t("activerecord.copy", name: from.name),
slug: "#{from.slug}_clone",
prefix: from.prefix,
@@ -196,9 +208,15 @@ class Referential < ActiveRecord::Base
projection_type || ""
end
- before_validation :assign_line_and_stop_area_referential, :on => :create, if: :workbench
- before_validation :assign_slug, :on => :create
- before_validation :assign_prefix, :on => :create
+ before_validation :assign_line_and_stop_area_referential, on: :create, if: :workbench
+ before_validation :assign_slug, on: :create
+ before_validation :assign_prefix, on: :create
+
+ # Lock the `referentials` table to prevent duplicate referentials from being
+ # created simultaneously in separate transactions. This must be the last hook
+ # to minimise the duration of the lock.
+ before_save :lock_table, on: [:create, :update]
+
before_create :create_schema
after_create :clone_schema, if: :created_from
@@ -293,7 +311,11 @@ class Referential < ActiveRecord::Base
def create_schema
unless created_from
- Apartment::Tenant.create slug
+ report = Benchmark.measure do
+ Apartment::Tenant.create slug
+ end
+
+ Rails.logger.info("Schema create benchmark: '#{slug}'\t#{report}")
Rails.logger.error( "Schema migrations count for Referential #{slug} " + Referential.connection.select_value("select count(*) from #{slug}.schema_migrations;").to_s )
end
end
@@ -378,4 +400,13 @@ class Referential < ActiveRecord::Base
not metadatas_overlap?
end
+ private
+
+ def lock_table
+ # No explicit unlock is needed as it will be released at the end of the
+ # transaction.
+ ActiveRecord::Base.connection.execute(
+ 'LOCK referentials IN ACCESS EXCLUSIVE MODE'
+ )
+ end
end
diff --git a/spec/features/lines_spec.rb b/spec/features/lines_spec.rb
index f142b7da9..f176e34fe 100644
--- a/spec/features/lines_spec.rb
+++ b/spec/features/lines_spec.rb
@@ -61,22 +61,22 @@ describe "Lines", type: :feature do
# it "creates line and return to show" do
# visit line_referential_lines_path(line_referential)
# click_link "Ajouter une ligne"
- # fill_in "line_name", :with => "Line 1"
- # fill_in "Numéro d'enregistrement", :with => "1"
- # fill_in "Identifiant Neptune", :with => "chouette:test:Line:999"
+ # fill_in "line_name", with: "Line 1"
+ # fill_in "Numéro d'enregistrement", with: "1"
+ # fill_in "Identifiant Neptune", with: "chouette:test:Line:999"
# click_button("Créer ligne")
# expect(page).to have_content("Line 1")
# end
# end
# Fixme #1780
- # describe "new with group of line", :js => true do
+ # describe "new with group of line", truncation: true do
# it "creates line and return to show" do
# visit new_line_referential_line_path(line_referential)
- # fill_in "line_name", :with => "Line 1"
- # fill_in "Numéro d'enregistrement", :with => "1"
- # fill_in "Identifiant Neptune", :with => "test:Line:999"
- # fill_in_token_input('line_group_of_line_tokens', :with => "#{group_of_line.name}")
+ # fill_in "line_name", with: "Line 1"
+ # fill_in "Numéro d'enregistrement", with: "1"
+ # fill_in "Identifiant Neptune", with: "test:Line:999"
+ # fill_in_token_input('line_group_of_line_tokens', with: "#{group_of_line.name}")
# find_button("Créer ligne").trigger("click")
# expect(page).to have_text("Line 1")
# expect(page).to have_text("#{group_of_line.name}")
@@ -88,8 +88,8 @@ describe "Lines", type: :feature do
# it "edit line" do
# visit line_referential_line_path(line_referential, subject)
# click_link "Editer cette ligne"
- # fill_in "line_name", :with => "Line Modified"
- # fill_in "Numéro d'enregistrement", :with => "test-1"
+ # fill_in "line_name", with: "Line Modified"
+ # fill_in "Numéro d'enregistrement", with: "test-1"
# click_button("Editer ligne")
# expect(page).to have_content("Line Modified")
# end
diff --git a/spec/features/networks_spec.rb b/spec/features/networks_spec.rb
index 1a822e7c9..8586b2a16 100644
--- a/spec/features/networks_spec.rb
+++ b/spec/features/networks_spec.rb
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
require 'spec_helper'
-describe "Networks", :type => :feature do
+describe "Networks", type: :feature do
login_user
let(:line_referential) { create :line_referential, member: @user.organisation }
@@ -54,9 +54,9 @@ describe "Networks", :type => :feature do
# # allow(subject).to receive(:stop_areas).and_return(Array.new(2) { create(:stop_area) })
# visit line_referential_networks_path(line_referential)
# click_link "Ajouter un réseau"
- # fill_in "network_name", :with => "Network 1"
- # fill_in "Numéro d'enregistrement", :with => "test-1"
- # fill_in "Identifiant Neptune", :with => "chouette:test:GroupOfLine:1"
+ # fill_in "network_name", with: "Network 1"
+ # fill_in "Numéro d'enregistrement", with: "test-1"
+ # fill_in "Identifiant Neptune", with: "chouette:test:GroupOfLine:1"
# click_button("Créer réseau")
# expect(page).to have_content("Network 1")
# end
@@ -67,14 +67,14 @@ describe "Networks", :type => :feature do
# # allow(subject).to receive(:stop_areas).and_return(Array.new(2) { create(:stop_area) })
# visit line_referential_network_path(line_referential, subject)
# click_link "Editer ce réseau"
- # fill_in "network_name", :with => "Network Modified"
- # fill_in "Numéro d'enregistrement", :with => "test-1"
+ # fill_in "network_name", with: "Network Modified"
+ # fill_in "Numéro d'enregistrement", with: "test-1"
# click_button("Editer réseau")
# expect(page).to have_content("Network Modified")
# end
# end
- # describe "delete", :js => true do
+ # describe "delete", truncation: true do
# it "delete network and return to the list" do
# subject.stub(:stop_areas).and_return(Array.new(2) { create(:stop_area) })
# visit line_referential_network_path(line_referential, subject)
diff --git a/spec/features/users/user_delete_spec.rb b/spec/features/users/user_delete_spec.rb
index 48f4e35d1..8b2ffcbe5 100644
--- a/spec/features/users/user_delete_spec.rb
+++ b/spec/features/users/user_delete_spec.rb
@@ -7,7 +7,7 @@ Warden.test_mode!
# As a user
# I want to delete my user profile
# So I can close my account
-feature 'User delete', :devise, :js do
+feature 'User delete', :devise, :truncation do
after(:each) do
Warden.test_reset!
diff --git a/spec/models/referential/referential_lock_during_creation_spec.rb b/spec/models/referential/referential_lock_during_creation_spec.rb
new file mode 100644
index 000000000..717a96136
--- /dev/null
+++ b/spec/models/referential/referential_lock_during_creation_spec.rb
@@ -0,0 +1,198 @@
+RSpec.describe Referential, type: :model do
+ let (:workbench) { create(:workbench) }
+
+ def with_a_mutual_lock timeout: false
+ @with_a_mutual_lock = true
+ yield
+ thread_1 = Thread.new do
+ ActiveRecord::Base.transaction do
+ # seize LOCK
+ @locking_thread_content.try :call
+ sleep 10
+ # release LOCK
+ end
+ end
+
+ thread_2 = Thread.new do
+ sleep 5
+ ActiveRecord::Base.transaction do
+ if timeout
+ ActiveRecord::Base.connection.execute "SET lock_timeout = '1s'"
+ end
+ # waits for LOCK, (because of sleep 5)
+ @waiting_thread_content.try :call
+ # when lock was eventually obtained validation failed
+ end
+ end
+
+ thread_1.join
+ if timeout
+ expect do
+ thread_2.join
+ end.to raise_error(TableLockTimeoutError)
+ else
+ thread_2.join
+ end
+ @locking_thread_content = nil
+ @waiting_thread_content = nil
+ @with_a_mutual_lock = false
+ end
+
+ def locking_thread
+ raise "this method is intended to be called inside a `with_a_mutual_lock`" unless @with_a_mutual_lock
+ @locking_thread_content = yield
+ end
+
+ def waiting_thread
+ @waiting_thread_content = yield
+ end
+
+ context "when two identical Referentials are created, only one is saved" do
+ it "works synchronously" do
+ referential_1 = build(
+ :referential,
+ workbench: workbench,
+ organisation: workbench.organisation
+ )
+ referential_2 = referential_1.dup
+ referential_2.slug = "#{referential_1.slug}_different"
+
+ metadata_1 = build(:referential_metadata, referential: nil)
+ metadata_2 = metadata_1.dup
+
+ referential_1.metadatas << metadata_1
+ referential_2.metadatas << metadata_2
+
+ referential_1.save
+ referential_2.save
+
+ expect(referential_1).to be_persisted
+ expect(referential_2).not_to be_persisted
+ end
+
+ context truncation: true do
+ it "works asynchronously" do
+ skip('The truncation strategy breaks all subsequent tests (See #5295)')
+
+ begin
+ referential_1 = build(
+ :referential,
+ workbench: workbench,
+ organisation: workbench.organisation
+ )
+ referential_2 = referential_1.dup
+ referential_2.slug = "#{referential_1.slug}_different"
+ referential_3 = nil
+
+ metadata_1 = build(:referential_metadata, referential: nil)
+ metadata_2 = metadata_1.dup
+
+ referential_1.metadatas << metadata_1
+ referential_2.metadatas << metadata_2
+
+ with_a_mutual_lock do
+ locking_thread do
+ referential_1.save
+ end
+ waiting_thread do
+ referential_2.save
+ referential_3 = create(:referential)
+ end
+ end
+
+ expect(referential_1).to be_persisted
+ expect(referential_2).not_to be_persisted
+ expect(referential_3).to be_persisted
+ ensure
+ Apartment::Tenant.drop(referential_1.slug) if referential_1.persisted?
+ Apartment::Tenant.drop(referential_2.slug) if referential_2.persisted?
+
+ if referential_3.try(:persisted?)
+ Apartment::Tenant.drop(referential_3.slug)
+ end
+ end
+ end
+
+ it "works asynchronously when one is updated" do
+ skip('The truncation strategy breaks all subsequent tests (See #5295)')
+
+ begin
+ referential_1 = nil
+ referential_2 = nil
+
+ ActiveRecord::Base.transaction do
+ referential_1 = create(
+ :referential,
+ workbench: workbench,
+ organisation: workbench.organisation
+ )
+ referential_2 = referential_1.dup
+ referential_2.name = 'Another'
+ referential_2.slug = "#{referential_1.slug}_different"
+ referential_2.save!
+ end
+
+ metadata_2 = build(:referential_metadata, referential: nil)
+ metadata_1 = metadata_2.dup
+ with_a_mutual_lock do
+ locking_thread do
+ referential_1.metadatas_attributes = [metadata_1.attributes]
+ referential_1.save
+ end
+ waiting_thread do
+ referential_2.metadatas_attributes = [metadata_2.attributes]
+ referential_2.save
+ end
+ end
+
+ expect(referential_1).to be_valid
+ expect(referential_2).not_to be_valid
+ ensure
+ Apartment::Tenant.drop(referential_1.slug) if referential_1.persisted?
+ Apartment::Tenant.drop(referential_2.slug) if referential_2.persisted?
+ end
+ end
+ end
+ end
+
+ context "when two Referentials are created at the same time", truncation: true do
+ it "raises an error when the DB lock timeout is reached" do
+ skip('The truncation strategy breaks all subsequent tests (See #5295)')
+
+ begin
+ referential_1 = build(
+ :referential,
+ workbench: workbench,
+ organisation: workbench.organisation
+ )
+ referential_2 = referential_1.dup
+ referential_2.slug = "#{referential_1.slug}_different"
+ referential_3 = nil
+
+ metadata_1 = build(:referential_metadata, referential: nil)
+ metadata_2 = metadata_1.dup
+
+ referential_1.metadatas << metadata_1
+ referential_2.metadatas << metadata_2
+ with_a_mutual_lock timeout: true do
+ locking_thread do
+ referential_1.save
+ end
+ waiting_thread do
+ referential_2.save
+ referential_3 = create(:referential)
+ end
+ end
+
+ expect(referential_1).to be_persisted
+ ensure
+ Apartment::Tenant.drop(referential_1.slug) if referential_1.persisted?
+ Apartment::Tenant.drop(referential_2.slug) if referential_2.persisted?
+
+ if referential_3.try(:persisted?)
+ Apartment::Tenant.drop(referential_3.slug)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/models/referential_spec.rb b/spec/models/referential_spec.rb
index d0b1d6447..7816e7232 100644
--- a/spec/models/referential_spec.rb
+++ b/spec/models/referential_spec.rb
@@ -1,13 +1,6 @@
-require 'spec_helper'
-
describe Referential, :type => :model do
let(:ref) { create :workbench_referential, metadatas: [create(:referential_metadata)] }
- # it "create a rule_parameter_set" do
- # referential = create(:referential)
- # expect(referential.rule_parameter_sets.size).to eq(1)
- # end
-
it { should have_many(:metadatas) }
it { should belong_to(:workbench) }
it { should belong_to(:referential_suite) }
@@ -131,4 +124,5 @@ describe Referential, :type => :model do
end
end
end
+
end
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 47578405e..9eb628d95 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -32,11 +32,6 @@ RSpec.configure do |config|
# Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
config.fixture_path = "#{::Rails.root}/spec/fixtures"
- # If you're not using ActiveRecord, or you'd prefer not to run each of your
- # examples within a transaction, remove the following line or assign false
- # instead of true.
- config.use_transactional_fixtures = true
-
# RSpec Rails can automatically mix in different behaviours to your tests
# based on their file location, for example enabling you to call `get` and
# `post` in specs under `spec/controllers`.
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 9679952df..2d13d3802 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -64,11 +64,11 @@ RSpec.configure do |config|
Capybara.javascript_driver = :poltergeist
# :meta tests can be run seperately in case of doubt about the tests themselves
# they serve mainly as an explanataion of complicated tests (as e.g. PG information_schema introspection)
- config.filter_run_excluding :meta => true
- config.filter_run_excluding :js => true
- config.filter_run :wip => true
+ config.filter_run_excluding meta: true
+ config.filter_run_excluding truncation: true
+ config.filter_run_excluding wip: true
config.run_all_when_everything_filtered = true
- config.include TokenInputHelper, :type => :feature
+ config.include TokenInputHelper, type: :feature
# ## Mock Framework
#
@@ -93,7 +93,7 @@ RSpec.configure do |config|
# You can disable this behaviour by removing the line below, and instead
# explicitly tag your specs with their type, e.g.:
#
- # RSpec.describe UsersController, :type => :controller do
+ # RSpec.describe UsersController, type: :controller do
# # ...
# end
#
diff --git a/spec/support/referential.rb b/spec/support/referential.rb
index 7f56e5143..497ff47a8 100644
--- a/spec/support/referential.rb
+++ b/spec/support/referential.rb
@@ -77,7 +77,7 @@ RSpec.configure do |config|
first_referential.switch
end
- config.before(:each, :js => true) do
+ config.before(:each, truncation: true) do
DatabaseCleaner.strategy = :truncation, { except: %w[spatial_ref_sys] }
end
generated by cgit v1.2.3 (git 2.25.1) at 2025-11-01 22:38:47 +0000