#428

backend/app/controllers/tags_controller.rb

@@ -368,6 +368,7 @@ class TagsController < ApplicationController
     return if performed?
 
     ApplicationRecord.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
       tag.lock!
 
       requested_keys = rows.map { |row| [row[:platform], row[:code]] }.uniq

backend/app/controllers/wiki_pages_controller.rb

@@ -214,6 +214,7 @@ class WikiPagesController < ApplicationController
     base_revision_id = params[:base_revision_id].presence
 
     ApplicationRecord.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
       tag.lock!
       old_title = tag_name.name
       if title != old_title

backend/app/models/locale.rb

@@ -1,4 +1,5 @@
 class Locale < ApplicationRecord
+  before_save :lock_tag_creation!, if: :will_generate_tag_names?
   after_save :generate_tag_names!, if: :needs_tag_names?
 
   belongs_to :language, foreign_key: :language_code, primary_key: :code
@@ -10,6 +11,14 @@ class Locale < ApplicationRecord
 
   private
 
+  def will_generate_tag_names?
+    deprecated_at.nil? && (will_save_change_to_code? || will_save_change_to_deprecated_at?)
+  end
+
+  def lock_tag_creation!
+    SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+  end
+
   def needs_tag_names?
     deprecated_at.nil? && (saved_change_to_code? || saved_change_to_deprecated_at?)
   end

backend/app/models/tag.rb

@@ -259,7 +259,8 @@ class Tag < ApplicationRecord
       name = name.to_s.strip
 
       ApplicationRecord.transaction do
-        tn = TagName.find_by(language_code:, name:)
+        SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+        tn = TagName.lock.find_by(language_code:, name:)
         next tn.tag if tn
 
         tag = Tag.create!(category:)

backend/app/services/shared_resource_id_allocator.rb

@@ -13,42 +13,14 @@ class SharedResourceIdAllocator
           ['tag_description_versions', 'tag_id']],
       'wiki' => [['tag_group_versions', 'tag_group_id']] }.freeze
   LOCK_TIMEOUT = 10
+  TRANSACTION_LOCKS = ObjectSpace::WeakMap.new
 
   def self.assign! record
     space, tables = SPACES.find { |_name, names| names.include?(record.class.table_name) }
     raise ArgumentError, 'Unknown shared ID resource' unless tables
 
     connection = record.class.connection
-    transaction = record.class.current_transaction
-    unless transaction.open?
-      raise ActiveRecord::ActiveRecordError, 'Shared ID requires a transaction'
-    end
-
-    database = connection.pool.db_config.database
-    name = "btrc:ids:#{ Digest::SHA256.hexdigest(database)[0, 16] }:#{ space }"
-    quoted_name = connection.quote(name)
-    acquired = record.class.uncached do
-      connection.select_value("SELECT GET_LOCK(#{ quoted_name }, #{ LOCK_TIMEOUT })")
-    end
-    unless acquired.to_i == 1
-      raise ActiveRecord::LockWaitTimeout, "Shared ID lock unavailable: #{ space }"
-    end
-
-    release = lambda do
-      result = record.class.uncached do
-        connection.select_value("SELECT RELEASE_LOCK(#{ quoted_name })")
-      end
-      unless result.to_i == 1
-        raise ActiveRecord::ActiveRecordError, "Shared ID lock release failed: #{ space }"
-      end
-    end
-    begin
-      transaction.after_commit(&release)
-      transaction.after_rollback(&release)
-    rescue StandardError
-      release.call
-      raise
-    end
+    lock_space!(record.class, space)
 
     # 明示-tic な Id. 指定は許可する．
     return if record.id.present?
@@ -66,6 +38,47 @@ class SharedResourceIdAllocator
     record.id = next_id
   end
 
