tags および tag_names にて論理削除を物理削除に変更 (#416) (#418)

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

backend/spec/db/delete_discarded_tags_spec.rb

@@ -0,0 +1,123 @@
+require 'rails_helper'
+require_relative '../../db/migrate/20260921020000_delete_discarded_records_from_tags'
+require_relative '../../db/migrate/20260921030000_delete_discarded_records_from_tag_names'
+
+RSpec.describe 'discarded tag cleanup migrations' do
+  [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |migration_class|
+    it "rejects rollback of #{ migration_class.name }" do
+      expect { migration_class.new.down }
+          .to raise_error(ActiveRecord::IrreversibleMigration)
+    end
+  end
+
+  context 'with legacy records' do
+    self.use_transactional_tests = false
+
+    before do
+      record_class = Class.new(ActiveRecord::Base) do
+        self.abstract_class = true
+      end
+      stub_const('TagCleanupMigrationRecord', record_class)
+      config = ActiveRecord::Base.connection_db_config.configuration_hash
+      @database = "btrc_hub_test_tag_cleanup_#{ Process.pid }_#{ SecureRandom.hex(4) }"
+      record_class.establish_connection(config.merge(database: nil))
+      @connection = record_class.lease_connection
+      @connection.create_database(@database)
+      @database_created = true
+      @connection.execute("USE #{ @connection.quote_table_name(@database) }")
+    end
+
+    after do
+      @connection.drop_database(@database) if @database_created
+    ensure
+      TagCleanupMigrationRecord.remove_connection
+    end
+
+    before do
+      @connection.create_table(:tag_names) do |t|
+        t.string :name, null: false, index: { unique: true }
+        t.bigint :canonical_id
+        t.datetime :discarded_at, index: true
+      end
+      @connection.add_foreign_key(:tag_names, :tag_names, column: :canonical_id)
+      @connection.create_table(:tags) do |t|
+        t.references :tag_name, null: false, foreign_key: true, index: { unique: true }
+        t.datetime :discarded_at, index: true
+      end
+      @connection.create_table(:nico_tag_relations) do |t|
+        t.references :tag, null: false, foreign_key: true
+        t.references :nico_tag, null: false, foreign_key: { to_table: :tags }
+      end
+      @connection.create_table(:tag_implications) do |t|
+        t.references :tag, null: false, foreign_key: true
+        t.references :parent_tag, null: false, foreign_key: { to_table: :tags }
+      end
+      [:tag_versions, :nico_tag_versions, :material_versions].each do |table|
+        @connection.create_table(table) do |t|
+          t.references :tag, null: false, foreign_key: true
+        end
+      end
+
+      @connection.execute(<<~SQL)
+        INSERT INTO tag_names (id, name, canonical_id, discarded_at) VALUES
+          (1, 'kept', NULL, NULL),
+          (2, 'merged_alias', 1, NULL),
+          (3, 'nico:deleted', NULL, '2026-09-20'),
+          (4, 'nico:kept', NULL, NULL),
+          (5, 'deleted_name', NULL, '2026-09-20')
+      SQL
+      @connection.execute(<<~SQL)
+        INSERT INTO tags (id, tag_name_id, discarded_at) VALUES
+          (1, 1, NULL), (2, 2, '2026-09-20'),
+          (3, 3, '2026-09-20'), (4, 4, NULL)
+      SQL
+      @connection.execute(<<~SQL)
+        INSERT INTO nico_tag_relations (id, tag_id, nico_tag_id) VALUES
+          (1, 1, 4), (2, 2, 4), (3, 1, 3), (4, 2, 3)
+      SQL
+      @connection.execute(<<~SQL)
+        INSERT INTO tag_implications (id, tag_id, parent_tag_id) VALUES
+          (1, 1, 4), (2, 2, 1), (3, 1, 2), (4, 2, 3)
+      SQL
+      @connection.execute('INSERT INTO tag_versions (tag_id) VALUES (1), (2)')
+      @connection.execute('INSERT INTO nico_tag_versions (tag_id) VALUES (3), (4)')
+      @connection.execute('INSERT INTO material_versions (tag_id) VALUES (1), (2)')
+    end
+
+    it 'removes discarded records and their links while retaining aliases and history' do
+      [DeleteDiscardedRecordsFromTags, DeleteDiscardedRecordsFromTagNames].each do |klass|
+        migration = klass.new
+        allow(migration).to receive(:connection).and_return(@connection)
+        migration.suppress_messages { migration.up }
+      end
+
+      expect(@connection.select_values('SELECT id FROM tags ORDER BY id')).to eq([1, 4])
+      expect(@connection.select_rows('SELECT id, canonical_id FROM tag_names ORDER BY id'))
+          .to eq([[1, nil], [2, 1], [4, nil]])
+      expect(@connection.select_values('SELECT id FROM nico_tag_relations')).to eq([1])
+      expect(@connection.select_values('SELECT id FROM tag_implications')).to eq([1])
+      expect(@connection.select_values('SELECT tag_id FROM tag_versions ORDER BY tag_id'))
+          .to eq([1, 2])
+      expect(@connection.select_values('SELECT tag_id FROM nico_tag_versions ORDER BY tag_id'))
+          .to eq([3, 4])
+      expect(@connection.select_values('SELECT tag_id FROM material_versions ORDER BY tag_id'))
+          .to eq([1, 2])
+
+      [:tags, :tag_names].each do |table|
+        expect(@connection.column_exists?(table, :discarded_at)).to be(false)
+        expect(@connection.index_exists?(table, :discarded_at)).to be(false)
+      end
+      [:tag_versions, :nico_tag_versions, :material_versions].each do |table|
+        expect(@connection.foreign_key_exists?(table, :tags, column: :tag_id)).to be(false)
+      end
+      expect(@connection.foreign_key_exists?(:tags, :tag_names)).to be(true)
+      expect(@connection.foreign_key_exists?(:tag_names, :tag_names, column: :canonical_id))
+          .to be(true)
+      expect(@connection.index_exists?(:tag_names, :name, unique: true)).to be(true)
+      expect(@connection.index_exists?(:tags, :tag_name_id, unique: true)).to be(true)
+      [:nico_tag_relations, :tag_implications].each do |table|
+        expect(@connection.foreign_keys(table).map(&:to_table)).to eq(['tags', 'tags'])
+      end
+    end
+  end
+end

backend/spec/models/tag_name_sanitisation_rule_spec.rb

@@ -57,7 +57,7 @@ RSpec.describe TagNameSanitisationRule, type: :model do
 
       it 'deletes the source tag_name' do
         described_class.apply!
-        expect(TagName.exists?(source.id)).to be(false)
+        expect(TagName.unscoped.exists?(source.id)).to be(false)
         expect(existing.reload.name).to eq('foobar')
       end
     end
@@ -75,7 +75,27 @@ RSpec.describe TagNameSanitisationRule, type: :model do
         described_class.apply!
         expected_tag_name_id = existing.canonical_id || existing.id
         expect(source_tag.reload.tag_name_id).to eq(expected_tag_name_id)
-        expect(TagName.exists?(source_tag_name_id)).to be(false)
+        expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false)
+      end
+    end
+
+    context 'when the sanitised name is an alias of an existing tag' do
+      let!(:existing_tag) { create(:tag) }
+      let!(:alias_name) do
+        TagName.create!(name: 'foobar', canonical: existing_tag.tag_name)
+      end
+      let!(:source) do
+        TagName.create!(name: 'tmp').tap do |tn|
+          tn.update_columns(name: 'foo_bar', updated_at: Time.current)
+        end
+      end
+
+      it 'deletes only the source and preserves the alias and its canonical tag' do
+        described_class.apply!
+
+        expect(TagName.unscoped.exists?(source.id)).to be(false)
+        expect(alias_name.reload.canonical).to eq(existing_tag.tag_name)
+        expect(Tag.find(existing_tag.id)).to eq(existing_tag)
       end
     end
 
@@ -92,13 +112,15 @@ RSpec.describe TagNameSanitisationRule, type: :model do
       end
 
       it 'merges the source tag into the existing tag and deletes the source tag_name' do
-        expect(TagName.find_by(name: 'foobar')&.tag&.id).to eq(existing_tag.id)
-        expect(TagName.find_by(name: 'foo_bar')&.tag&.id).to eq(source_tag.id)
+        post = create(:post)
+        PostTag.create!(post:, tag: source_tag)
 
         described_class.apply!
 
-        expect(Tag.exists?(source_tag.id)).to be(false)
-        expect(TagName.exists?(source_tag.tag_name_id)).to be(false)
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
+        expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false)
+        expect(post.reload.tags).to contain_exactly(existing_tag)
+        expect(existing_tag.reload.name).to eq('foobar')
       end
     end
   end

