#428

backend/app/controllers/tag_versions_controller.rb

@@ -35,8 +35,18 @@ class TagVersionsController < ApplicationController
   private
 
   def serialise_versions rows, language_code
+    rows = rows.to_a
     user_ids = rows.map(&:created_by_user_id).compact.uniq
     users_by_id = User.where(id: user_ids).pluck(:id, :name).to_h
+    parent_ids = rows.flat_map do |row|
+      split_parent_tag_ids(row.parent_tag_ids) +
+          split_parent_tag_ids(row.attributes['prev_parent_tag_ids'])
+    end.uniq
+    parents_by_id = Tag.includes(
+        :tag_names, :materials, :deerjikists, :tag_descriptions,
+        parents: [:tag_names, :materials, :deerjikists, :tag_descriptions])
+        .where(id: parent_ids).index_by(&:id)
+    parent_json = parents_by_id.transform_values { TagRepr.base(_1, language_code:) }
 
     rows.map do |row|
       current_names = names_for_locale(row.names, language_code)
@@ -47,18 +57,10 @@ class TagVersionsController < ApplicationController
       cur_parent_tag_ids = split_parent_tag_ids(row.parent_tag_ids)
       prev_parent_tag_ids = split_parent_tag_ids(row.attributes['prev_parent_tag_ids'])
 
-      all_parent_tag_ids = (cur_parent_tag_ids | prev_parent_tag_ids)
-
-      tags_by_id =
-          Tag
-          .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions)
-          .where(id: all_parent_tag_ids)
-          .index_by(&:id)
-
       parent_tags =
           build_version_values(cur_parent_tag_ids, prev_parent_tag_ids, key: :tag_id)
           .map do |h|
-            { tag: TagRepr.base(tags_by_id[h[:tag_id]], language_code:),
+            { tag: parent_json.fetch(h[:tag_id]),
               type: h[:type] }
           end
 
@@ -66,6 +68,7 @@ class TagVersionsController < ApplicationController
         version_no: row.version_no,
         event_type: row.event_type,
         names: row.names,
+        name_changes: name_changes(row.names, row.attributes['prev_names']),
         name: { current: current_names['name'], prev: previous_names['name'] },
         category: { current: row.category, prev: row.attributes['prev_category'] },
         deprecated_at: { current: row.deprecated_at&.iso8601,
@@ -97,11 +100,44 @@ class TagVersionsController < ApplicationController
   def split_values(values) = values.to_s.split(/\s+/).reject(&:blank?)
 
   def names_for_locale names, language_code
+    values = snapshot_names(names)[language_code]
+    values.is_a?(Hash) ? values : {}
+  end
+
+  def snapshot_names names
     values = names.is_a?(String) ? JSON.parse(names) : names
-    values&.fetch(language_code, {}) || {}
+    values.is_a?(Hash) ? values : {}
   rescue JSON::ParserError
     {}
   end
 
+  def name_changes current_names, previous_names
+    current = snapshot_names(current_names)
+    previous = snapshot_names(previous_names)
+    (current.keys | previous.keys).sort.filter_map do |language|
+      current_name = language_snapshot(current[language])
+      previous_name = language_snapshot(previous[language])
+      next if current_name == previous_name
+
+      { language_code: language, current: current_name, prev: previous_name }
+    end
+  end
+
+  def language_snapshot entry
+    return nil unless entry.is_a?(Hash)
+
+    aliases = Array(entry['aliases']).filter_map do |value|
+      value = { 'name' => value } if value.is_a?(String)
+      next unless value.is_a?(Hash) && value['name'].is_a?(String)
+
+      { name: value['name'], script_code: value['script_code'] }
+    end
+    aliases = aliases.uniq.sort_by { [_1[:name], _1[:script_code].to_s] }
+    { name: entry['name'],
+      script_code: entry['script_code'],
+      auto_generated: entry['auto_generated'],
+      aliases: }
+  end
+
   def split_parent_tag_ids(values) = split_values(values).map(&:to_i)
 end

backend/app/controllers/wiki_pages_controller.rb

@@ -37,27 +37,18 @@ class WikiPagesController < ApplicationController
   def autolink_candidates
     locale = resolve_locale!
     language = params.fetch(:language, locale.language_code)
+    language = 'ja' if language == 'und'
     return head :not_found unless language.is_a?(String)
-    target_locale =
-        if language == 'und'
-          locale
-        else
-          Locale.where(language_code: language, deprecated_at: nil,
-                       code: Locale.codes.values).order(:code).first
-        end
-    return head :not_found unless target_locale
+    return head :not_found unless Language.exists?(code: language)
 
-    candidates = []
-    unless language == 'und'
-      names = TagDescription.joins(tag: :tag_names)
-          .where(language_code: language, tag_names: { language_code: language })
-          .pluck('tag_descriptions.tag_id', 'tag_descriptions.id', 'tag_names.name')
-      candidates = names.map do |tag_id, description_id, name|
-        WikiPageRepr.autolink_tag(target_locale, tag_id:, description_id:, name:)
-      end
+    names = TagDescription.joins(tag: :tag_names)
+        .where(language_code: locale.language_code, tag_names: { language_code: language })
+        .pluck('tag_descriptions.tag_id', 'tag_descriptions.id', 'tag_names.name')
+    candidates = names.map do |tag_id, description_id, name|
+      WikiPageRepr.autolink_tag(locale, tag_id:, description_id:, name:, language:)
     end
     candidates.concat(TagGroup.order(:id).pluck(:id, :name).map do |id, name|
-      WikiPageRepr.autolink_group(target_locale, id:, name:, language:)
+      WikiPageRepr.autolink_group(locale, id:, name:, language:)
     end)
     render json: candidates
   end

backend/app/models/locale.rb

@@ -1,8 +1,7 @@
 class Locale < ApplicationRecord
   attr_accessor :certified_tag_names_path
 
-  before_save :lock_tag_creation!, if: :will_generate_tag_names?
-  after_save :generate_tag_names!, if: :needs_tag_names?
+  around_save :save_with_tag_names!, if: :will_generate_tag_names?
 
   belongs_to :language, foreign_key: :language_code, primary_key: :code
   belongs_to :script, foreign_key: :script_code, primary_key: :code
@@ -17,11 +16,17 @@ class Locale < ApplicationRecord
     deprecated_at.nil? && (will_save_change_to_code? || will_save_change_to_deprecated_at?)
   end
 
-  def lock_tag_creation! = SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+  def save_with_tag_names!
+    completed = self.class.transaction(requires_new: true) do
+      SharedResourceIdAllocator.lock_space!(Tag, 'tags')
+      saved = yield
+      raise ActiveRecord::RecordNotSaved, 'Locale save was halted' unless saved
 
-  def needs_tag_names?
-    deprecated_at.nil? && (saved_change_to_code? || saved_change_to_deprecated_at?)
+      TagNameCompletion.for_locale!(self)
+      true
+    end
+    unless completed
+      raise ActiveRecord::RecordNotSaved, 'Locale tag completion was rolled back'
+    end
   end
-
-  def generate_tag_names! = TagNameCompletion.for_locale!(self)
 end

backend/app/models/tag.rb

@@ -111,6 +111,11 @@ class Tag < ApplicationRecord
   def has_deerjikists = deerjikists.loaded? ? deerjikists.any? : deerjikists.exists?
 
   def snapshot_aliases language_code
+    if tag_names.loaded?
+      return tag_names.select { _1.language_code == language_code && !_1.primary? }
+          .sort_by(&:name).map(&:name)
+    end
+
     tag_names.where(language_code:, primary_flg: false).order(:name).pluck(:name)
   end
 

backend/app/representations/wiki_page_repr.rb

@@ -27,13 +27,13 @@ module WikiPageRepr
     TagGroupRepr.tag_group(tag_group).merge('resource_type' => 'tag_group')
   end
 
-  def autolink_tag locale, tag_id:, description_id:, name:
+  def autolink_tag locale, tag_id:, description_id:, name:, language:
     { resource_type: 'tag_description',
       resource_id: description_id,
       tag_id:,
       tag_description_id: description_id,
       name:,
-      language_code: locale.language_code,
+      language_code: language,
       locale_code: locale.code_before_type_cast }
   end
 

backend/app/services/shared_resource_id_allocator.rb

@@ -13,7 +13,19 @@ class SharedResourceIdAllocator
           ['tag_description_versions', 'tag_id']],
       'wiki' => [['tag_group_versions', 'tag_group_id']] }.freeze
   LOCK_TIMEOUT = 10
