#424

backend/spec/models/tag_repr_spec.rb

@@ -0,0 +1,50 @@
+require 'rails_helper'
+
+RSpec.describe TagRepr do
+  let(:tag) { create(:tag) }
+
+  def description_for record
+    TagDescription.create!(tag: record, language_code: 'ja', body: '', version_no: 1)
+  end
+
+  it 'keeps has_wiki boolean for the selected language in base and inline' do
+    description_for(tag)
+    [:base, :inline].each do |method|
+      expect(described_class.public_send(method, tag).as_json.fetch('has_wiki')).to be(true)
+      expect(described_class.public_send(method, tag, language_code: 'en')
+          .as_json.fetch('has_wiki')).to be(false)
+    end
+  end
+
+  it 'uses the same language for a tag and its parents' do
+    parent = create(:tag)
+    description_for(tag)
+    description_for(parent)
+    TagImplication.create!(tag:, parent_tag: parent)
+    ['ja', 'en'].each do |language_code|
+      row = described_class.base(tag, language_code:).as_json
+      expected = language_code == 'ja'
+      expect(row.fetch('has_wiki')).to be(expected)
+      expect(row.fetch('parents').sole.fetch('has_wiki')).to be(expected)
+    end
+  end
+
+  it 'preloads existence checks for many tags and their parents without Wiki reads' do
+    parent = create(:tag)
+    description_for(parent)
+    records = Array.new(3) do
+      create(:tag).tap { TagImplication.create!(tag: _1, parent_tag: parent) }
+    end
+    description_for(records.first)
+    queries = []
+    subscriber = ->(*args) { queries << args.last.fetch(:sql) }
+    rows = nil
+    ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+      rows = described_class.many(records).as_json
+    end
+    expect(rows.map { _1.fetch('has_wiki') }).to eq([true, false, false])
+    expect(rows.map { _1.fetch('parents').sole.fetch('has_wiki') }).to eq([true] * 3)
+    expect(queries.grep(/\bSELECT\b.*\btag_descriptions\b/im).size).to be <= 2
+    expect(queries.grep(/\bwiki_(pages|revisions|versions)\b/i)).to be_empty
+  end
+end

backend/spec/models/tag_spec.rb

@@ -1,6 +1,47 @@
 require 'rails_helper'
 
 RSpec.describe Tag, type: :model do
+  describe '#has_description?' do
+    let(:tag) { create(:tag) }
+
+    it 'checks the requested language and treats an empty description as present' do
+      TagDescription.create!(tag:, language_code: 'ja', body: '', version_no: 1)
+      expect(tag.has_description?(language_code: 'ja')).to be(true)
+      expect(tag.has_description?(language_code: 'en')).to be(false)
+    end
+
+    it 'returns false without a description' do
+      expect(tag.has_description?(language_code: 'ja')).to be(false)
+    end
+
+    it 'does not use a Wiki page as evidence of a description' do
+      user = create_member_user!
+      Wiki::Commit.create_legacy_content!(tag_name: tag.tag_name('ja'), body: 'legacy',
+                                          created_by_user: user)
+      expect(tag.has_description?(language_code: 'ja')).to be(false)
+    end
+
+    it 'reuses preloaded descriptions without reading Wiki or querying again' do
+      TagDescription.create!(tag:, language_code: 'ja', body: '', version_no: 1)
+      loaded = Tag.includes(:tag_descriptions).find(tag.id)
+      queries = []
+      subscriber = ->(*args) { queries << args.last.fetch(:sql) }
+      ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+        expect(loaded.has_description?(language_code: 'ja')).to be(true)
+        expect(loaded.has_description?(language_code: 'en')).to be(false)
+      end
+      expect(queries).to be_empty
+    end
+
+    it 'keeps language selection generic without enabling another Language record' do
+      tag.association(:tag_descriptions).target = [
+          TagDescription.new(tag:, language_code: 'en', body: '', version_no: 1)]
+      tag.association(:tag_descriptions).loaded!
+      expect(tag.has_description?(language_code: 'en')).to be(true)
+      expect(tag.has_description?(language_code: 'ja')).to be(false)
+    end
+  end
+
   describe 'external tag separation' do
     ['nico:reserved', 'NiCo:reserved'].each do |name|
       it "rejects the reserved prefix #{ name } for internal tags" do

backend/spec/requests/materials_spec.rb

@@ -178,6 +178,7 @@ RSpec.describe 'Materials API', type: :request do
   end
 
   describe 'GET /materials/:id' do
+    include WikiRecords
     let!(:tag) do
       create(:tag, primary_name: 'material_show', category: :material)
     end
@@ -205,6 +206,56 @@ RSpec.describe 'Materials API', type: :request do
       get '/materials/999999999'
       expect(response).to have_http_status(:not_found)
     end