backend/spec/models/tag_spec.rb

@@ -173,6 +173,47 @@ RSpec.describe Tag, type: :model do
     end
   end
 
+  describe '.find_or_create_by_tag_name!' do
+    it 'creates a tag and name with the requested category after stripping whitespace' do
+      tag = nil
+
+      expect {
+        tag = described_class.find_or_create_by_tag_name!(
+            '  lookup_new  ', category: :character)
+      }.to change(Tag, :count).by(1).and change(TagName, :count).by(1)
+
+      expect(tag.name).to eq('lookup_new')
+      expect(tag.category).to eq('character')
+    end
+
+    it 'reuses the canonical tag for an alias without changing its category' do
+      tag = create(:tag, category: :character)
+      alias_name = TagName.create!(name: 'lookup_alias', canonical: tag.tag_name)
+      found = nil
+
+      expect {
+        found = described_class.find_or_create_by_tag_name!(
+            alias_name.name, category: :general)
+      }.to change(Tag, :count).by(0).and change(TagName, :count).by(0)
+
+      expect(found).to eq(tag)
+      expect(found.category).to eq('character')
+    end
+
+    it 'creates a tag for an existing canonical name reached through an alias' do
+      canonical = create(:tag_name)
+      alias_name = TagName.create!(name: 'lookup_alias', canonical:)
+      tag = nil
+
+      expect {
+        tag = described_class.find_or_create_by_tag_name!(
+            alias_name.name, category: :general)
+      }.to change(Tag, :count).by(1).and change(TagName, :count).by(0)
+
+      expect(tag.tag_name).to eq(canonical)
+    end
+  end
+
   describe '.merge_tags!' do
     let!(:target_tag) { create(:tag, category: :general) }
     let!(:source_tag) { create(:tag, category: :general) }
