From 5f66dc881775fb559f04d9b70572208287beab45 Mon Sep 17 00:00:00 2001 From: miteruzo Date: Wed, 11 Nov 2026 22:13:31 +0900 Subject: [PATCH] #445 --- .../controllers/post_versions_controller.rb | 11 +- backend/app/models/external_tag.rb | 25 +++-- backend/app/models/history_access.rb | 18 +--- backend/app/models/nico_tag_version.rb | 7 +- .../representations/counts_version_repr.rb | 5 +- backend/app/services/counts_candidates.rb | 6 +- backend/app/services/counts_population.rb | 2 +- .../services/counts_resource_definition.rb | 21 +++- backend/app/services/counts_state_query.rb | 4 +- backend/app/services/counts_table_query.rb | 2 + .../app/services/nico_tag_version_recorder.rb | 4 +- backend/spec/models/external_tag_spec.rb | 51 +++++++-- backend/spec/models/history_access_spec.rb | 6 +- backend/spec/models/shared_tag_ids_spec.rb | 7 +- backend/spec/models/version_record_spec.rb | 20 +++- backend/spec/requests/counts_table_spec.rb | 101 ++++++++++++++++-- backend/spec/requests/nico_tags_spec.rb | 22 ++-- .../requests/post_history_privacy_spec.rb | 2 +- backend/spec/services/counts_export_spec.rb | 49 +++++++++ .../nico_tag_version_recorder_spec.rb | 26 +++-- docs/counts-exports.md | 16 ++- 21 files changed, 309 insertions(+), 96 deletions(-) diff --git a/backend/app/controllers/post_versions_controller.rb b/backend/app/controllers/post_versions_controller.rb index cf07119..963fee3 100644 --- a/backend/app/controllers/post_versions_controller.rb +++ b/backend/app/controllers/post_versions_controller.rb @@ -100,18 +100,9 @@ class PostVersionsController < ApplicationController def external_tag_names_for snapshots ids = snapshots.flatten.filter_map { _1['external_tag_id'] }.uniq - names = ExternalTag.where(id: ids).pluck(:id, :platform, :name).to_h { |id, platform, name| + ExternalTag.where(id: ids).pluck(:id, :platform, :name).to_h { |id, platform, name| [id, "#{ platform }:#{ name }"] } - missing_ids = ids - names.keys - - HistoryAccess.visible(NicoTagVersion.all) - .where(tag_id: missing_ids) - .order(:tag_id, version_no: :desc) - .pluck(:tag_id, :name) - .each { |id, name| names[id] ||= name } - - names end def snapshot_tag_literals snapshots, tag_names, external_tag_names diff --git a/backend/app/models/external_tag.rb b/backend/app/models/external_tag.rb index a1f03a2..16f368b 100644 --- a/backend/app/models/external_tag.rb +++ b/backend/app/models/external_tag.rb @@ -1,8 +1,11 @@ class ExternalTag < ApplicationRecord before_create { SharedResourceIdAllocator.assign!(self) } + # Must run before dependent association callbacks. + before_destroy :prevent_destroy enum :platform, nico: 'nico' validates :platform, presence: true, inclusion: { in: ExternalTag.platforms.keys } + validate :immutable_identity, on: :update has_many :post_external_tags, dependent: :delete_all has_many :posts, through: :post_external_tags @@ -16,11 +19,21 @@ class ExternalTag < ApplicationRecord has_many :linked_tags, through: :nico_tag_relations, source: :tag - def snapshot_linked_tag_names language_code = 'ja' - linked_tags - .joins(:tag_names) - .where(tag_names: { language_code:, primary_flg: true }) - .order('tag_names.name') - .pluck('tag_names.name') + def snapshot_linked_tag_ids = linked_tags.distinct.order(:id).pluck(:id) + + def delete + raise ActiveRecord::ReadOnlyRecord, '外部タグは削除できません．' + end + + private + + def immutable_identity + errors.add(:platform, 'は変更できません．') if will_save_change_to_platform? + errors.add(:name, 'は変更できません．') if will_save_change_to_name? + end + + def prevent_destroy + errors.add(:base, '外部タグは削除できません．') + throw(:abort) end end diff --git a/backend/app/models/history_access.rb b/backend/app/models/history_access.rb index a0ef71f..a94e846 100644 --- a/backend/app/models/history_access.rb +++ b/backend/app/models/history_access.rb @@ -4,7 +4,7 @@ require 'digest' class HistoryAccess < ActiveSupport::CurrentAttributes attribute :user class Forbidden < StandardError; end - ARTIFACT_POLICY = 'counts-masked-summary-1'.freeze + ARTIFACT_POLICY = 'counts-masked-summary-2'.freeze KEYS = { 'post_versions' => ['post_id'], 'tag_versions' => ['tag_id'], 'nico_tag_versions' => ['tag_id'], @@ -106,21 +106,7 @@ class HistoryAccess < ActiveSupport::CurrentAttributes snapshot_reference(TagVersion, row), snapshot_reference(NicoTagVersion, row, external: true)] when 'nico_tag_versions' - # Legacy linked_tags snapshots contain names, not IDs. Match recorded names - # conservatively; never expose a private linked name through an external tag. - names = "JSON_MERGE_PRESERVE(JSON_ARRAY(private_tag.name), " + - "COALESCE(JSON_EXTRACT(private_tag.names, '$**.name'), JSON_ARRAY()))" - # DISTINCT prevents merging this independent name set into the correlated scan. - # Keep byte-distinct names even when the column collation considers them equal. - private_names = 'SELECT DISTINCT private_name.name, ' + - 'CAST(private_name.name AS BINARY) AS name_bytes FROM tag_versions private_tag ' + - "JOIN (#{ discarded_keys('tag_versions', 'tag_id') }) blocked " + - 'ON blocked.tag_id = private_tag.tag_id ' + - "JOIN JSON_TABLE(#{ names }, '$[*]' COLUMNS (name TEXT PATH '$')) private_name " + - "WHERE private_name.name <> ''" - ["NOT EXISTS (SELECT 1 FROM (#{ private_names }) private_name WHERE " + - "LOCATE(CONCAT(' ', private_name.name, ' '), " + - "CONCAT(' ', COALESCE(#{ row }.linked_tags, ''), ' ')) > 0)"] + [list_reference('tag_versions', 'tag_id', "#{ row }.linked_tag_ids")] when 'tag_versions' [list_reference('tag_versions', 'tag_id', "#{ row }.parent_tag_ids")] when 'tag_description_versions' diff --git a/backend/app/models/nico_tag_version.rb b/backend/app/models/nico_tag_version.rb index e83ecff..76358c9 100644 --- a/backend/app/models/nico_tag_version.rb +++ b/backend/app/models/nico_tag_version.rb @@ -3,5 +3,10 @@ class NicoTagVersion < ApplicationRecord belongs_to :external_tag, foreign_key: :tag_id, inverse_of: :nico_tag_versions - validates :name, presence: true + validates :event_type, inclusion: { in: ['create', 'update'] } + + def name + tag = external_tag + "#{ tag.platform }:#{ tag.name }" if tag + end end diff --git a/backend/app/representations/counts_version_repr.rb b/backend/app/representations/counts_version_repr.rb index 8073b7b..a1ef12b 100644 --- a/backend/app/representations/counts_version_repr.rb +++ b/backend/app/representations/counts_version_repr.rb @@ -139,6 +139,8 @@ class CountsVersionRepr result = {} table = model.table_name comparison = @inclusive ? '<=' : '<' + name = model == NicoTagVersion ? + CountsResourceDefinition.new('nico_tags').expression('name', table) : 'name' requests.each_slice(100) do |batch| queries = batch.map do |id, point| scope = HistoryAccess.visible(model.where(tag_id: id)) @@ -148,7 +150,8 @@ class CountsVersionRepr "AND newer.version_no > #{ table }.version_no)", point) quoted = model.connection.quote(point.iso8601(6)) names = model == TagVersion ? 'names' : 'JSON_OBJECT() AS names' - scope.select("tag_id, name, #{ names }, #{ quoted } AS reference_point").to_sql + scope.select("tag_id, #{ name } AS name, #{ names }, " + + "#{ quoted } AS reference_point").to_sql end next if queries.empty? diff --git a/backend/app/services/counts_candidates.rb b/backend/app/services/counts_candidates.rb index ded9d1d..9f5a548 100644 --- a/backend/app/services/counts_candidates.rb +++ b/backend/app/services/counts_candidates.rb @@ -28,16 +28,18 @@ class CountsCandidates [external, 'external', 'NULL', 'JSON_OBJECT()']] sources = sources.map do |population, kind, category, names| scope = population.scope.except(:select) + name = population.definition.expression('name') if @params['query'].present? pattern = "%#{ ApplicationRecord.sanitize_sql_like(@params['query']) }%" - scope = scope.where("(v.name LIKE ? OR CAST(v.tag_id AS CHAR) = ? OR EXISTS (" + + scope = scope.where("(#{ name } LIKE ? OR CAST(v.tag_id AS CHAR) = ? OR EXISTS (" + "SELECT 1 FROM JSON_TABLE(COALESCE(JSON_EXTRACT(#{ names }, '$**.name'), " + "JSON_ARRAY()), '$[*]' COLUMNS (name TEXT PATH '$')) candidate_name " + 'WHERE candidate_name.name LIKE ?))', pattern, @params['query'], pattern) end scope.select(Arel.sql( "CONCAT('#{ kind }:', v.tag_id) AS value, v.tag_id AS id, " + - "v.name AS name, #{ names } AS names, #{ category } AS category, '#{ kind }' AS kind")) + "#{ name } AS name, #{ names } AS names, " + + "#{ category } AS category, '#{ kind }' AS kind")) .to_sql end clauses = [] diff --git a/backend/app/services/counts_population.rb b/backend/app/services/counts_population.rb index 3776444..7f764cc 100644 --- a/backend/app/services/counts_population.rb +++ b/backend/app/services/counts_population.rb @@ -106,7 +106,7 @@ class CountsPopulation else raise ArgumentError unless definition.axes(history).include?(key) - scope = scope.where(definition.expression(key) => entry['value']) + scope = scope.where("#{ definition.expression(key) } <=> ?", entry['value']) end scope end diff --git a/backend/app/services/counts_resource_definition.rb b/backend/app/services/counts_resource_definition.rb index 448c3b2..82a37bc 100644 --- a/backend/app/services/counts_resource_definition.rb +++ b/backend/app/services/counts_resource_definition.rb @@ -105,12 +105,25 @@ class CountsResourceDefinition def time_column = 'created_at' def column(key) raise ArgumentError unless fields.key?(key) + # Keep the V1 Counts field name at the API boundary only. + return 'linked_tag_ids' if resource == 'nico_tags' && key == 'linked_tags' { 'resource_id' => keys.first, 'user' => 'created_by_user_id' }.fetch(key, key) end - def expression(key, row = 'v') = "#{ row }.#{ column(key) }" - def changes = fields.keys - COMMON.keys - ['updated_at', 'updated_by_user_id'] + def expression(key, row = 'v') + if resource == 'nico_tags' && key == 'name' + return "(SELECT CONCAT(external_tags.platform, ':', external_tags.name) " + + "FROM external_tags WHERE external_tags.id = #{ row }.tag_id)" + end + + "#{ row }.#{ column(key) }" + end + + def changes + columns = fields.keys - COMMON.keys - ['updated_at', 'updated_by_user_id'] + resource == 'nico_tags' ? columns - ['name'] : columns + end def axes(history) fields.reject { |_key, type| type == :json }.keys + (resource == 'posts' ? ['tags'] : []) + (history ? ['changed_field'] : []) @@ -118,7 +131,9 @@ class CountsResourceDefinition def metadata fields.map do |key, type| - { key:, type:, nullable: model.columns_hash.fetch(column(key)).null, + nullable = (resource == 'nico_tags' && key == 'name') || + model.columns_hash.fetch(column(key)).null + { key:, type:, nullable:, operators: OPERATORS.fetch(type, []) } end end diff --git a/backend/app/services/counts_state_query.rb b/backend/app/services/counts_state_query.rb index b141718..77abb8f 100644 --- a/backend/app/services/counts_state_query.rb +++ b/backend/app/services/counts_state_query.rb @@ -201,6 +201,8 @@ class CountsStateQuery end def listing + external_name = CountsResourceDefinition.new('nico_tags') + .expression('name', 'external_version') catalog = <<~SQL , tag_counts AS ( SELECT membership.kind, membership.tag_id, COUNT(*) AS post_count @@ -209,7 +211,7 @@ class CountsStateQuery SELECT 'internal' AS kind, tag_id, name, names, deprecated_at FROM tag_at WHERE event_type <> 'discard' UNION ALL - SELECT tc.kind, tc.tag_id, external_version.name, JSON_OBJECT() AS names, + SELECT tc.kind, tc.tag_id, #{ external_name } AS name, JSON_OBJECT() AS names, NULL AS deprecated_at FROM tag_counts tc LEFT JOIN tag_at internal_version diff --git a/backend/app/services/counts_table_query.rb b/backend/app/services/counts_table_query.rb index 92acf8c..fcb0ad1 100644 --- a/backend/app/services/counts_table_query.rb +++ b/backend/app/services/counts_table_query.rb @@ -182,6 +182,7 @@ class CountsTableQuery order = sort_sql { detail_expression(_1) } query = query.order(Arel.sql((order + ['v.id ASC']).join(', '))) .limit(limit).offset((page - 1) * limit) + query = query.includes(:external_tag) if @definition.resource == 'nico_tags' rows = query.to_a if ['posts', 'tags'].include?(@definition.resource) point = population.history ? population.at : @@ -494,6 +495,7 @@ class CountsTableQuery table = model.table_name scope = HistoryAccess.visible(model.where(tag_id: ids)).where('created_at <= ?', point) + scope = scope.includes(:external_tag) if model == NicoTagVersion bound = @connection.quote(point) if population.history && @definition.resource == 'posts' key = kind == 'internal' ? 'tag_id' : 'external_tag_id' diff --git a/backend/app/services/nico_tag_version_recorder.rb b/backend/app/services/nico_tag_version_recorder.rb index 149fb17..4fcb754 100644 --- a/backend/app/services/nico_tag_version_recorder.rb +++ b/backend/app/services/nico_tag_version_recorder.rb @@ -1,4 +1,5 @@ class NicoTagVersionRecorder < VersionRecorder + EVENT_TYPES = ['create', 'update'].freeze def self.record! external_tag:, event_type:, created_by_user: new(external_tag:, event_type:, created_by_user:).record! end @@ -14,8 +15,7 @@ class NicoTagVersionRecorder < VersionRecorder def record_key = :external_tag def snapshot_attributes - { name: "#{ @record.platform }:#{ @record.name }", - linked_tags: @record.snapshot_linked_tag_names.join(' ') } + { linked_tag_ids: @record.snapshot_linked_tag_ids.join(' ') } end def tracks_version_no_on_record? = false diff --git a/backend/spec/models/external_tag_spec.rb b/backend/spec/models/external_tag_spec.rb index 3d0bd86..774398c 100644 --- a/backend/spec/models/external_tag_spec.rb +++ b/backend/spec/models/external_tag_spec.rb @@ -11,28 +11,63 @@ RSpec.describe ExternalTag, type: :model do expect(external.reload.name).to eq('raw tag[]') end - it 'deletes post associations without deleting posts or version history' do + it 'does not allow a persisted external tag to change its platform or name' do + external = create(:external_tag, name: 'immutable') + + expect { external.update!(name: 'renamed') } + .to raise_error(ActiveRecord::RecordInvalid) + expect(external.errors[:name]).to include('は変更できません．') + + external.reload + external.platform = nil + expect { external.save! }.to raise_error(ActiveRecord::RecordInvalid) + expect(external.errors[:platform]).to include('は変更できません．') + + expect(external.reload).to have_attributes(platform: 'nico', name: 'immutable') + end + + it 'rejects delete, which bypasses destroy callbacks' do + external = create(:external_tag) + + expect { external.delete }.to raise_error(ActiveRecord::ReadOnlyRecord) + expect(external.reload).to be_persisted + end + + it 'increments the counter through post associations without changing identity' do + external = create(:external_tag, name: 'counter') + post = create(:post) + + expect { + PostExternalTag.create!(post:, external_tag: external) + }.to change { external.reload.post_count }.by(1) + + expect(external.reload).to have_attributes(platform: 'nico', name: 'counter') + end + + it 'does not destroy external tags or their post associations and history' do external = create(:external_tag) post = create(:post) PostExternalTag.create!(post:, external_tag: external) version = NicoTagVersionRecorder.record!( external_tag: external, event_type: :create, created_by_user: nil) - external.destroy! + expect { external.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed) - expect(PostExternalTag.where(external_tag_id: external.id)).to be_empty - expect(post.reload.external_tags).to be_empty + expect(external.reload).to be_persisted + expect(PostExternalTag.exists?(post:, external_tag: external)).to be(true) + expect(post.reload.external_tags).to contain_exactly(external) expect(version.reload.tag_id).to eq(external.id) end - it 'deletes external links without deleting linked internal tags' do + it 'does not destroy an external tag or its links to internal tags' do external = create(:external_tag) tag = create(:tag) - NicoTagRelation.create!(nico_tag: external, tag:) + relation = NicoTagRelation.create!(nico_tag: external, tag:) - external.destroy! + expect { external.destroy! }.to raise_error(ActiveRecord::RecordNotDestroyed) - expect(NicoTagRelation.where(nico_tag_id: external.id)).to be_empty + expect(external.reload).to be_persisted + expect(relation.reload).to be_persisted expect(Tag.exists?(tag.id)).to be(true) end end diff --git a/backend/spec/models/history_access_spec.rb b/backend/spec/models/history_access_spec.rb index c087c35..385e5cd 100644 --- a/backend/spec/models/history_access_spec.rb +++ b/backend/spec/models/history_access_spec.rb @@ -76,7 +76,7 @@ RSpec.describe 'Counts artifact policy and external references' do end end - it 'does not expose a private linked tag name through an external history snapshot' do + it 'does not expose a private linked tag ID through an external history snapshot' do tag = create(:tag, primary_name: 'private-linked-name') TagVersion.create!(tag:, version_no: 1, event_type: 'discard', name: 'private-linked-name', category: 'general', aliases: '', parent_tag_ids: '', created_at: Time.utc(2024), @@ -84,7 +84,7 @@ RSpec.describe 'Counts artifact policy and external references' do auto_generated: false, aliases: [] } }) external = create(:external_tag, name: 'public-external') row = NicoTagVersion.create!(external_tag: external, version_no: 1, event_type: 'create', - name: 'nico:public-external', linked_tags: 'private-linked-name', + linked_tag_ids: tag.id.to_s, created_at: Time.utc(2024)) post = create(:post) snapshot = PostVersion.create!(post:, version_no: 1, event_type: 'create', @@ -99,7 +99,7 @@ RSpec.describe 'Counts artifact policy and external references' do expect(HistoryAccess.visible(NicoTagVersion.where(id: row.id))).to exist end public_row = NicoTagVersion.create!(external_tag: external, version_no: 2, event_type: 'update', - name: 'nico:public-external', linked_tags: 'private-linked-name-extra', + linked_tag_ids: '', created_at: Time.utc(2024, 7)) HistoryAccess.set(user: nil) do expect(HistoryAccess.visible(NicoTagVersion.where(id: public_row.id))).to exist diff --git a/backend/spec/models/shared_tag_ids_spec.rb b/backend/spec/models/shared_tag_ids_spec.rb index 378eebc..4aae92e 100644 --- a/backend/spec/models/shared_tag_ids_spec.rb +++ b/backend/spec/models/shared_tag_ids_spec.rb @@ -10,23 +10,22 @@ RSpec.describe 'Shared Tag and ExternalTag IDs', type: :model do expect(next_tag.id).to be > external.id end - it 'does not reuse physically deleted IDs in either table' do + it 'does not reuse IDs after internal deletion while external tags remain' do tag = create(:tag) external = create(:external_tag) TagName.where(tag_id: tag.id).delete_all tag.destroy! - external.destroy! + expect(ExternalTag.exists?(external.id)).to be(true) expect(create(:tag).id).to be > external.id expect(create(:external_tag).id).to be > external.id end - it 'retains existing overlapping IDs without renumbering on update or insert' do + it 'retains overlapping IDs after a tag update and a new allocation' do tag = create(:tag) external = create(:external_tag) external.update_columns(id: tag.id) tag.update!(category: :meta) - external.update!(name: 'updated_external') before_tag = tag.reload.attributes before_external = external.reload.attributes diff --git a/backend/spec/models/version_record_spec.rb b/backend/spec/models/version_record_spec.rb index 561c19b..cbb9267 100644 --- a/backend/spec/models/version_record_spec.rb +++ b/backend/spec/models/version_record_spec.rb @@ -47,24 +47,34 @@ RSpec.describe VersionRecord, type: :model do external_tag: nico_tag, version_no: 1, event_type: 'create', - name: "nico:#{ nico_tag.name }", - linked_tags: '', + linked_tag_ids: '', created_at: Time.current, created_by_user: nil ) expect { - version.update!(name: 'nico:changed') + version.update!(linked_tag_ids: '123') }.to raise_error(ActiveRecord::ReadOnlyRecord) end + + it 'rejects discard and restore versions for external tags' do + [:discard, :restore].each do |event_type| + version = NicoTagVersion.new( + external_tag: nico_tag, version_no: 1, event_type:, + linked_tag_ids: '', created_at: Time.current) + + expect(version).not_to be_valid + expect(version.errors[:event_type]).not_to be_empty + end + end + it 'prevents NicoTagVersion destroy' do version = NicoTagVersion.create!( external_tag: nico_tag, version_no: 1, event_type: 'create', - name: "nico:#{ nico_tag.name }", - linked_tags: '', + linked_tag_ids: '', created_at: Time.current, created_by_user: nil ) diff --git a/backend/spec/requests/counts_table_spec.rb b/backend/spec/requests/counts_table_spec.rb index 5e13aac..517d9e1 100644 --- a/backend/spec/requests/counts_table_spec.rb +++ b/backend/spec/requests/counts_table_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'Counts table API', type: :request do names: { ja: { name: 'alpha', script_code: 'Jpan', auto_generated: false, aliases: [] } }, created_at: Time.utc(2023)) NicoTagVersion.create!(external_tag: external, version_no: 1, - event_type: 'create', name: 'beta', linked_tags: '', created_at: Time.utc(2023)) + event_type: 'create', linked_tag_ids: '', created_at: Time.utc(2023)) snapshots = [ { tag_id: tag.id, version_no: 1, name: 'alpha', category: 'general', sections: [] }, { external_tag_id: external.id }] @@ -50,6 +50,96 @@ RSpec.describe 'Counts table API', type: :request do def versions(result) = result.fetch('versions').map { _1.fetch('version_no') } + context 'nico link ID snapshots through the V1 field name' do + let(:base) do + super().merge(resource: 'nico_tags') + end + + before do + NicoTagVersion.create!(external_tag: external, version_no: 2, + event_type: 'update', linked_tag_ids: tag.id.to_s, + created_at: Time.utc(2024, 1, 1)) + NicoTagVersion.create!(external_tag: external, version_no: 3, + event_type: 'update', linked_tag_ids: '', + created_at: Time.utc(2024, 2, 1)) + end + + it 'returns ID values under linked_tags without exposing the physical column key' do + result = query + expect(versions(result)).to eq([1, 2, 3]) + expect(result['versions'].map { _1.fetch('linked_tags') }).to eq(['', tag.id.to_s, '']) + result['versions'].each do |row| + expect(row).not_to have_key('linked_tag_ids') + expect(row.fetch('name')).to eq('nico:beta') + end + end + + it 'advertises the legacy field, label and default columns only' do + get '/counts/definitions/nico_tags', params: { locale: 'ja' } + expect(response).to have_http_status(:ok) + body = response.parsed_body + ['fields', 'filter_fields', 'output_fields'].each do |collection| + keys = body.fetch(collection).map { _1.fetch('key') } + expect(keys).to include('linked_tags') + expect(keys).not_to include('linked_tag_ids') + end + field = body.fetch('output_fields').find { _1['key'] == 'linked_tags' } + expect(field.fetch('label')).to eq('counts:attributes.linked_tags') + body.fetch('axes').each_value do |keys| + expect(keys).to include('linked_tags') + expect(keys).not_to include('linked_tag_ids') + end + body.fetch('defaults').select { _1['mode'] == 'details' }.each do |entry| + settings = entry.fetch('params').to_h { [_1.fetch('key'), _1.fetch('value')] } + expect(settings.fetch('columns').split).to include('name', 'linked_tags') + expect(settings.fetch('columns').split).not_to include('linked_tag_ids') + end + end + + it 'filters IDs and legacy drills and compares link changes through linked_tags' do + predicate = { conditions: '', 'conditions.0.field' => 'linked_tags', + 'conditions.0.operator' => 'eq', 'conditions.0.value' => tag.id.to_s } + expect(versions(query(predicate))).to eq([2]) + expect(versions(query(axis('linked_tags', tag.id.to_s, root: 'drill')))).to eq([2]) + expect(versions(query(axis('changed_field', 'linked_tags')))).to eq([2, 3]) + expect(versions(query(predicate.merge('conditions.0.value' => 'alpha')))).to be_empty + end + + it 'sorts ID snapshots through the public field in both directions' do + expect(versions(query(sort: 'linked_tags:asc version_no:asc'))).to eq([1, 3, 2]) + expect(versions(query(sort: 'linked_tags:desc version_no:asc'))).to eq([2, 1, 3]) + end + + it 'restores historical link IDs before applying a latest snapshot filter' do + predicate = { definition: 'latest', at: '2024-01-15T00:00:00Z', + conditions: '', 'conditions.0.field' => 'linked_tags', + 'conditions.0.operator' => 'eq', 'conditions.0.value' => tag.id.to_s } + expect(versions(query(predicate))).to eq([2]) + expect(versions(query(predicate.except(:at)))).to be_empty + end + + it 'groups by linked_tags ID values and accepts the resulting drill condition' do + sign_in_as(create(:user, :admin)) + result = query(mode: 'summary', rows: 'linked_tags', cols: '', sort: '') + expect(result.fetch('cells')).to match_array([ + { 'values' => [''], 'count' => 2 }, + { 'values' => [tag.id.to_s], 'count' => 1 }]) + expect(versions(query(axis('linked_tags', tag.id.to_s)))).to eq([2]) + end + + it 'rejects the physical column as a public filter, axis, sort or change field' do + sign_in_as(create(:user, :admin)) + [ + axis('linked_tag_ids', tag.id.to_s), + axis('changed_field', 'linked_tag_ids'), + { mode: 'summary', rows: 'linked_tag_ids', cols: '', sort: '' }, + { sort: 'linked_tag_ids:asc' }].each do |invalid| + get '/counts/query', params: base.merge(invalid) + expect(response).to have_http_status(:bad_request), invalid.inspect + end + end + end + it 'identifies a totals timeout without exposing SQL in the error response' do allow_any_instance_of(CountsPivotTotals).to receive(:call) .and_raise(CountsPivotTotals::TimedOut, 'private SQL') @@ -223,11 +313,10 @@ RSpec.describe 'Counts table API', type: :request do expect(versions(query(tags: 'renamed-alpha'))).to eq([1, 2, 4]) end - it 'resolves external names from current registrations too' do - external.update!(name: 'renamed-beta') - expect(versions(query(tags: 'nico:renamed-beta'))).to eq([1]) - expect(versions(query(tags: 'nico:beta'))).to eq([]) - expect(versions(query(tags: 'not:nico:renamed-beta'))).to eq([2, 3, 4]) + it 'resolves immutable external names across historical snapshots' do + expect(versions(query(tags: 'nico:beta'))).to eq([1]) + expect(versions(query(tags: 'nico:renamed-beta'))).to eq([]) + expect(versions(query(tags: 'not:nico:beta'))).to eq([2, 3, 4]) end end diff --git a/backend/spec/requests/nico_tags_spec.rb b/backend/spec/requests/nico_tags_spec.rb index a554d26..264f844 100644 --- a/backend/spec/requests/nico_tags_spec.rb +++ b/backend/spec/requests/nico_tags_spec.rb @@ -38,12 +38,9 @@ RSpec.describe 'NicoTags', type: :request do end it 'filters by nico tag name, linked tag name, and link status' do - linked = create(:external_tag) - linked.update!(name: 'search_linked') - unlinked = create(:external_tag) - unlinked.update!(name: 'search_unlinked') - other = create(:external_tag) - other.update!(name: 'other') + linked = create(:external_tag, name: 'search_linked') + unlinked = create(:external_tag, name: 'search_unlinked') + other = create(:external_tag, name: 'other') destination = create(:tag, category: :general) primary_tag_name_for(destination, 'ja').update!(name: 'destination_search') NicoTagRelation.create!(nico_tag: linked, tag: destination) @@ -80,11 +77,9 @@ RSpec.describe 'NicoTags', type: :request do end it 'sorts by name and timestamps' do - older = create(:external_tag) - older.update!(name: 'ordered_a') + older = create(:external_tag, name: 'ordered_a') older.update_columns(created_at: 2.days.ago) - newer = create(:external_tag) - newer.update!(name: 'ordered_b') + newer = create(:external_tag, name: 'ordered_b') newer.update_columns(created_at: 1.day.ago) older_post_tag = PostExternalTag.create!(post: create(:post), external_tag: older) @@ -189,9 +184,8 @@ RSpec.describe 'NicoTags', type: :request do versions = nico_tag.reload.nico_tag_versions.order(:version_no) expect(versions.map(&:event_type)).to eq(['create', 'update']) - expect(versions.last.linked_tags.split).to match_array([ - 'nico_linked_a', - 'nico_linked_b']) + expect(versions.last.linked_tag_ids.split.map(&:to_i)) + .to match_array([linked_a.id, linked_b.id]) expect(versions.last.created_by_user_id).to eq(admin.id) end @@ -210,7 +204,7 @@ RSpec.describe 'NicoTags', type: :request do expect(json).to eq([]) expect(nico_tag.reload.linked_tags).to be_empty expect(nico_tag.nico_tag_versions.order(:version_no).last) - .to have_attributes(linked_tags: '', created_by_user: member) + .to have_attributes(linked_tag_ids: '', created_by_user: member) end it 'returns the tags field error when a nico tag is specified directly' do diff --git a/backend/spec/requests/post_history_privacy_spec.rb b/backend/spec/requests/post_history_privacy_spec.rb index 635fb76..ff9df89 100644 --- a/backend/spec/requests/post_history_privacy_spec.rb +++ b/backend/spec/requests/post_history_privacy_spec.rb @@ -20,7 +20,7 @@ RSpec.describe 'Post history privacy', type: :request do created_at: origin + index.hours) end NicoTagVersion.create!(external_tag: external, version_no: 1, event_type: 'create', - name: 'nico:external-history-tag', linked_tags: 'private-history-tag', + linked_tag_ids: tag.id.to_s, created_at: origin) ['create', 'discard', 'restore', 'update'].each_with_index do |event, index| snapshots = diff --git a/backend/spec/services/counts_export_spec.rb b/backend/spec/services/counts_export_spec.rb index 94b82c1..b890323 100644 --- a/backend/spec/services/counts_export_spec.rb +++ b/backend/spec/services/counts_export_spec.rb @@ -22,6 +22,55 @@ RSpec.describe 'Counts export contracts' do .and_return(@counts_export_dir) end + context 'nico link IDs with the V1 export field name' do + let(:tag) { create(:tag, primary_name: 'linked export tag') } + let(:external) { create(:external_tag, name: 'external export tag') } + let(:payload) do + super().merge('resource' => 'nico_tags', 'columns' => 'linked_tags', + 'headers' => '["linked_tags"]') + end + + before do + NicoTagVersion.create!(external_tag: external, version_no: 1, + event_type: 'create', linked_tag_ids: '', created_at: Time.utc(2023)) + NicoTagVersion.create!(external_tag: external, version_no: 2, + event_type: 'update', linked_tag_ids: tag.id.to_s, created_at: Time.utc(2024)) + end + + it 'exports recorded IDs with the legacy JSON key and metadata' do + result = JSON.parse(CountsTableExport.new(payload).body.to_a.join) + expect(result.fetch('data')).to eq([ + { 'linked_tags' => '' }, { 'linked_tags' => tag.id.to_s }]) + expect(result.fetch('metadata').fetch('columns')).to eq(['linked_tags']) + keys = result.fetch('metadata').fetch('fields').map { _1.fetch('key') } + expect(keys).to include('linked_tags') + expect(keys).not_to include('linked_tag_ids') + end + + it 'exports the selected legacy CSV header and ID values' do + body = CountsTableExport.new(payload.merge('format' => 'csv')).body.to_a.join + expect(CSV.parse(body.delete_prefix("\uFEFF"))) + .to eq([['linked_tags'], [''], [tag.id.to_s]]) + end + + it 'keeps the legacy link column in default JSON exports' do + options = payload.except('columns', 'headers') + result = JSON.parse(CountsTableExport.new(options).body.to_a.join) + expect(result.fetch('metadata').fetch('columns')).to include('linked_tags') + result.fetch('data').each do |row| + expect(row).to have_key('linked_tags') + expect(row).not_to have_key('linked_tag_ids') + end + end + + it 'rejects the physical column as a selected CSV or JSON field' do + ['csv', 'json'].each do |format| + options = payload.merge('format' => format, 'columns' => 'linked_tag_ids') + expect { CountsTableExport.new(options) }.to raise_error(ArgumentError) + end + end + end + context 'serialization after the authorised query' do let(:rows) { [{ title: "=formula\n\"quoted\"" }, { title: 'second' }] } let(:page) do diff --git a/backend/spec/services/nico_tag_version_recorder_spec.rb b/backend/spec/services/nico_tag_version_recorder_spec.rb index e48745b..d6c961e 100644 --- a/backend/spec/services/nico_tag_version_recorder_spec.rb +++ b/backend/spec/services/nico_tag_version_recorder_spec.rb @@ -8,21 +8,23 @@ RSpec.describe NicoTagVersionRecorder do described_class.record!(external_tag:, event_type:, created_by_user: member) end - it 'records the external association and platform-qualified name' do + it 'records the external association and derives the platform-qualified name' do version = record(:create) expect(version).to have_attributes( external_tag:, version_no: 1, event_type: 'create', - name: 'nico:raw tag[]', linked_tags: '', created_by_user: member) + name: 'nico:raw tag[]', linked_tag_ids: '', created_by_user: member) expect(external_tag.reload.nico_tag_versions).to contain_exactly(version) end it 'uses the latest history number when the record has no version_no column' do first = record(:create) - external_tag.update!(name: 'changed') + tag = create(:tag) + NicoTagRelation.create!(nico_tag: external_tag, tag:) second = record(:update) - expect(second).to have_attributes(version_no: 2, name: 'nico:changed') + expect(second).to have_attributes(version_no: 2, name: 'nico:raw tag[]', + linked_tag_ids: tag.id.to_s) expect(first.reload.name).to eq('nico:raw tag[]') expect(external_tag.reload.has_attribute?(:version_no)).to be(false) end @@ -40,7 +42,7 @@ RSpec.describe NicoTagVersionRecorder do expect(version).to have_attributes( external_tag: external, name: 'registered_external:foo', version_no: 1, - event_type: 'create', linked_tags: '', created_by_user: member) + event_type: 'create', linked_tag_ids: '', created_by_user: member) end it 'returns the latest version without appending an unchanged snapshot' do @@ -64,13 +66,21 @@ RSpec.describe NicoTagVersionRecorder do expect(external_tag.nico_tag_versions).to contain_exactly(first) end - it 'records sorted linked internal names and later link removal' do + + it 'rejects discard and restore events for external tags' do + [:discard, :restore].each do |event_type| + expect { record(event_type) } + .to raise_error(ArgumentError, "Invalid event_type: #{ event_type }") + end + end + + it 'records sorted linked internal IDs and later link removal' do tags = ['z_link', 'a_link'].map { |name| create(:tag, primary_name: name, category: :general) } tags.each { |tag| NicoTagRelation.create!(nico_tag: external_tag, tag:) } - expect(record(:create).linked_tags).to eq('a_link z_link') + expect(record(:create).linked_tag_ids).to eq(tags.map(&:id).sort.join(' ')) external_tag.linked_tags = [tags.first] - expect(record(:update)).to have_attributes(version_no: 2, linked_tags: 'z_link') + expect(record(:update)).to have_attributes(version_no: 2, linked_tag_ids: tags.first.id.to_s) end end diff --git a/docs/counts-exports.md b/docs/counts-exports.md index 3d00934..e02a8b7 100644 --- a/docs/counts-exports.md +++ b/docs/counts-exports.md @@ -158,7 +158,15 @@ private ID の正誤に応じて許可可否を変えないため、条件の一 非公開列を含む列順変更は API の column_values_editable でも不可と通知する。 公開列の値を使うソートは引き続き利用できる。 明細には元の認可を適用し、抽出条件自体や取得済みの全言語値を表示設定で改変しない。 -外部タグ履歴の linked_tags は名前しか記録していないため、非公開タグの記録名を含む -スナップショットを保守的に非公開とする。名前全体を空白区切りの境界で照合する。 -区切りが曖昧な旧データでは過剰な秘匿の可能性はあるが、参照 ID がない旧形式から -名前の公開可否を楽観的に推定しない。 +外部タグ履歴の linked_tag_ids は、連携する内部タグの ID を半角スペース区切りで +記録する。除票履歴のある内部タグの ID を含むスナップショットを非管理者へ公開しない。 +名前や UI ロカールを参照 ID の判定に使わない。 +外部タグ名は履歴に保存せず、external_tags の platform と name を参照する。 + +V1 Counts の公開フィールド名は linked_tags を維持し、DB の linked_tag_ids へ対応付ける。 +明細・フィルタ・集計軸・変更項目・ソート・CSV / JSON 出力で同じ対応付けを使う。 +既定列と既存の表示ラベルも linked_tags のままとし、値は ID のスペース区切りとする。 +linked_tag_ids は物理カラム名であり、Counts の公開フィールドとしては受け付けない。 +旧 URL や保存済み設定のフィールド指定は維持できるが、値にタグ名を指定した検索条件や +ドリル条件は ID 値への見直しが必要となる。旧履歴の名称は移行後の ID 列から復元できず、 +現在の名称からの推定変換も行わない。