diff options
| -rw-r--r-- | app/controllers/referentials_controller.rb | 6 | ||||
| -rw-r--r-- | app/models/referential.rb | 37 | ||||
| -rw-r--r-- | spec/features/lines_spec.rb | 20 | ||||
| -rw-r--r-- | spec/features/networks_spec.rb | 14 | ||||
| -rw-r--r-- | spec/features/users/user_delete_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/referential/referential_lock_during_creation_spec.rb | 80 | ||||
| -rw-r--r-- | spec/models/referential_spec.rb | 8 | ||||
| -rw-r--r-- | spec/spec_helper.rb | 10 | ||||
| -rw-r--r-- | spec/support/referential.rb | 2 |
9 files changed, 134 insertions, 45 deletions
diff --git a/app/controllers/referentials_controller.rb b/app/controllers/referentials_controller.rb index ee1236912..227651a59 100644 --- a/app/controllers/referentials_controller.rb +++ b/app/controllers/referentials_controller.rb @@ -8,13 +8,13 @@ class ReferentialsController < ChouetteController def new new! do - build_referenial + build_referential end end def create create! do |format| - build_referenial + build_referential if !!@referential.created_from_id format.html { redirect_to workbench_path(@referential.workbench) } @@ -132,7 +132,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/models/referential.rb b/app/models/referential.rb index ca20c639f..f89eafee8 100644 --- a/app/models/referential.rb +++ b/app/models/referential.rb @@ -12,17 +12,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 @@ -78,7 +78,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 @@ -141,7 +141,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, @@ -191,9 +191,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_validation :lock_table, on: :create + before_create :create_schema after_create :clone_schema, if: :created_from @@ -277,7 +283,7 @@ class Referential < ActiveRecord::Base def detect_overlapped_referentials self.class.where(id: overlapped_referential_ids).each do |referential| - errors.add :metadatas, I18n.t("referentials.errors.overlapped_referential", :referential => referential.name) + errors.add :metadatas, I18n.t("referentials.errors.overlapped_referential", referential: referential.name) end end @@ -371,4 +377,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 2a442bd2f..ecb90668c 100644 --- a/spec/features/lines_spec.rb +++ b/spec/features/lines_spec.rb @@ -60,22 +60,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}") @@ -87,8 +87,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 75070e7fa..19f17d900 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 } @@ -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..1e36bd976 --- /dev/null +++ b/spec/models/referential/referential_lock_during_creation_spec.rb @@ -0,0 +1,80 @@ +RSpec.describe Referential, type: :model do + + context "when two identical Referentials are created, only one is saved" do + let( :workbench ){ create :workbench } + + 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) + 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 + + it "works asynchronously", truncation: true do + 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) + metadata_2 = metadata_1.dup + + referential_1.metadatas << metadata_1 + referential_2.metadatas << metadata_2 + + thread_1 = Thread.new do + ActiveRecord::Base.transaction do + # seize LOCK + referential_1.save + sleep 10 + # release LOCK + end + end + + thread_2 = Thread.new do + sleep 5 + ActiveRecord::Base.transaction do + # waits for LOCK, (because of sleep 5) + referential_2.save + # when lock was eventually obtained validation failed + referential_3 = create(:referential) + end + end + + thread_1.join + thread_2.join + + 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 + 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/spec_helper.rb b/spec/spec_helper.rb index 9679952df..9fdca585e 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 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 b615491da..c0ae35779 100644 --- a/spec/support/referential.rb +++ b/spec/support/referential.rb @@ -78,7 +78,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 |