@@ -185,17 +226,14 @@ RSpec.describe Tag, type: :model do
     context 'when merging a simple source tag' do
       let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) }
 
-      it 'deletes the source link, links the target, and aliases the discarded source tag' do
+      it 'deletes the source tag, moves its post link, and keeps its name as an alias' do
         described_class.merge_tags!(target_tag, [source_tag])
 
         target_link = PostTag.find_by(post: post_record, tag: target_tag)
 
         expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false)
         expect(target_link).to be_present
-        expect(source_tag.reload.post_count).to eq(0)
-
-        expect(Tag.with_discarded.find(source_tag.id)).to be_discarded
-        expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
         expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
         expect(target_tag.reload.post_count).to eq(1)
       end
@@ -218,17 +256,76 @@ RSpec.describe Tag, type: :model do
 
         expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false)
         expect(target_links).to contain_exactly(target_post_tag)
-        expect(source_tag.reload.post_count).to eq(0)
         expect(PostTagSection.where(post: post_record, tag: source_tag)).to be_empty
         expect(target_post_tag.reload.sections).to contain_exactly(target_section)
 
-        expect(Tag.with_discarded.find(source_tag.id)).to be_discarded
-        expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
         expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
         expect(target_tag.reload.post_count).to eq(1)
       end
     end
 