+  def self.lock_space! record_class, space
+    raise ArgumentError, 'Unknown shared ID resource' unless SPACES.key?(space)
+
+    connection = record_class.connection
+    transaction = record_class.current_transaction
+    unless transaction.open?
+      raise ActiveRecord::ActiveRecordError, 'Shared ID requires a transaction'
+    end
+
+    database = connection.pool.db_config.database
+    name = "btrc:ids:#{ Digest::SHA256.hexdigest(database)[0, 16] }:#{ space }"
+    held_locks = TRANSACTION_LOCKS[transaction] ||= {}
+    return if held_locks.key?(name)
+
+    quoted_name = connection.quote(name)
+    acquired = record_class.uncached do
+      connection.select_value("SELECT GET_LOCK(#{ quoted_name }, #{ LOCK_TIMEOUT })")
+    end
+    unless acquired.to_i == 1
+      raise ActiveRecord::LockWaitTimeout, "Shared ID lock unavailable: #{ space }"
+    end
+    held_locks[name] = true
+
+    release = lambda do
+      held_locks.delete(name)
+      result = record_class.uncached do
+        connection.select_value("SELECT RELEASE_LOCK(#{ quoted_name })")
+      end
+      unless result.to_i == 1
+        raise ActiveRecord::ActiveRecordError, "Shared ID lock release failed: #{ space }"
+      end
+    end
+    begin
+      transaction.after_commit(&release)
+      transaction.after_rollback(&release)
+    rescue StandardError
+      release.call
+      raise
+    end
+  end
+
   def self.next_ids connection, tables
     expiry = connection.select_value('SELECT @@SESSION.information_schema_stats_expiry').to_i
     begin

backend/app/services/tag_description_operation.rb

@@ -5,6 +5,7 @@ class TagDescriptionOperation
 
   def self.create! locale, tag:, body:, created_by_user:, message: nil
     ApplicationRecord.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
       tag.lock!
       raise ActiveRecord::RecordNotUnique if TagDescription.exists?(
           tag_id: tag.id, language_code: locale.language_code)
@@ -19,6 +20,7 @@ class TagDescriptionOperation
 
   def self.update! locale, tag:, body:, updated_by_user:, message: nil, base_revision_id: nil
     ApplicationRecord.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
       tag.lock!
       description = TagDescription.lock.find_by!(
           tag_id: tag.id, language_code: locale.language_code)

backend/app/services/tag_name_completion.rb

@@ -1,31 +1,50 @@
 class TagNameCompletion
   class << self
     def for_locale! locale
-      named_tags =
-          TagName.where(language_code: locale.language_code, primary_flg: true)
-          .select(:tag_id)
-      Tag.where.not(id: named_tags).find_each do |tag|
-        for_tag!(tag, locales: [locale])
+      completed = Tag.transaction do
+        SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+        named_tags =
+            TagName.where(language_code: locale.language_code, primary_flg: true)
+            .select(:tag_id)
+        Tag.where.not(id: named_tags).lock.find_each(order: :asc) do |tag|
+          for_tag!(tag, locales: [locale])
+        end
+        true
+      end
+      unless completed
+        raise ActiveRecord::RecordNotSaved, 'Locale tag completion was rolled back'
       end
     end
 
     def for_tag! tag, locales: nil
-      locales ||= Locale.where(deprecated_at: nil).order(:language_code, :created_at, :script_code)
-      tag.with_lock do
-        primary_names = tag.tag_names.where(primary_flg: true).index_by(&:language_code)
-        missing_locales = locales.to_a.uniq(&:language_code).reject { |locale|
-          primary_names.key?(locale.language_code)
-        }
-        next if missing_locales.empty?
+      locales ||=
+          Locale.where(deprecated_at: nil)
+          .order(:language_code, :created_at, :script_code).lock
+      Tag.transaction do
+        SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+        completed = tag.with_lock do
+          primary_names = tag.tag_names.where(primary_flg: true).lock.index_by(&:language_code)
+          missing_locales = locales.to_a.uniq(&:language_code).reject { |locale|
+            primary_names.key?(locale.language_code)
+          }
+          next true if missing_locales.empty?
 
-        TagVersioning.ensure_snapshot!(tag, created_by_user: nil)
-        missing_locales.each do |locale|
-          source_language = locale.language_code == 'ja' ? 'ain' : 'ja'
-          name = TagName.generate_name(locale, tag, primary_names[source_language])
-          primary_names[locale.language_code] = create_name!(locale, tag, name)
+          TagVersioning.ensure_snapshot!(tag, created_by_user: nil)
+          missing_locales.each do |locale|
+            source_language = locale.language_code == 'ja' ? 'ain' : 'ja'
+            name = TagName.generate_name(locale, tag, primary_names[source_language])
+            primary_names[locale.language_code] = create_name!(locale, tag, name)
+          end
+          tag.association(:tag_names).reset
+          version = TagVersioning.record!(tag, event_type: :update, created_by_user: nil)
+          unless version&.persisted?
+            raise ActiveRecord::RecordNotSaved, 'Tag version was not persisted'
+          end
+          true
+        end
+        unless completed
+          raise ActiveRecord::RecordNotSaved, 'Tag completion was rolled back'
         end
