外部タグの履歴管理方式を変更 (#445) (#480)

Co-authored-by: Codex Night Worker <codex-night@localhost>
Reviewed-on: #480
Co-authored-by: miteruzo <miteruzo@naver.com>

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

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

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
 

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
     )

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,115 @@ 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 'preloads external names separately for aliased link-change details' do
+      statements = []
+      listener = lambda do |_name, _start, _finish, _id, payload|
+        statements << payload[:sql]
+      end
+      ActiveSupport::Notifications.subscribed(listener, 'sql.active_record') do
+        result = query(axis('changed_field', 'linked_tags'))
+        expect(versions(result)).to eq([2, 3])
+        expect(result.fetch('versions').map { _1.fetch('name') })
+            .to eq(['nico:beta', 'nico:beta'])
+      end
+
+      details = statements.find { _1.include?('AS counts_latest') }
+      expect(details).not_to be_nil
+      expect(details).not_to include('JOIN `external_tags`')
+      names = statements.select { _1.match?(/SELECT .* FROM `external_tags`/) }
+      expect(names.length).to eq(1)
+    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 +332,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
 

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

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 =

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

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