+    it 'keeps source history and records the new target alias after deleting the source' do
+      user = create_member_user!
+      source_name = source_tag.name
+      source_alias = TagName.create!(name: 'merge_alias', canonical: source_tag_name)
+      TagVersioning.ensure_snapshot!(source_tag, created_by_user: user)
+      original_version = source_tag.tag_versions.first
+
+      described_class.merge_tags!(target_tag, [source_tag], created_by_user: user)
+
+      versions = TagVersion.where(tag_id: source_tag.id).order(:version_no)
+      expect(versions.pluck(:version_no, :event_type))
+          .to eq([[1, 'create'], [2, 'discard']])
+      expect(versions.first).to eq(original_version)
+      expect(versions.last).to have_attributes(
+          name: source_name, aliases: source_alias.name, created_by_user: user)
+      expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
+
+      target_versions = target_tag.tag_versions.order(:version_no)
+      expect(target_versions.pluck(:event_type)).to eq(['create', 'update'])
+      expect(target_versions.last.aliases.split).to eq([source_name])
+    end
+
+    it 'deletes source relationships while preserving unrelated relationships' do
+      parent = create(:tag)
+      child = create(:tag)
+      nico_tag = create(:tag, :nico)
+      TagImplication.create!(tag: source_tag, parent_tag: parent)
+      TagImplication.create!(tag: child, parent_tag: source_tag)
+      kept_implication = TagImplication.create!(tag: target_tag, parent_tag: parent)
+      NicoTagRelation.create!(tag: source_tag, nico_tag:)
+      kept_relation = NicoTagRelation.create!(tag: target_tag, nico_tag:)
+      TagSimilarity.create!(tag: source_tag, target_tag:, cos: 0.5)
+      TagSimilarity.create!(tag: target_tag, target_tag: source_tag, cos: 0.5)
+      kept_similarity = TagSimilarity.create!(tag: target_tag, target_tag: parent,
+                                               cos: 0.5)
+
+      described_class.merge_tags!(target_tag, [source_tag])
+
+      expect(TagImplication.all).to contain_exactly(kept_implication)
+      expect(NicoTagRelation.all).to contain_exactly(kept_relation)
+      expect(TagSimilarity.all).to contain_exactly(kept_similarity)
+      expect(TagVersion.where(tag_id: source_tag.id).order(:version_no).last.parent_tag_ids)
+          .to eq(parent.id.to_s)
+    end
+
+    it 'preserves material history referencing the deleted source tag' do
+      source_tag.update!(category: :material)
+      target_tag.update!(category: :material)
+      material = Material.create!(tag: source_tag, url: 'https://example.com/material')
+      version = MaterialVersionRecorder.record!(
+          material:, event_type: :create, created_by_user: nil)
+      material.update!(tag: target_tag)
+
+      described_class.merge_tags!(target_tag, [source_tag])
+
+      expect(version.reload).to have_attributes(
+          tag_id: source_tag.id, tag_name: source_tag_name.name, tag_category: 'material')
+      expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
+      expect(material.reload.tag).to eq(target_tag)
+    end
+
     context 'when source_tags includes the target itself' do
       let!(:source_post_tag) { PostTag.create!(post: post_record, tag: source_tag) }
 
@@ -238,7 +335,7 @@ RSpec.describe Tag, type: :model do
         target_link = PostTag.find_by(post: post_record, tag: target_tag)
 
         expect(Tag.find(target_tag.id)).to be_present
-        expect(Tag.with_discarded.find(source_tag.id)).to be_discarded
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
         expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false)
         expect(target_link).to be_present
         expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
@@ -263,7 +360,7 @@ RSpec.describe Tag, type: :model do
         )
       end
 
-      it 'still merges, but discards the source tag_name instead of aliasing it' do
+      it 'still merges and keeps the source name as an alias without validating it' do
         described_class.merge_tags!(target_tag, [source_tag])
 
         target_link = PostTag.find_by(post: post_record, tag: target_tag)
@@ -271,7 +368,8 @@ RSpec.describe Tag, type: :model do
         expect(PostTag.exists?(post: post_record, tag: source_tag)).to be(false)
         expect(target_link).to be_present
 
-        expect(Tag.with_discarded.find(source_tag.id)).to be_discarded
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
+        expect(source_tag_name.reload.canonical_id).to eq(target_tag.tag_name_id)
         expect(target_tag.reload.post_count).to eq(1)
       end
     end
@@ -288,17 +386,23 @@ RSpec.describe Tag, type: :model do
             message: 'init')
       end
 
-      it 'rolls back the transaction' do
+      it 'rolls back earlier deletions, links, and history when a later source has a wiki' do
+        earlier_source = create(:tag)
+        earlier_name = earlier_source.tag_name
         source_section = create(:post_tag_section, post: post_record,
                                                    tag: source_tag,
                                                    begin_ms: 1000, end_ms: 2000)
 
         expect {
-          described_class.merge_tags!(target_tag, [source_tag])
+          described_class.merge_tags!(target_tag, [earlier_source, source_tag])
         }.to raise_error(ActiveRecord::RecordInvalid)
 
-        expect(Tag.with_discarded.find(source_tag.id)).not_to be_discarded
-        expect(TagName.with_discarded.find(source_tag_name.id)).not_to be_discarded
+        expect(Tag.unscoped.exists?(earlier_source.id)).to be(true)
+        expect(earlier_name.reload.canonical_id).to be_nil
+        expect(TagVersion.where(tag_id: [earlier_source.id, source_tag.id, target_tag.id]))
+            .to be_empty
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(true)
+        expect(TagName.unscoped.exists?(source_tag_name.id)).to be(true)
         expect(source_post_tag.reload.tag_id).to eq(source_tag.id)
         expect(source_post_tag.sections).to contain_exactly(source_section)
         expect(PostTag.find_by(post: post_record, tag: target_tag)).to be_nil
