#431

backend/spec/services/shared_resource_id_allocator_spec.rb

@@ -1,6 +1,100 @@
 require 'rails_helper'
 
 RSpec.describe SharedResourceIdAllocator do
+  let(:tag) { create(:tag) }
+
+  def resource table, id: nil, number: 1
+    case table
+    when 'tags'
+      Tag.new(id:, category: :general)
+    when 'external_tags'
+      ExternalTag.new(id:, platform: :nico, name: "external #{ number }")
+    when 'tag_descriptions'
+      TagDescription.new(id:, tag:, language_code: 'ja', body: 'body', version_no: number)
+    when 'tag_groups'
+      TagGroup.new(id:, name: "group #{ number }", version_no: number)
+    when 'tag_description_versions'
+      TagDescriptionVersion.new(
+          id:, tag_id: tag.id, language_code: 'ja', body: 'body', version_no: number,
+          event_type: 'create', created_at: Time.current)
+    when 'tag_group_versions'
+      TagGroupVersion.new(
+          id:, tag_group_id: group.id, name: group.name, version_no: number,
+          event_type: 'create', members: [], placements: [], created_at: Time.current)
+    end
+  end
+
+  let(:group) { TagGroup.create!(name: 'history owner', version_no: 1) }
+
+  described_class::SPACES.each do |space, tables|
+    tables.permutation.each do |first_table, second_table|
+      it "keeps deleted #{ first_table } IDs reserved for #{ second_table }" do
+        first = resource(first_table)
+        first.save!
+        used_id = first.id
+        first.class.where(id: first.id).delete_all
+        second = resource(second_table)
+        second.save!
+        expect(second.id).to be > used_id
+      end
+
+      it "advances #{ space } after an explicit INSERT in #{ first_table }" do
+        first = resource(first_table)
+        first.save!
+        high = first.id + 100
+        explicit = resource(first_table, id: high, number: 2)
+        first.class.where(id: first.id).delete_all
+        explicit.save!
+        explicit.class.where(id: explicit.id).delete_all
+        second = resource(second_table)
+        second.save!
+        expect(second.id).to be > high
+      end
+    end
+
+    it "rejects explicit IDs below the reserved #{ space } position" do
+      first = resource(tables.first)
+      first.save!
+      first.class.where(id: first.id).delete_all
+      expect { resource(tables.last, id: first.id).save! }
+          .to raise_error(ActiveRecord::RecordNotUnique)
+    end
+
+    it "preserves historical #{ space } overlaps without repairing them" do
+      first = resource(tables.first)
+      first.save!
+      second = resource(tables.last)
+      second.save!
+      second.class.where(id: second.id).update_all(id: first.id)
+      second = second.class.find(first.id)
+      snapshots = [first.reload.attributes, second.reload.attributes]
+      next_record = resource(tables.last, number: 2)
+      next_record.save!
+      expect(next_record.id).to be > first.id
+      expect(first.reload.attributes).to eq(snapshots.first)
+      expect(second.reload.attributes).to eq(snapshots.last)
+    end
+  end
+
+  it 'reserves a Tag ID referenced only by remaining description history' do
+    high = [Tag.maximum(:id).to_i, ExternalTag.maximum(:id).to_i].max + 1000
+    TagDescriptionVersion.create!(
+        tag_id: high, language_code: 'ja', version_no: 1, body: 'history',
+        event_type: 'create', created_at: Time.current)
+    expect(create(:tag).id).to be > high
+    expect(create(:external_tag).id).to be > high
+  end
+
+  it 'reserves a group ID referenced only by remaining group history' do
+    high = [TagGroup.maximum(:id).to_i, TagDescription.maximum(:id).to_i].max + 1000
+    version = resource('tag_group_versions')
+    version.save!
+    TagGroupVersion.where(id: version.id).update_all(tag_group_id: high)
+    expect(TagGroup.create!(name: 'replacement', version_no: 1).id).to be > high
+  end
+end
+
+RSpec.describe SharedResourceIdAllocator, 'lock lifecycle' do
   let(:record) { build(:external_tag) }
   let(:connection) { double('connection') }
   let(:transaction) { double('transaction', open?: true) }
@@ -11,6 +105,14 @@ RSpec.describe SharedResourceIdAllocator do
     config = double('config', database: 'test_shared_ids')
     allow(connection).to receive_message_chain(:pool, :db_config).and_return(config)
     allow(connection).to receive(:quote) { "'#{ _1 }'" }
+    allow(connection).to receive(:quote_column_name) { "`#{ _1 }`" }
+    allow(connection).to receive(:execute) { sql << _1 }
+    allow(connection).to receive(:select_all) do |statement|
+      tables = described_class::SPACES.values.flatten.select do |table|
+        statement.include?("'#{ table }'")
+      end
+      tables.map { { 'TABLE_NAME' => _1, 'ENGINE' => 'InnoDB', 'AUTO_INCREMENT' => 9 } }
+    end
     allow(connection).to receive(:quote_table_name) { "`#{ _1 }`" }
     allow(connection).to receive(:select_value) do |statement|
       sql << statement
@@ -35,7 +137,6 @@ RSpec.describe SharedResourceIdAllocator do
       described_class.assign!(record)
       expect(record.id).to eq(9)
       expect(sql.grep(/RELEASE_LOCK/)).to be_empty
-      expect(sql.grep(/FOR UPDATE/).size).to eq(2)
       callbacks.fetch(event).call
       expect(sql.grep(/RELEASE_LOCK/).size).to eq(1)
     end
@@ -49,22 +150,22 @@ RSpec.describe SharedResourceIdAllocator do
   end
 
   it 'releases a successfully acquired lock when allocation fails and rolls back' do
-    allow(connection).to receive(:select_value).with(/FOR UPDATE/).and_raise('query failed')
+    allow(connection).to receive(:select_all).and_raise('query failed')
     expect { described_class.assign!(record) }.to raise_error(RuntimeError, 'query failed')
     callbacks.fetch(:after_rollback).call
     expect(sql.grep(/RELEASE_LOCK/).size).to eq(1)
   end
 
-  it 'preserves explicitly supplied historical IDs' do
-    record.id = 5
+  it 'accepts an explicit ID above the reserved position' do
+    record.id = 50
     described_class.assign!(record)
-    expect(record.id).to eq(5)
+    expect(record.id).to eq(50)
     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|
+    locks = [Tag, ExternalTag, TagDescription, TagGroup, TagDescriptionVersion, TagGroupVersion].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
@@ -76,7 +177,8 @@ RSpec.describe SharedResourceIdAllocator do
     end
     expect(locks[0]).to eq(locks[1])
     expect(locks[2]).to eq(locks[3])
-    expect(locks[0]).not_to eq(locks[2])
+    expect(locks[4]).to eq(locks[5])
+    expect([locks[0], locks[2], locks[4]].uniq.size).to eq(3)
   end
 
   it 'reports lock release failures' do