diff --git a/CHANGELOG.md b/CHANGELOG.md index 1acecdc..8b907d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Development (unreleased) +* Pin the transaction strategy's connection on Rails 7.2+, as Rails' transactional tests do, so an executor wrap can't release it mid-test; add the `:lock_threads` option + ## v2.2.2 2025-07-30 * Release database connections after cleaning: https://github.com/DatabaseCleaner/database_cleaner-active_record/pull/122 diff --git a/README.md b/README.md index 3d7d49c..1bd47fb 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,15 @@ Some people report much faster speeds with `:deletion` while others say `:trunca ## Strategy configuration options -The transaction strategy accepts no options. +On Rails 7.2 and later the transaction strategy pins the connection, as Rails' own transactional tests do: it stays leased while the test runs, even across executor wraps that release connections (such as `perform_enqueued_jobs` or a request in a request spec), and every thread uses it, so all of them see the test's uncommitted data. + +The transaction strategy accepts the following option: + +* `:lock_threads` (Rails 7.2+, default `true`) makes threads take turns on the shared connection, like Rails' `lock_threads`. Set it to `false` only if no two threads query at the same time. + + ```ruby + DatabaseCleaner[:active_record].strategy = DatabaseCleaner::ActiveRecord::Transaction.new(lock_threads: false) + ``` The truncation and deletion strategies may accept the following options: diff --git a/lib/database_cleaner/active_record/transaction.rb b/lib/database_cleaner/active_record/transaction.rb index c7391f0..c800984 100644 --- a/lib/database_cleaner/active_record/transaction.rb +++ b/lib/database_cleaner/active_record/transaction.rb @@ -1,13 +1,31 @@ module DatabaseCleaner module ActiveRecord class Transaction < Base + def initialize(opts={}) + if !opts.empty? && !(opts.keys - [:lock_threads]).empty? + raise ArgumentError, "The only valid option is :lock_threads. You specified #{opts.keys.join(',')}." + end + + @lock_threads = opts.has_key?(:lock_threads) ? !!opts[:lock_threads] : true + end + def start - connection = if ::ActiveRecord.version >= Gem::Version.new("7.2") - connection_class.lease_connection - else - connection_class.connection + if pin_connection? + # Pin the connection the way Rails' transactional tests do, so it stays leased (and its + # transaction visible) even when an executor wrap releases connections, and so other + # threads share it instead of checking out one that can't see the uncommitted data. + pool = connection_class.connection_pool + pool.pin_connection!(@lock_threads) + connection = pool.lease_connection + # Rails 7.2-8.1 open the transaction in pin_connection!; later versions leave it to the caller. + began_transaction = !connection.transaction_open? + connection.begin_transaction(joinable: false, _lazy: false) if began_transaction + pins << began_transaction + return end + connection = connection_class.connection + # Hack to make sure that the connection is properly set up before cleaning connection.transaction {} @@ -16,6 +34,19 @@ def start def clean + if pin_connection? + pool = connection_class.connection_pool + until pins.empty? + if pins.pop + connection = pool.lease_connection + connection.rollback_transaction if connection.transaction_open? + end + pool.unpin_connection! + end + pool.release_connection + return + end + connection_class.connection_pool.connections.each do |connection| connection.lock.synchronize do next unless connection.open_transactions > 0 @@ -25,6 +56,16 @@ def clean connection_class.connection_pool.release_connection end + + private + + def pin_connection? + ::ActiveRecord.version >= Gem::Version.new("7.2") + end + + def pins + @pins ||= [] + end end end end diff --git a/spec/database_cleaner/active_record/transaction_spec.rb b/spec/database_cleaner/active_record/transaction_spec.rb index 4def03d..cd10b94 100644 --- a/spec/database_cleaner/active_record/transaction_spec.rb +++ b/spec/database_cleaner/active_record/transaction_spec.rb @@ -4,6 +4,16 @@ RSpec.describe DatabaseCleaner::ActiveRecord::Transaction do subject(:strategy) { described_class.new } + describe "options" do + it "accepts :lock_threads" do + expect { described_class.new(lock_threads: false) }.to_not raise_error + end + + it "rejects unknown options" do + expect { described_class.new(only: []) }.to raise_error(ArgumentError) + end + end + DatabaseHelper.with_all_dbs do |helper| context "using a #{helper.db} connection" do around do |example| @@ -56,6 +66,52 @@ end end + if ActiveRecord.version >= Gem::Version.new("7.2") + describe "connection pinning" do + let(:pool) { ActiveRecord::Base.connection_pool } + + after { strategy.clean } + + it "keeps the uncommitted records visible after an executor wrap releases connections" do + strategy.start + user = User.create! + + ActiveRecord::ConnectionAdapters::ConnectionPool::ExecutorHooks.complete(nil) + + # Another thread (e.g. the pool reaper) checks out the released connection meanwhile + leased, done = Queue.new, Queue.new + holder = Thread.new do + pool.lease_connection + leased << true + done.pop + pool.release_connection + end + leased.pop + + expect(User.exists?(user.id)).to be true + ensure + done << true + holder.join + end + + it "shares the connection with other threads" do + strategy.start + user = User.create! + + expect(Thread.new { User.exists?(user.id) }.value).to be true + end + + it "unpins the connection on #clean" do + strategy.start + connection = pool.lease_connection + strategy.clean + + expect(connection.transaction_open?).to be false + expect(pool.lease_connection.transaction_open?).to be false + end + end + end + describe "#cleaning" do context "with records" do it "should clean all tables" do