From 11e6f732872c2316d65dfb381ac422b986d4bd53 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Mon, 12 Oct 2026 21:34:02 +0900 Subject: [PATCH] #424 --- .../app/controllers/materials_controller.rb | 5 +- .../app/controllers/nico_tags_controller.rb | 2 +- backend/app/controllers/posts_controller.rb | 6 +- .../controllers/tag_versions_controller.rb | 2 +- backend/app/controllers/tags_controller.rb | 16 +- backend/app/models/tag.rb | 9 +- backend/app/representations/tag_repr.rb | 15 +- backend/spec/models/tag_repr_spec.rb | 50 ++++++ backend/spec/models/tag_spec.rb | 41 +++++ backend/spec/requests/materials_spec.rb | 51 +++++++ backend/spec/requests/tags_spec.rb | 16 ++ ...shared_resource_id_allocator_mysql_spec.rb | 143 ++++++++++++++++++ .../shared_resource_id_allocator_spec.rb | 23 +++ frontend/scripts/help-assets.test.ts | 112 ++++++++++++++ frontend/src/lib/help.test.ts | 25 +++ frontend/src/lib/helpPrefetch.test.tsx | 24 +++ frontend/src/test/helpFixtures.tsx | 6 +- 17 files changed, 524 insertions(+), 22 deletions(-) create mode 100644 backend/spec/models/tag_repr_spec.rb create mode 100644 backend/spec/services/shared_resource_id_allocator_mysql_spec.rb create mode 100644 frontend/scripts/help-assets.test.ts diff --git a/backend/app/controllers/materials_controller.rb b/backend/app/controllers/materials_controller.rb index f94f511..7c16a1a 100644 --- a/backend/app/controllers/materials_controller.rb +++ b/backend/app/controllers/materials_controller.rb @@ -70,7 +70,10 @@ class MaterialsController < ApplicationController .find_by(id: params[:id]) return head :not_found unless material - wiki_page_body = material.tag&.tag_name('ja')&.wiki_page&.current_revision&.body + wiki_page_body = + TagDescription + .find_by(tag_id: material.tag_id, language_code: locale.language_code) + &.body render json: MaterialRepr.base(material, host: request.base_url, language_code: locale.language_code).merge(wiki_page_body:) diff --git a/backend/app/controllers/nico_tags_controller.rb b/backend/app/controllers/nico_tags_controller.rb index 64f74ab..effb0e9 100644 --- a/backend/app/controllers/nico_tags_controller.rb +++ b/backend/app/controllers/nico_tags_controller.rb @@ -26,7 +26,7 @@ class NicoTagsController < ApplicationController ExternalTag .joins("LEFT JOIN (#{ post_tag_max_sql }) post_tag_max " \ 'ON post_tag_max.external_tag_id = external_tags.id') - .includes(linked_tags: { tag_names: :wiki_page }) + .includes(linked_tags: [:tag_names, :tag_descriptions]) if name q = q.where(('external_tags.name LIKE ? ' + "OR CONCAT(external_tags.platform, ':', external_tags.name) LIKE ?"), diff --git a/backend/app/controllers/posts_controller.rb b/backend/app/controllers/posts_controller.rb index 3843bb7..d08e466 100644 --- a/backend/app/controllers/posts_controller.rb +++ b/backend/app/controllers/posts_controller.rb @@ -53,7 +53,7 @@ class PostsController < ApplicationController .reselect('posts.*', Arel.sql("#{ updated_at_all_sql } AS updated_at_all")) .preload(:external_tags, :uploaded_user, :parents, :children, post_tags: [:sections, { tag: [:deerjikists, :materials, - { tag_names: :wiki_page }] }]) + :tag_names, :tag_descriptions] }]) .with_attached_thumbnail q = q.where('posts.url LIKE ?', "%#{ url }%") if url @@ -111,7 +111,7 @@ class PostsController < ApplicationController filtered_posts(locale) .preload(:uploaded_user, :parents, :children, post_tags: [:sections, { tag: [:deerjikists, :materials, - { tag_names: :wiki_page }] }]) + :tag_names, :tag_descriptions] }]) .with_attached_thumbnail .order('RAND()') .first @@ -198,7 +198,7 @@ class PostsController < ApplicationController Post .includes(:uploaded_user, :parents, :children, post_tags: [:sections, { tag: [:deerjikists, :materials, - { tag_names: :wiki_page }] }]) + :tag_names, :tag_descriptions] }]) .with_attached_thumbnail .find_by(id: params[:id]) return head :not_found unless post diff --git a/backend/app/controllers/tag_versions_controller.rb b/backend/app/controllers/tag_versions_controller.rb index f45d042..9c05085 100644 --- a/backend/app/controllers/tag_versions_controller.rb +++ b/backend/app/controllers/tag_versions_controller.rb @@ -51,7 +51,7 @@ class TagVersionsController < ApplicationController tags_by_id = Tag - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .where(id: all_parent_tag_ids) .index_by(&:id) diff --git a/backend/app/controllers/tags_controller.rb b/backend/app/controllers/tags_controller.rb index 7919a24..d60d56d 100644 --- a/backend/app/controllers/tags_controller.rb +++ b/backend/app/controllers/tags_controller.rb @@ -175,7 +175,7 @@ class TagsController < ApplicationController tags_by_id = Tag .joins(:tag_names) - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .where(tag_names: { language_code: locale.language_code, primary_flg: true }, id: tag_ids) .index_by(&:id) @@ -238,7 +238,7 @@ class TagsController < ApplicationController base = Tag .joins(:tag_names) - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .where(tag_names: { language_code: locale.language_code, primary_flg: true }, deprecated_at: nil) base = base.where('tags.post_count > 0') if present_only @@ -282,7 +282,7 @@ class TagsController < ApplicationController tag = Tag .joins(:tag_names) - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .find_by(id: params[:id], tag_names: { language_code: locale.language_code, primary_flg: true }) return render json: TagRepr.base(tag, language_code: locale.language_code) if tag @@ -302,7 +302,7 @@ class TagsController < ApplicationController tag = Tag .joins(:tag_names) - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .find_by(tag_names: { name:, language_code: locale.language_code, primary_flg: true }) return render json: TagRepr.base(tag, language_code: locale.language_code) if tag @@ -320,7 +320,7 @@ class TagsController < ApplicationController tag = Tag.joins(:tag_names) - .includes(:tag_names, tag_names: :wiki_page) + .includes(:tag_names, :tag_descriptions, parents: :tag_descriptions) .find_by(id: params[:id], tag_names: { language_code: locale.language_code, primary_flg: true }) return head :not_found unless tag @@ -340,7 +340,7 @@ class TagsController < ApplicationController tag = Tag .joins(:tag_names) - .includes(:tag_names, tag_names: :wiki_page) + .includes(:tag_names, :tag_descriptions, parents: :tag_descriptions) .find_by(tag_names: { name:, language_code: locale.language_code, primary_flg: true }) return head :not_found unless tag @@ -359,7 +359,7 @@ class TagsController < ApplicationController tag = Tag .joins(:tag_names) - .includes(:tag_names, tag_names: :wiki_page) + .includes(:tag_names, :tag_descriptions, parents: :tag_descriptions) .find_by(id: params[:id], tag_names: { language_code: locale.language_code, primary_flg: true }) return head :not_found unless tag @@ -424,7 +424,7 @@ class TagsController < ApplicationController tag = Tag .joins(:tag_names) - .includes(:tag_names, :materials, tag_names: :wiki_page) + .includes(:tag_names, :materials, :tag_descriptions, parents: :tag_descriptions) .find_by(tag_names: { name:, language_code: locale.language_code, primary_flg: true }) return head :not_found unless tag diff --git a/backend/app/models/tag.rb b/backend/app/models/tag.rb index 8c8ab33..173a52a 100644 --- a/backend/app/models/tag.rb +++ b/backend/app/models/tag.rb @@ -53,6 +53,7 @@ class Tag < ApplicationRecord has_many :tag_versions has_many :tag_names + has_many :tag_descriptions attribute :version_no, :integer, default: 1 @@ -98,7 +99,13 @@ class Tag < ApplicationRecord def deprecated? = deprecated_at? - def has_wiki = wiki_page.present? + def has_description? language_code: + if tag_descriptions.loaded? + tag_descriptions.any? { _1.language_code == language_code } + else + tag_descriptions.exists?(language_code:) + end + end def material_id = materials.first&.id diff --git a/backend/app/representations/tag_repr.rb b/backend/app/representations/tag_repr.rb index 9e3a42e..a8f61d1 100644 --- a/backend/app/representations/tag_repr.rb +++ b/backend/app/representations/tag_repr.rb @@ -3,21 +3,28 @@ module TagRepr BASE = { only: [:id, :category, :post_count, :created_at, :updated_at, :deprecated_at], - methods: [:has_wiki, :material_id, :has_deerjikists] }.freeze + methods: [:material_id, :has_deerjikists] }.freeze module_function def base tag, language_code: 'ja' - tag.as_json(BASE).merge(name: name_for(tag, language_code), + tag.as_json(BASE).merge('has_wiki' => tag.has_description?(language_code:), + name: name_for(tag, language_code), aliases: tag.snapshot_aliases(language_code), parents: tag.parents.map { inline(_1, language_code:) }) end def inline tag, language_code: 'ja' - tag.as_json(BASE).merge(name: name_for(tag, language_code), aliases: [], parents: []) + tag.as_json(BASE).merge('has_wiki' => tag.has_description?(language_code:), + name: name_for(tag, language_code), aliases: [], parents: []) end - def many(tags, language_code: 'ja') = tags.map { |tag| base(tag, language_code:) } + def many tags, language_code: 'ja' + records = tags.to_a + ActiveRecord::Associations::Preloader.new( + records:, associations: [:tag_descriptions, { parents: :tag_descriptions }]).call + records.map { |tag| base(tag, language_code:) } + end def name_for tag, language_code tag.name(language_code) || (tag.system? ? tag.name('ja') : nil) diff --git a/backend/spec/models/tag_repr_spec.rb b/backend/spec/models/tag_repr_spec.rb new file mode 100644 index 0000000..2c3fd64 --- /dev/null +++ b/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 diff --git a/backend/spec/models/tag_spec.rb b/backend/spec/models/tag_spec.rb index 1d08b42..0cc9d8c 100644 --- a/backend/spec/models/tag_spec.rb +++ b/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 diff --git a/backend/spec/requests/materials_spec.rb b/backend/spec/requests/materials_spec.rb index 14b5d8a..b891faa 100644 --- a/backend/spec/requests/materials_spec.rb +++ b/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 diff --git a/backend/spec/requests/tags_spec.rb b/backend/spec/requests/tags_spec.rb index 0d0b75a..d30f5e4 100644 --- a/backend/spec/requests/tags_spec.rb +++ b/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) diff --git a/backend/spec/services/shared_resource_id_allocator_mysql_spec.rb b/backend/spec/services/shared_resource_id_allocator_mysql_spec.rb new file mode 100644 index 0000000..40f9559 --- /dev/null +++ b/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 diff --git a/backend/spec/services/shared_resource_id_allocator_spec.rb b/backend/spec/services/shared_resource_id_allocator_spec.rb index 431af2e..823fb4a 100644 --- a/backend/spec/services/shared_resource_id_allocator_spec.rb +++ b/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 diff --git a/frontend/scripts/help-assets.test.ts b/frontend/scripts/help-assets.test.ts new file mode 100644 index 0000000..1e49d2f --- /dev/null +++ b/frontend/scripts/help-assets.test.ts @@ -0,0 +1,112 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { generateHelpAssets } from './help-assets' + +import type { ArticleMeta, HelpChange, HelpSearchEntry, HelpTitleIndex } from '@/lib/help' + +vi.hoisted (() => { + vi.stubGlobal ('Uint8Array', Object.getPrototypeOf (new TextEncoder ().encode ('')).constructor) +}) +afterAll (() => vi.unstubAllGlobals ()) + + +const readAsset = (modules: Map, name: string): T => { + const source = modules.get (`virtual:help/${ name }`)! + const argument = source.slice ('export default JSON.parse('.length, -1) + return JSON.parse (JSON.parse (argument)) +} + + +describe ('Build-time Help assets', () => { + let root: string + + beforeEach (async () => { + root = await mkdtemp (path.join (tmpdir (), 'btrc-help-assets-')) + }) + + afterEach (async () => { + await rm (root, { recursive: true, force: true }) + }) + + const writeArticle = async ( + identifier: string, + editions: Record, + current = '2026-10-02.10', + ) => { + const articleRoot = path.join (root, identifier) + const editionRoot = path.join (articleRoot, 'editions', 'ja') + await mkdir (editionRoot, { recursive: true }) + await writeFile (path.join (articleRoot, 'article.yml'), [ + `identifier: ${ identifier }`, 'canonical_language: ja', 'title: stale YAML title', + 'languages:', ' ja:', ` current_edition: ${ current }`].join ('\n')) + await Promise.all (Object.entries (editions).map (([edition, source]) => + writeFile (path.join (editionRoot, `${ edition }.mdx`), source))) + } + + it ('generates current search and ordered published history from each edition H1', async () => { + await writeArticle ('guide', { + '2026-10-01': '# 旧ホーム\n\n過去本文', + '2026-10-02': '# ホーム\n\n最初の同日版', + '2026-10-02.2': '# ホーム\n\n第二版', + '2026-10-02.3': '# ホーム\n\n第三版', + '2026-10-02.10': '\uFEFF# 最新ホーム\r\n\r\n現在本文\r\n# 別見出し', + '2026-10-02.11': '# 未公開\n\n検索に含めない' }) + const { modules } = await generateHelpAssets (root) + const articles = readAsset> (modules, 'articles') + expect (articles.guide.languages.ja).toEqual ({ + currentEdition: '2026-10-02.10', publishedEditions: [ + '2026-10-01', '2026-10-02', '2026-10-02.2', '2026-10-02.3', '2026-10-02.10'] }) + expect (readAsset> (modules, 'search').ja).toEqual ([{ + identifier: 'guide', edition: '2026-10-02.10', title: '最新ホーム', + body: '\r\n現在本文\r\n# 別見出し' }]) + const changes = readAsset> (modules, 'changes').ja + expect (changes.map (row => [row.edition, row.title, row.pred, row.change, row.date])) + .toEqual ([ + ['2026-10-02.10', '最新ホーム', '2026-10-02.3', 'update', '2026-10-02'], + ['2026-10-02.3', 'ホーム', '2026-10-02.2', 'update', '2026-10-02'], + ['2026-10-02.2', 'ホーム', '2026-10-02', 'update', '2026-10-02'], + ['2026-10-02', 'ホーム', '2026-10-01', 'update', '2026-10-02'], + ['2026-10-01', '旧ホーム', null, 'new', '2026-10-01']]) + const titles = readAsset> (modules, 'titles').ja + expect (titles).toEqual ({ '旧ホーム': ['guide'], 'ホーム': ['guide'], + '最新ホーム': ['guide'] }) + expect (modules.get ('virtual:help/loaders')).not.toContain ('2026-10-02.11') + expect (modules.get ('virtual:help/loaders')).toContain ('2026-10-02.10.mdx?raw') + expect (modules.get ('virtual:help/search')).not.toContain ('import(') + }) + + it ('retains ambiguous title identifiers and normalises width and case', async () => { + await writeArticle ('alpha', { '2026-10-02': '# ＨＯＭＥ\n本文' }, '2026-10-02') + await writeArticle ('beta', { '2026-10-02': '# Home\n本文' }, '2026-10-02') + const { modules } = await generateHelpAssets (root) + expect (readAsset> (modules, 'titles').ja.home) + .toEqual (['alpha', 'beta']) + expect (readAsset> (modules, 'changes').ja + .map (row => row.identifier)).toEqual (['alpha', 'beta']) + }) + + it ('rejects missing current files during generation', async () => { + await writeArticle ('guide', { '2026-10-02': '# ホーム' }) + await expect (generateHelpAssets (root)).rejects.toThrow ('Missing current Help edition') + }) + + it.each (['本文\n# 遅い見出し', '# \n本文']) ( + 'rejects missing leading H1 at build time: %s', async source => { + await writeArticle ('guide', { '2026-10-02.10': source }) + await expect (generateHelpAssets (root)).rejects.toThrow ('Missing leading Help H1') + }) + + it ('rejects absent canonical metadata instead of generating incomplete assets', async () => { + await writeArticle ('guide', { '2026-10-02.10': '# ホーム' }) + await writeFile (path.join (root, 'guide', 'article.yml'), + 'identifier: guide\nlanguages:\n ja:\n current_edition: 2026-10-02.10') + await expect (generateHelpAssets (root)).rejects.toThrow ('Invalid Help metadata') + }) + + it ('rejects an empty Help checkout', async () => { + await expect (generateHelpAssets (root)).rejects.toThrow ('No Help articles found') + }) +}) diff --git a/frontend/src/lib/help.test.ts b/frontend/src/lib/help.test.ts index d747cb2..8de955b 100644 --- a/frontend/src/lib/help.test.ts +++ b/frontend/src/lib/help.test.ts @@ -60,4 +60,29 @@ describe ('Help file catalogue', () => { expect (await api.getHelpEdition ('guide', 'ja', '2026-10-02.11')).toBeNull () expect (await api.getHelpEditionSource ('guide', 'ja', '2026-10-02.11')).toBeNull () }) + + it ('compares raw H1 and body lines in the requested direction', async () => { + const { api } = createHelpFixture () + const forward = await api.getHelpDiff ('guide', 'ja', '2026-10-01', '2026-10-02.10') + expect (forward?.from.title).toBe ('旧ホーム') + expect (forward?.to.title).toBe ('ホーム第十版') + expect (forward?.lines).toEqual (expect.arrayContaining ([ + { type: 'removed', content: '# 旧ホーム\n' }, + { type: 'added', content: '# ホーム第十版\n' }, + { type: 'unchanged', content: '\n' }])) + const reverse = await api.getHelpDiff ('guide', 'ja', '2026-10-02.10', '2026-10-01') + expect (reverse?.lines).toEqual (expect.arrayContaining ([ + { type: 'added', content: '# 旧ホーム\n' }, + { type: 'removed', content: '# ホーム第十版\n' }])) + }) + + it.each ([['missing', 'ja', '2026-10-01', '2026-10-02.10'], + ['guide', 'ja', '2026-10-01', '2026-10-02.11'], + ['guide', 'en', '2026-10-01', '2026-10-02']]) ( + 'rejects invalid diff editions for %s/%s without reading sources', + async (id, language, from, to) => { + const { api, raw } = createHelpFixture () + expect (await api.getHelpDiff (id, language, from, to)).toBeNull () + Object.values (raw).forEach (load => expect (load).not.toHaveBeenCalled ()) + }) }) diff --git a/frontend/src/lib/helpPrefetch.test.tsx b/frontend/src/lib/helpPrefetch.test.tsx index 537175a..788b8e3 100644 --- a/frontend/src/lib/helpPrefetch.test.tsx +++ b/frontend/src/lib/helpPrefetch.test.tsx @@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import PrefetchLink from '@/components/PrefetchLink' import * as prefetchers from '@/lib/prefetchers' import HelpDetailPage from '@/pages/help/HelpDetailPage' +import HelpDiffPage from '@/pages/help/HelpDiffPage' import { helpFixture } from '@/test/helpFixtures' import { renderWithProviders } from '@/test/render' @@ -76,6 +77,29 @@ describe ('Help prefetch integration', () => { Object.values (helpFixture.modules).forEach (load => expect (load).not.toHaveBeenCalled ()) }) + it ('reuses two prefetched raw editions when displaying their diff', async () => { + const path = '/help/guide/diff?from=2026-10-01&to=2026-10-02.10' + const { queryClient } = renderWithProviders ( + <> + diff + + + }/> + + ) + await Promise.all ([prefetchers.prefetchForURL (queryClient, path), + prefetchers.prefetchForURL (queryClient, path)]) + fireEvent.click (screen.getByRole ('link', { name: 'diff' })) + expect (await screen.findByRole ('link', { name: '旧ホーム (2026-10-01)' })) + .toHaveAttribute ('href', '/help/guide?edition=2026-10-01') + expect (screen.getByText ('+ # ホーム第十版')).toBeInTheDocument () + Object.entries (helpFixture.raw).forEach (([name, load]) => { + const expected = /\/ja\/(2026-10-01|2026-10-02\.10)\.mdx$/.test (name) ? 1 : 0 + expect (load).toHaveBeenCalledTimes (expected) + }) + Object.values (helpFixture.modules).forEach (load => expect (load).not.toHaveBeenCalled ()) + }) + it.each (['intent', 'off']) ('respects %s intent settings and still prefetches clicks', async mode => { settings.linkPreload = mode diff --git a/frontend/src/test/helpFixtures.tsx b/frontend/src/test/helpFixtures.tsx index 5ae0ac7..5f560e6 100644 --- a/frontend/src/test/helpFixtures.tsx +++ b/frontend/src/test/helpFixtures.tsx @@ -4,7 +4,7 @@ import { vi } from 'vitest' import source from '@/lib/help.ts?raw' -import type { ReactElement } from 'react' +import type { ComponentType } from 'react' import type * as Help from '@/lib/help' @@ -38,11 +38,11 @@ export const createHelpFixture = () => { vi.fn (async () => `# ${title}\n\n${body}`)])) const moduleLoads = Object.fromEntries (published.map (([id, language, edition, title, body]) => [ `../../help/${id}/editions/${language}/${edition}.mdx`, - vi.fn (async (): Promise<{ default: () => ReactElement }> => ({ + vi.fn (async (): Promise<{ default: ComponentType }> => ({ default: () => createElement ('div', null, createElement ('h1', null, title), createElement ('p', null, body)) }))])) // Dynamic imports reuse a module namespace even when called more than once. - const caches: Record ReactElement }> }> = { } + const caches: Record }> = { } const modules = Object.fromEntries (Object.entries (moduleLoads).map (([path, load]) => { caches[path] = { } return [path, vi.fn (() => caches[path].promise ??= load ())]