-        tag.association(:tag_names).reset
-        TagVersioning.record!(tag, event_type: :update, created_by_user: nil)
       end
     end
 
@@ -36,18 +55,26 @@ class TagNameCompletion
     rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => error
       fallback = "Tag_##{ tag.id }"
       invalid_name =
-          error.is_a?(ActiveRecord::RecordNotUnique) ||
-          error.record.errors[:name].present?
+          if error.is_a?(ActiveRecord::RecordNotUnique)
+            TagName.where(language_code: locale.language_code, name:).lock.exists?
+          else
+            error.record.errors[:name].present? &&
+                error.record.errors.attribute_names.all? { _1 == :name }
+          end
       raise if name == fallback || !(invalid_name)
 
       persist_name!(locale, tag, fallback)
     end
 
     def persist_name! locale, tag, name
-      TagName.transaction(requires_new: true) do
+      tag_name = TagName.transaction(requires_new: true) do
         TagName.create!(tag:, name:, language_code: locale.language_code,
                         script_code: locale.script_code, primary_flg: true, auto_generated: true)
       end
+      unless tag_name&.persisted?
+        raise ActiveRecord::RecordNotSaved, 'Generated tag name was not persisted'
+      end
+      tag_name
     end
   end
 end

backend/app/services/tag_version_recorder.rb

@@ -1,6 +1,9 @@
 class TagVersionRecorder < VersionRecorder
   def self.record! tag:, event_type:, created_by_user:, **_options
-    new(tag:, event_type:, created_by_user:).record!
+    Tag.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+      new(tag:, event_type:, created_by_user:).record!
+    end
   end
 
   def initialize tag:, event_type:, created_by_user:
@@ -16,17 +19,20 @@ class TagVersionRecorder < VersionRecorder
   def snapshot_attributes
     names = snapshot_names
     compatibility_names = names['ja'] || names.fetch('ain')
+    parent_ids =
+        TagImplication.where(tag_id: @record.id).order(:parent_tag_id)
+        .lock.pluck(:parent_tag_id)
 
     { name: compatibility_names.fetch('name'),
       category: @record.category,
       deprecated_at: @record.deprecated_at,
       aliases: compatibility_names.fetch('aliases').map { _1.fetch('name') }.join(' '),
       names:,
-      parent_tag_ids: @record.snapshot_parent_tag_ids.join(' ') }
+      parent_tag_ids: parent_ids.join(' ') }
   end
 
   def snapshot_names
-    @record.tag_names.order(:language_code, :name).group_by(&:language_code)
+    @record.tag_names.order(:language_code, :name).lock.group_by(&:language_code)
         .transform_values do |tag_names|
       primary = tag_names.find(&:primary?)
       aliases = tag_names.reject(&:primary?).sort_by(&:name).map { |tag_name|

backend/app/services/tag_versioning.rb

@@ -4,14 +4,23 @@ class TagVersioning
   end
 
   def self.ensure_snapshot! tag, created_by_user:, language_code: 'ja'
-    return if tag.tag_versions.exists?
+    Tag.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+      locked_tag = Tag.unscoped.lock.find(tag.id)
+      next if locked_tag.tag_versions.lock.exists?
 
-    TagVersionRecorder.record!(tag:, event_type: :create, created_by_user:, language_code:)
+      TagVersionRecorder.record!(
+          tag: locked_tag, event_type: :create, created_by_user:, language_code:)
+    end
   end
 
   def self.record_tag_snapshot! tag, created_by_user:, language_code: 'ja'
-    event_type = tag.tag_versions.exists? ? :update : :create
-    record!(tag, event_type:, created_by_user:, language_code:)
+    Tag.transaction do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+      locked_tag = Tag.unscoped.lock.find(tag.id)
+      event_type = locked_tag.tag_versions.lock.exists? ? :update : :create
+      record!(locked_tag, event_type:, created_by_user:, language_code:)
+    end
   end
 
   def self.record_tag_snapshots! tags, created_by_user:, language_code: 'ja'

backend/app/services/version_recorder.rb

@@ -35,7 +35,7 @@ class VersionRecorder
 
   private
 
-  def latest_version = versions.order(version_no: :desc).first
+  def latest_version = versions.order(version_no: :desc).lock.first
 
   def versions = @record.public_send(version_association)