+
+    def get_material_without_legacy_reads
+      queries = []
+      subscriber = lambda do |*args|
+        sql = args.last.fetch(:sql)
+        if sql.match?(/\bSELECT\b/i) && sql.match?(/\bwiki_(pages|revisions|versions)\b/i)
+          queries << sql
+        end
+      end
+      ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+        get "/materials/#{ material.id }"
+      end
+      expect(response).to have_http_status(:ok)
+      expect(queries).to be_empty
+    end
+
+    it 'reads the description even when the legacy revision has different content' do
+      page = seed_wiki_page(tag_name: tag.tag_name('ja'), body: 'legacy body',
+                            created_by_user: member_user)
+      TagDescription.find_by!(tag_id: tag.id, language_code: 'ja')
+          .update!(body: 'description body')
+      get_material_without_legacy_reads
+      expect(json.fetch('wiki_page_body')).to eq('description body')
+      expect(page.current_revision.body).to eq('legacy body')
+    end
+
+    it 'uses tag_id rather than description ID without a Wiki page or TagName' do
+      description = TagDescription.create!(id: tag.id + 1000, tag:, language_code: 'ja',
+                                           body: 'independent body', version_no: 1)
+      tag.tag_names.update_all(tag_id: nil)
+      get_material_without_legacy_reads
+      expect(description.id).not_to eq(tag.id)
+      expect(json.fetch('wiki_page_body')).to eq('independent body')
+      expect(json.fetch('tag').fetch('has_wiki')).to be(true)
+    end
+
+    it 'returns nil when only legacy Wiki content exists' do
+      seed_wiki_page(tag_name: tag.tag_name('ja'), body: 'legacy only',
+                     created_by_user: member_user)
+      TagDescription.where(tag_id: tag.id).delete_all
+      get_material_without_legacy_reads
+      expect(json.fetch('wiki_page_body')).to be_nil
+      expect(json.fetch('tag').fetch('has_wiki')).to be(false)
+    end
+
+    it 'returns nil for an untagged material' do
+      material.update!(tag: nil)
+      get_material_without_legacy_reads
+      expect(json.fetch('wiki_page_body')).to be_nil
+    end
   end
 
   describe 'POST /materials' do

backend/spec/requests/tags_spec.rb

@@ -38,6 +38,22 @@ RSpec.describe 'Tags API', type: :request do
   end
 
   describe 'GET /tags' do
+    it 'serialises descriptions and parents without loading legacy Wiki pages' do
+      TagDescription.create!(tag:, language_code: 'ja', body: '', version_no: 1)
+      TagDescription.create!(tag: tag2, language_code: 'ja', body: '', version_no: 1)
+      TagImplication.create!(tag:, parent_tag: tag2)
+      queries = []
+      subscriber = ->(*args) { queries << args.last.fetch(:sql) }
+      ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+        get '/tags', params: { name: 'spec_tag' }
+      end
+      expect(response).to have_http_status(:ok)
+      row = response_tags.sole
+      expect(row.fetch('has_wiki')).to be(true)
+      expect(row.fetch('parents').sole.fetch('has_wiki')).to be(true)
+      expect(queries.grep(/\bSELECT\b.*\bwiki_(pages|revisions|versions)\b/im)).to be_empty
+    end
+
     it 'includes legacy external JSON alongside an internal tag with the same id' do
       external = create(:external_tag, id: tag.id, name: 'spec_external', post_count: 3)
 

backend/spec/services/shared_resource_id_allocator_mysql_spec.rb

