diff --git a/lib/fixture_kit/coders/active_record_coder.rb b/lib/fixture_kit/coders/active_record_coder.rb index ec76257..f1ee5d0 100644 --- a/lib/fixture_kit/coders/active_record_coder.rb +++ b/lib/fixture_kit/coders/active_record_coder.rb @@ -43,6 +43,12 @@ def mount(data) end verify_foreign_keys!(connection) + + # Replayed INSERTs use explicit PKs, which Postgres sequences do not + # observe. Re-sync the sequence so subsequent Model.create calls don't + # collide with an id we just inserted. No-op on adapters whose PK + # generators advance from explicit-id INSERTs (MySQL, SQLite). + reset_primary_key_sequences(connection, models.map(&:table_name)) end end end @@ -108,6 +114,16 @@ def verify_foreign_keys!(connection) end end + def reset_primary_key_sequences(connection, tables) + # Rails main (>= 8.2) batches the reset in one round-trip per connection. + # Older versions fall back to one query per table. + if connection.respond_to?(:reset_column_sequences!) + connection.reset_column_sequences!(tables.map { |t| [t] }) + elsif connection.respond_to?(:reset_pk_sequence!) + tables.each { |t| connection.reset_pk_sequence!(t) } + end + end + def models_by_pool(data) seen = Set.new diff --git a/spec/integration/pk_sequence_repro_spec.rb b/spec/integration/pk_sequence_repro_spec.rb new file mode 100644 index 0000000..f57aa36 --- /dev/null +++ b/spec/integration/pk_sequence_repro_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Reproduces issue #51: when a cached fixture is mounted onto a database whose +# PK generator has not seen the inserted ids (e.g. a parallel test worker with +# its own DB copy), a subsequent Model.create can collide with one of the +# explicit ids the cache replayed. Postgres exhibits this; MySQL/SQLite advance +# their counters from explicit-id inserts and stay safe. +RSpec.describe "Primary key sequence after fixture mount" do + fixture do + User.create!(name: "Alice PK Repro", email: "alice-pk-repro@example.com") + User.create!(name: "Bob PK Repro", email: "bob-pk-repro@example.com") + end + + after do + User.connection.disable_referential_integrity do + User.connection.execute("DELETE FROM #{User.quoted_table_name}") + end + end + + it "lets a new record be created without colliding with replayed explicit ids" do + # Simulate a parallel-worker scenario: empty table, PK generator at its + # initial value. The fixture's auto-mount already populated the table for + # us, so wipe and reset before re-mounting. + wipe_and_reset_pk!(User) + + declaration = self.class.metadata[FixtureKit::RSpec::DECLARATION_METADATA_KEY] + declaration.mount + + expect { + User.create!(name: "Charlie PK Repro", email: "charlie-pk-repro@example.com") + }.not_to raise_error + end + + def wipe_and_reset_pk!(model) + connection = model.connection + connection.disable_referential_integrity do + connection.execute("DELETE FROM #{model.quoted_table_name}") + end + + case connection.adapter_name.to_s.downcase + when "postgresql" + sequence = connection.pk_and_sequence_for(model.table_name)&.last + connection.execute("ALTER SEQUENCE #{sequence} RESTART WITH 1") if sequence + when "mysql", "mysql2", "trilogy" + # MySQL advances AUTO_INCREMENT on explicit-id INSERTs, so the counter + # already keeps up with the cached ids. ALTER TABLE ... AUTO_INCREMENT + # implicitly commits, which would break the surrounding transactional + # fixture, so we leave it alone. The test should still pass on MySQL. + when "sqlite" + if connection.data_source_exists?("sqlite_sequence") + connection.execute("DELETE FROM sqlite_sequence WHERE name = #{connection.quote(model.table_name)}") + end + else + raise "Unsupported adapter for PK reset: #{connection.adapter_name.inspect}" + end + end +end diff --git a/spec/unit/coders/active_record_coder_spec.rb b/spec/unit/coders/active_record_coder_spec.rb index ba969a8..8ffd821 100644 --- a/spec/unit/coders/active_record_coder_spec.rb +++ b/spec/unit/coders/active_record_coder_spec.rb @@ -156,6 +156,45 @@ def upsert_all_options(model) coder.mount(records) end + it "uses the batched reset_column_sequences! when the adapter exposes it" do + records = { + User => "INSERT INTO users (id, name) VALUES (1, 'Alice')", + Project => "INSERT INTO projects (id, name, owner_id) VALUES (1, 'Website', 1)" + } + + fake_connection = stub_fake_connection + allow(fake_connection).to receive(:respond_to?).with(:reset_column_sequences!).and_return(true) + allow(fake_connection).to receive(:reset_column_sequences!) + stub_shared_pool([User, Project], fake_connection) + + coder.mount(records) + + expect(fake_connection).to have_received(:reset_column_sequences!) + .with([[User.table_name], [Project.table_name]]).once + end + + it "falls back to per-table reset_pk_sequence! when reset_column_sequences! is unavailable" do + records = { User => "INSERT INTO users (id, name) VALUES (1, 'Alice')" } + + fake_connection = stub_fake_connection + allow(fake_connection).to receive(:respond_to?).with(:reset_pk_sequence!).and_return(true) + allow(fake_connection).to receive(:reset_pk_sequence!) + stub_pool(User, fake_connection) + + coder.mount(records) + + expect(fake_connection).to have_received(:reset_pk_sequence!).with(User.table_name).once + end + + it "skips PK sequence reset on adapters that expose neither method" do + records = { User => "INSERT INTO users (id, name) VALUES (1, 'Alice')" } + + fake_connection = stub_fake_connection + stub_pool(User, fake_connection) + + expect { coder.mount(records) }.not_to raise_error + end + context "when ActiveRecord.verify_foreign_keys_for_fixtures is true" do around do |example| previous = ActiveRecord.verify_foreign_keys_for_fixtures @@ -212,13 +251,19 @@ def stub_fake_connection allow(fake_connection).to receive(:execute_batch) allow(fake_connection).to receive(:quote_table_name) { |name| %("#{name}") } allow(fake_connection).to receive(:check_all_foreign_keys_valid!) + allow(fake_connection).to receive(:respond_to?).with(:reset_column_sequences!).and_return(false) + allow(fake_connection).to receive(:respond_to?).with(:reset_pk_sequence!).and_return(false) fake_connection end def stub_pool(model, connection) - pool = double("pool-for-#{model.name}") + stub_shared_pool([model], connection) + end + + def stub_shared_pool(models, connection) + pool = double("pool-for-#{models.map(&:name).join('-')}") allow(pool).to receive(:with_connection).and_yield(connection) - allow(model).to receive(:connection_pool).and_return(pool) + models.each { |m| allow(m).to receive(:connection_pool).and_return(pool) } end end