Transaction strategy configured with symbol(connection name) does not work
kaorukobo opened this issue · 1 comments
(I will post my suggestion next to this report.)
Summary
When we add another database to a cleaner by specifying a symbol (connection name) with the :transaction
strategy, it does not open a transaction.
Look at the reproduction below. Even though the cleaner is configured to clean :db => :bar
with the transaction strategy, the transaction is not opened*.
(*In fact, a transaction is opened on ActiveRecord::Base.connection
instead.)
Reproduction
#!/usr/bin/env ruby
require "bundler/inline"
gemfile(true) do
source 'https://rubygems.org'
gem 'rails', '~> 7.0.0'
gem 'sqlite3'
gem 'rspec'
gem 'database_cleaner-active_record', git: "https://github.com/DatabaseCleaner/database_cleaner-active_record.git", ref: "aafa5b2"
end
require "yaml"
ActiveRecord::Base.configurations = YAML.load(<<~YAML)
foo:
adapter: sqlite3
database: ":memory:"
bar:
adapter: sqlite3
database: ":memory:"
YAML
RSpec.describe do
before do
ActiveRecord::Base.establish_connection :foo
class Bar < ActiveRecord::Base
establish_connection :bar
end
end
it "should open a transaction on Bar" do
cleaner = DatabaseCleaner::Cleaners.new
cleaner[:active_record, :db => :bar].strategy = :transaction
cleaner.cleaning do
# expected: true
# got: false
expect(Bar.connection.transaction_open?).to eq(true)
end
end
it "should not open a transaction on ActiveRecord::Base where we removed it from cleaner" do
cleaner = DatabaseCleaner::Cleaners.new
cleaner[:active_record].strategy = nil
cleaner[:active_record, :db => :bar].strategy = :transaction
cleaner.cleaning do
# expected: false
# got: true
expect(ActiveRecord::Base.connection.transaction_open?).to eq(false)
end
end
end
Suggested Solution
In the reproduction above, try replacing all the :db => :bar
with :db => Bar
. The test passes.
Looking into the database_cleaner/active_record/base.rb
, it is managing to obtain the ActiveRecord class from a given connection name.
It makes the code have too much responsibility and very complex. That is, it is reading ActiveRecord::Base.configurations
, reading config/database.yml
, and searching through connection pools and ActiveRecord classes.
This complexity can easily bring bugs such as #10, #18 and #97.
Though it is a breaking change... if we make the cleaner only accept an ActiveRecord class object as a value for :db
option, doesn't database_cleaner/active_record/base.rb
become far simpler and reduce the possibility of bugs so much?