@@ -0,0 +1,143 @@
+require 'rails_helper'
+require 'timeout'
+
+RSpec.describe 'Shared resource IDs across MySQL connections', type: :model do
+  self.use_transactional_tests = false
+
+  before do
+    expect(Rails.env.test?).to be(true)
+    expect(ApplicationRecord.connection.adapter_name).to match(/mysql/i)
+    expect(ApplicationRecord.connection_pool.size).to be >= 3
+    @prefix = "shared_ids_#{ SecureRandom.hex(8) }"
+    @names = [0, 1].map { create(:tag_name, name: "#{ @prefix }_#{ _1 }") }
+    @tag = Tag.create!(tag_name_id: @names.first.id, category: :general)
+    @names.first.update!(tag_id: @tag.id)
+    @threads = []
+    @release = Queue.new
+  end
+
+  after do
+    @release << true if @release
+    begin
+      Array(@threads).each do |thread|
+        raise 'Shared ID worker did not finish' unless thread.join(15)
+      end
+    ensure
+      if @prefix
+        ExternalTag.where(name: "#{ @prefix }_external").delete_all
+        TagGroup.where(name: "#{ @prefix }_group").delete_all
+      end
+      TagDescription.where(tag_id: @tag.id).delete_all if @tag
+      if @names
+        ids = @names.map(&:id)
+        TagName.where(id: ids).update_all(tag_id: nil)
+        Tag.where(tag_name_id: ids).delete_all
+        TagName.where(id: ids).delete_all
+      end
+    end
+  end
+
+  def await_queue queue
+    Timeout.timeout(15) { queue.pop }
+  end
+
+  def worker &block
+    Thread.new do
+      Thread.current.report_on_exception = false
+      ApplicationRecord.connection_pool.with_connection(&block)
+    end.tap { @threads << _1 }
+  end
+
+  def create_resource kind
+    case kind
+    when :tag
+      Tag.create!(tag_name_id: @names.last.id, category: :general)
+    when :external
+      ExternalTag.create!(platform: :nico, name: "#{ @prefix }_external")
+    when :description
+      TagDescription.create!(tag: @tag, language_code: 'ja', body: '', version_no: 1)
+    when :group
+      TagGroup.create!(name: "#{ @prefix }_group", version_no: 1)
+    end
+  end
+
+  def hold_creation kind, rollback: false
+    ready = Queue.new
+    thread = worker do |connection|
+      id = nil
+      connection.transaction do
+        lock_name = nil
+        subscriber = lambda do |*args|
+          match = args.last.fetch(:sql).match(/GET_LOCK\('([^']+)'/)
+          lock_name = match[1] if match
+        end
+        ActiveSupport::Notifications.subscribed(subscriber, 'sql.active_record') do
+          id = create_resource(kind).id
+        end
+        ready << [id, connection.select_value('SELECT CONNECTION_ID()').to_i, lock_name]
+        await_queue(@release)
+        raise ActiveRecord::Rollback if rollback
+      end
+      id
+    end
+    [thread, await_queue(ready)]
+  end
+
+  def result thread
+    raise 'Shared ID worker did not finish' unless thread.join(15)
+    thread.value
+  end
+
+  { tags: [:external, :tag], wiki: [:description, :group] }.each do |space, kinds|
+    [false, true].each do |rollback|
+      it "serialises #{ space } inserts through #{ rollback ? 'rollback' : 'commit' }" do
+        tables = SharedResourceIdAllocator::SPACES.fetch(space.to_s)
+        maxima = tables.map do |table|
+          ApplicationRecord.connection.select_value("SELECT MAX(id) FROM #{ table }").to_i
+        end
+        first, (first_id, connection_id, lock_name) = hold_creation(kinds.first, rollback:)
+        expect(first_id).to eq(maxima.max + 1)
+        connection = ApplicationRecord.connection
+        quoted_lock = connection.quote(lock_name)
+        expect(connection.select_value("SELECT IS_USED_LOCK(#{ quoted_lock })").to_i)
+            .to eq(connection_id)
+
+        started = Queue.new
+        second = worker do |other|
+          started << other.select_value('SELECT CONNECTION_ID()').to_i
+          create_resource(kinds.last).id
+        end
+        expect(await_queue(started)).not_to eq(connection_id)
+        @release << true
+        expect(result(first)).to eq(first_id)
+        expect(result(second)).to eq(rollback ? first_id : first_id + 1)
+        expect(connection.select_value("SELECT IS_FREE_LOCK(#{ quoted_lock })").to_i).to eq(1)
+      end
+    end
+  end
+
+  it 'allows the other ID space to commit while the first space remains locked' do
+    first, (_id, connection_id, lock_name) = hold_creation(:external)
+    second = worker { create_resource(:group).id }
+    expect(result(second)).to be_positive
+    connection = ApplicationRecord.connection
+    quoted_lock = connection.quote(lock_name)
+    expect(connection.select_value("SELECT IS_USED_LOCK(#{ quoted_lock })").to_i)
+        .to eq(connection_id)
+    @release << true
+    result(first)
+  end
+
+  it 'raises on a real lock acquisition failure without inserting a record' do
+    first, (_id, _connection_id, lock_name) = hold_creation(:external, rollback: true)
+    stub_const('SharedResourceIdAllocator::LOCK_TIMEOUT', 0)
+    expect { create_resource(:external) }.to raise_error(ActiveRecord::LockWaitTimeout)
+    expect(ExternalTag.where(name: "#{ @prefix }_external")).not_to exist
+    @release << true
+    result(first)
+    connection = ApplicationRecord.connection
+    quoted_lock = connection.quote(lock_name)
+    expect(connection.select_value("SELECT IS_FREE_LOCK(#{ quoted_lock })").to_i)
+        .to eq(1)
+  end
+end

backend/spec/services/shared_resource_id_allocator_spec.rb

@@ -62,4 +62,27 @@ RSpec.describe SharedResourceIdAllocator do
     expect(sql.grep(/FOR UPDATE/)).to be_empty
     callbacks.fetch(:after_commit).call
   end
+
+  it 'uses the same named lock within each space and different locks between spaces' do
+    locks = [Tag, ExternalTag, TagDescription, TagGroup].map do |model|
+      allow(model).to receive(:connection).and_return(connection)
+      allow(model).to receive(:current_transaction).and_return(transaction)
+      allow(model).to receive(:uncached).and_yield
+      record = model.new(id: 50)
+      described_class.assign!(record)
+      statement = sql.grep(/GET_LOCK/).last
+      callbacks.fetch(:after_commit).call
+      statement
+    end
+    expect(locks[0]).to eq(locks[1])
+    expect(locks[2]).to eq(locks[3])
+    expect(locks[0]).not_to eq(locks[2])
+  end
+
+  it 'reports lock release failures' do
+    described_class.assign!(record)
+    allow(connection).to receive(:select_value).with(/RELEASE_LOCK/).and_return(nil)
+    expect { callbacks.fetch(:after_commit).call }
+        .to raise_error(ActiveRecord::ActiveRecordError, /lock release failed/)
+  end
 end