-  TRANSACTION_LOCKS = ObjectSpace::WeakMap.new
+  CONNECTION_LOCKS = ObjectSpace::WeakMap.new
+
+  class LockCleanup
+    def initialize &finish
+      @finish = finish
+    end
+
+    def trigger_transactional_callbacks? = false
+    def before_committed! = nil
+    def committed!(**) = @finish.call(self)
+    def rolledback!(**) = @finish.call(self)
+  end
+  private_constant :LockCleanup
 
   def self.assign! record
     space, tables = SPACES.find { |_name, names| names.include?(record.class.table_name) }
@@ -42,14 +54,14 @@ class SharedResourceIdAllocator
     raise ArgumentError, 'Unknown shared ID resource' unless SPACES.key?(space)
 
     connection = record_class.connection
-    transaction = record_class.current_transaction
+    transaction = connection.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] ||= {}
+    held_locks = CONNECTION_LOCKS[connection] ||= {}
     return if held_locks.key?(name)
 
     quoted_name = connection.quote(name)
@@ -61,20 +73,41 @@ class SharedResourceIdAllocator
     end
     held_locks[name] = true
 
-    release = lambda do
-      held_locks.delete(name)
+    finish = lambda do |cleanup|
+      next unless held_locks.key?(name)
+
+      # Savepoint callbacks may run before the outer transaction ends.
+      # Keep one session lock and transfer its cleanup to the remaining parent.
+      parent = connection.current_transaction
+      if parent.open?
+        connection.add_transaction_record(cleanup)
+        next
+      end
       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
+      held_locks.delete(name)
+    rescue StandardError
+      # Never return a session with an untracked advisory lock to the pool.
+      begin
+        connection.disconnect!
+      ensure
+        held_locks.clear
+      end
+      raise
     end
     begin
-      transaction.after_commit(&release)
-      transaction.after_rollback(&release)
+      # Rails finalises remaining records in ensure if another callback raises.
+      connection.add_transaction_record(LockCleanup.new(&finish))
     rescue StandardError
-      release.call
+      begin
+        connection.disconnect!
+      ensure
+        held_locks.clear
+      end
       raise
     end
   end