@@ -309,21 +413,45 @@ RSpec.describe Tag, type: :model do
     end
 
     context 'when merging a nico source tag' do
-      let!(:target_tag) { create(:tag, category: :nico, name: 'nico:foo') }
-      let!(:source_tag) { create(:tag, category: :nico, name: 'nico:bar') }
+      let!(:target_tag) do
+        create(:tag, category: :nico, tag_name: create(:tag_name, name: 'nico:foo'))
+      end
+      let!(:source_tag) do
+        create(:tag, category: :nico, tag_name: create(:tag_name, name: 'nico:bar'))
+      end
       let!(:source_tag_name_id) { source_tag.tag_name_id }
 
-      it 'discards the source tag_name instead of aliasing it' do
+      it 'deletes the source tag and name instead of keeping an alias' do
         described_class.merge_tags!(target_tag, [source_tag])
 
-        discarded_source_tag = Tag.with_discarded.find(source_tag.id)
-        discarded_source_tag_name = TagName.with_discarded.find(source_tag_name_id)
-
-        expect(discarded_source_tag).to be_discarded
-        expect(discarded_source_tag_name).to be_discarded
-        expect(discarded_source_tag_name.canonical_id).to be_nil
+        expect(Tag.unscoped.exists?(source_tag.id)).to be(false)
+        expect(TagName.unscoped.exists?(source_tag_name_id)).to be(false)
         expect(target_tag.reload.post_count).to eq(0)
       end
+
+      it 'keeps nico history while deleting source links and allows recreating the name' do
+        linked_tag = create(:tag)
+        NicoTagRelation.create!(nico_tag: source_tag, tag: linked_tag)
+        kept_relation = NicoTagRelation.create!(nico_tag: target_tag, tag: linked_tag)
+        user = create_member_user!
+        source_name = source_tag.name
+
+        described_class.merge_tags!(target_tag, [source_tag], created_by_user: user)
+
+        expect(NicoTagRelation.all).to contain_exactly(kept_relation)
+        versions = NicoTagVersion.where(tag_id: source_tag.id).order(:version_no)
+        expect(versions.pluck(:version_no, :event_type))
+            .to eq([[1, 'create'], [2, 'discard']])
+        expect(versions.last).to have_attributes(
+            name: source_name, linked_tags: linked_tag.name, created_by_user: user)
+
+        recreated = described_class.find_or_create_by_tag_name!(source_name, category: :nico)
+
+        expect(recreated.id).not_to eq(source_tag.id)
+        expect(recreated.tag_name_id).not_to eq(source_tag_name_id)
+        expect(recreated.nico_tag_versions).to be_empty
+        expect(versions.reload.size).to eq(2)
+      end
     end
 
     def snapshot_tags(post)

backend/spec/requests/posts_spec.rb

@@ -1252,9 +1252,7 @@ RSpec.describe 'Posts API', type: :request do
 
     context 'when nico tag already exists in tags' do
       before do
-        Tag.find_undiscard_or_create_by!(
-            tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'),
-            category: :nico)
+        Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico)
       end
 
       it 'returns 422 with tag field errors' do
@@ -1580,9 +1578,7 @@ RSpec.describe 'Posts API', type: :request do
 
     context 'when nico tag already exists in tags' do
       before do
-        Tag.find_undiscard_or_create_by!(
-            tag_name: TagName.find_undiscard_or_create_by!(name: 'nico:nico_tag'),
-            category: :nico)
+        Tag.find_or_create_by_tag_name!('nico:nico_tag', category: :nico)
       end
 
       it 'returns 422 with tag field errors' do

backend/spec/tasks/nico_sync_spec.rb

@@ -8,8 +8,7 @@ RSpec.describe 'nico:sync' do
   end
 
   def create_tag!(name, category:)
-    tn = TagName.find_undiscard_or_create_by!(name: name.to_s.strip)
-    Tag.find_undiscard_or_create_by!(tag_name_id: tn.id) { |t| t.category = category }
+    Tag.find_or_create_by_tag_name!(name, category:)
   end
 
   def link_nico_to_tag!(nico_tag, tag)