ヘルプを Wiki から分離 (#424) (#430)

画面上でのテストは完了．
移行にす早さが求められるので，準備ができたらマージする．

Reviewed-on: #430
Co-authored-by: miteruzo <miteruzo@naver.com>

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

@@ -0,0 +1,88 @@
+require 'rails_helper'
+
+RSpec.describe SharedResourceIdAllocator do
+  let(:record) { build(:external_tag) }
+  let(:connection) { double('connection') }
+  let(:transaction) { double('transaction', open?: true) }
+  let(:callbacks) { { } }
+  let(:sql) { [] }
+
+  before do
+    config = double('config', database: 'test_shared_ids')
+    allow(connection).to receive_message_chain(:pool, :db_config).and_return(config)
+    allow(connection).to receive(:quote) { "'#{ _1 }'" }
+    allow(connection).to receive(:quote_table_name) { "`#{ _1 }`" }
+    allow(connection).to receive(:select_value) do |statement|
+      sql << statement
+      if statement.include?('GET_LOCK') || statement.include?('RELEASE_LOCK')
+        1
+      elsif statement.include?('`tags`')
+        8
+      else
+        3
+      end
+    end
+    allow(ExternalTag).to receive(:connection).and_return(connection)
+    allow(ExternalTag).to receive(:current_transaction).and_return(transaction)
+    allow(ExternalTag).to receive(:uncached).and_yield
+    [:after_commit, :after_rollback].each do |event|
+      allow(transaction).to receive(event) { |&callback| callbacks[event] = callback }
+    end
+  end
+
+  [:after_commit, :after_rollback].each do |event|
+    it "holds the shared lock until #{ event }" do
+      described_class.assign!(record)
+      expect(record.id).to eq(9)
+      expect(sql.grep(/RELEASE_LOCK/)).to be_empty
+      expect(sql.grep(/FOR UPDATE/).size).to eq(2)
+      callbacks.fetch(event).call
+      expect(sql.grep(/RELEASE_LOCK/).size).to eq(1)
+    end
+  end
+
+  it 'does not silently proceed after an acquisition timeout' do
+    allow(connection).to receive(:select_value).and_return(0)
+    expect { described_class.assign!(record) }.to raise_error(ActiveRecord::LockWaitTimeout)
+    expect(record.id).to be_nil
+    expect(callbacks).to be_empty
+  end
+
+  it 'releases a successfully acquired lock when allocation fails and rolls back' do
+    allow(connection).to receive(:select_value).with(/FOR UPDATE/).and_raise('query failed')
+    expect { described_class.assign!(record) }.to raise_error(RuntimeError, 'query failed')
+    callbacks.fetch(:after_rollback).call
+    expect(sql.grep(/RELEASE_LOCK/).size).to eq(1)
+  end
+
+  it 'preserves explicitly supplied historical IDs' do
+    record.id = 5
+    described_class.assign!(record)
+    expect(record.id).to eq(5)
+    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

backend/spec/services/wiki/commit_integrity_spec.rb

@@ -2,41 +2,42 @@ require 'digest'
 require 'rails_helper'
 
 RSpec.describe Wiki::Commit do
+  include WikiRecords
   let(:user) { create_member_user! }
 
   def create_page title:, body: 'initial body'
-    described_class.create_content!(
-        tag_name: create(:tag_name, name: title),
+    seed_wiki_page(
+        tag_name: create(:tag, primary_name: title).tag_name('ja'),
         body:,
         created_by_user: user,
         message: 'init')
   end
 
   describe '.create_content!' do
-    it 'creates page, revision, and version with normalised body' do
-      expect {
-        described_class.create_content!(
-            tag_name: create(:tag_name, name: 'commit_integrity_create'),
-            body: "a\r\nb\r\n\r\n",
-            created_by_user: user,
-            message: 'init')
-      }
-          .to change(WikiPage, :count).by(1)
+    it 'rejects standalone Help creation without adding page or history' do
+      name = create(:tag_name, name: 'Help:frozen_service')
+      counts = [WikiPage.count, WikiRevision.count, WikiVersion.count]
+      expect do
+        described_class.create_content!(tag_name: name, body: 'initial', created_by_user: user)
+      end.to raise_error(ActiveRecord::RecordInvalid)
+      expect([WikiPage.count, WikiRevision.count, WikiVersion.count]).to eq(counts)
+    end
+
+    it 'keeps the internal compatibility writer available' do
+      name = create(:tag_name, name: 'タググループ:compatibility_service')
+      page = nil
+      expect do
+        page = described_class.create_legacy_content!(
+            tag_name: name, body: 'initial', created_by_user: user)
+      end.to change(WikiPage, :count).by(1)
           .and change(WikiRevision, :count).by(1)
           .and change(WikiVersion, :count).by(1)
-
-      page = WikiPage.joins(:tag_name).find_by!(tag_names: { name: 'commit_integrity_create' })
-      revision = page.current_revision
-      version = page.wiki_versions.order(:version_no).last
-
-      expect(page.body).to eq("a\nb")
-      expect(revision.lines_count).to eq(2)
-      expect(version.body).to eq("a\nb")
-      expect(version.reason).to eq('init')
+      expect(page.current_revision.body).to eq('initial')
+      expect(page.wiki_versions.sole.body).to eq('initial')
     end
 
     it 'rejects body that becomes blank after normalisation' do
-      tag_name = create(:tag_name, name: 'commit_integrity_blank')
+      tag_name = create(:tag, primary_name: 'commit_integrity_blank').tag_name('ja')
 
       expect {
         described_class.create_content!(
@@ -52,6 +53,17 @@ RSpec.describe Wiki::Commit do
   end
 
   describe '.content!' do
+    it 'rejects standalone Help updates without modifying histories' do
+      page = seed_wiki_page(tag_name: create(:tag_name, name: 'Help:standalone'),
+                            body: 'before', created_by_user: user)
+      counts = [WikiRevision.count, WikiVersion.count]
+      expect do
+        described_class.content!(page:, body: 'after', created_user: user)
+      end.to raise_error(ActiveRecord::RecordInvalid)
+      expect(page.reload.body).to eq('before')
+      expect([WikiRevision.count, WikiVersion.count]).to eq(counts)
+    end
+
     it 'updates page body, revision, and version' do
       page = create_page(title: 'commit_integrity_update', body: 'before')
       current_id = page.current_revision.id
@@ -80,7 +92,7 @@ RSpec.describe Wiki::Commit do
       tag = create(:tag, primary_tag_name: tag_name, category: :general)
 
       page =
-          described_class.create_content!(
+          seed_wiki_page(
               tag_name:,
               body: 'before',
               created_by_user: user,

backend/spec/services/wiki/commit_spec.rb

@@ -1,12 +1,13 @@
 require 'rails_helper'
 
 RSpec.describe Wiki::Commit do
+  include WikiRecords
   let(:user) { create_member_user! }
 
   def create_page(title: 'commit_spec_page', body: 'initial body')
-    tag_name = create(:tag_name, name: title)
+    tag_name = create(:tag, primary_name: title).tag_name('ja')
 
-    Wiki::Commit.create_content!(
+    seed_wiki_page(
         tag_name:,
         body:,
         created_by_user: user,
@@ -84,7 +85,7 @@ RSpec.describe Wiki::Commit do
       tag = create(:tag, primary_tag_name: tag_name, category: :general)
 
       page =
-          described_class.create_content!(
+          seed_wiki_page(
               tag_name:,
               body: 'before',
               created_by_user: user,
@@ -111,7 +112,7 @@ RSpec.describe Wiki::Commit do
       tag = create(:tag, primary_tag_name: tag_name, category: :general)
 
       page =
-          described_class.create_content!(
+          seed_wiki_page(
               tag_name:,
               body: 'before',
               created_by_user: user,

backend/spec/services/wiki_version_recorder_spec.rb

@@ -1,10 +1,11 @@
 require 'rails_helper'
 
 RSpec.describe WikiVersionRecorder do
+  include WikiRecords
   let(:user) { create_member_user! }
 
   def create_page title:, body: 'body'
-    Wiki::Commit.create_content!(
+    seed_wiki_page(
         tag_name: create(:tag_name, name: title),
         body:,
         created_by_user: user,