From 7086a76c88e040581447cfaa1bb8462132fecc36 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Wed, 21 Oct 2020 17:19:41 -0400 Subject: [PATCH] Ok - the state of things Things pass except for the fixture connection tests and one test that is getting too many shards in the list because we're no longer cleaning up connections at the end of the tests. We need a way I think to turn off shared connection pools for the AR tests, or at least when we're not doing multi-connection work. https://github.com/rails/rails/pull/40384 is interesting but I didn't have time to make it work with our stuff. So basically we're in a better state but I'm not quite sure yet how to get just the 2 tests that need shared pools to use them. Thoughts? --- .../lib/active_record/test_fixtures.rb | 21 ++--- .../test/cases/base_prevent_writes_test.rb | 1 - activerecord/test/cases/fixtures_test.rb | 79 ++++++++++++++++--- activerecord/test/cases/helper.rb | 17 ---- 4 files changed, 82 insertions(+), 36 deletions(-) diff --git a/activerecord/lib/active_record/test_fixtures.rb b/activerecord/lib/active_record/test_fixtures.rb index 2b512a38a74dd..dafb7aca3c8a1 100644 --- a/activerecord/lib/active_record/test_fixtures.rb +++ b/activerecord/lib/active_record/test_fixtures.rb @@ -189,19 +189,22 @@ def enlist_fixture_connections # need to share a connection pool so that the reading connection # can see data in the open transaction on the writing connection. def setup_shared_connection_pool + return if ENV["OFFFFFFF"] if ActiveRecord::Base.legacy_connection_handling - writing_handler = ActiveRecord::Base.connection_handlers[ActiveRecord::Base.writing_role] + ActiveSupport::Deprecation.silence do + writing_handler = ActiveRecord::Base.connection_handlers[ActiveRecord::Base.writing_role] - ActiveRecord::Base.connection_handlers.values.each do |handler| - if handler != writing_handler - handler.connection_pool_names.each do |name| - writing_pool_manager = writing_handler.send(:owner_to_pool_manager)[name] - return unless writing_pool_manager + ActiveRecord::Base.connection_handlers.values.each do |handler| + if handler != writing_handler + handler.connection_pool_names.each do |name| + writing_pool_manager = writing_handler.send(:owner_to_pool_manager)[name] + return unless writing_pool_manager - writing_pool_config = writing_pool_manager.get_pool_config(nil, :default) + writing_pool_config = writing_pool_manager.get_pool_config(nil, :default) - pool_manager = handler.send(:owner_to_pool_manager)[name] - pool_manager.set_pool_config(nil, :default, writing_pool_config) + pool_manager = handler.send(:owner_to_pool_manager)[name] + pool_manager.set_pool_config(nil, :default, writing_pool_config) + end end end end diff --git a/activerecord/test/cases/base_prevent_writes_test.rb b/activerecord/test/cases/base_prevent_writes_test.rb index e0e8489cececc..2773f65b69f72 100644 --- a/activerecord/test/cases/base_prevent_writes_test.rb +++ b/activerecord/test/cases/base_prevent_writes_test.rb @@ -202,7 +202,6 @@ def teardown end test "preventing writes with multiple handlers" do - skip "ugg" ActiveRecord::Base.connects_to(database: { writing: :arunit, reading: :arunit }) conn1_error = assert_raises ActiveRecord::ReadOnlyError do diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 0b3aef9f02e0a..4f454fa9912b2 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -1404,11 +1404,11 @@ def setup handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new handler.establish_connection(db_config) - assert_deprecated do - ActiveRecord::Base.connection_handlers = {} - end ActiveRecord::Base.connection_handler = handler + ActiveRecord::Base.connects_to(database: { writing: :default, reading: :readonly }) + + setup_shared_connection_pool end def teardown @@ -1428,15 +1428,76 @@ def test_uses_writing_connection_for_fixtures end def test_writing_and_reading_connections_are_the_same - if ActiveRecord::Base.legacy_connection_handling - rw_conn = ActiveRecord::Base.connection_handlers[:writing].connection_pool_list.first.connection - ro_conn = ActiveRecord::Base.connection_handlers[:reading].connection_pool_list.first.connection - else - rw_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:writing).first.connection - ro_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:reading).first.connection + rw_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:writing).first.connection + ro_conn = ActiveRecord::Base.connection_handler.connection_pool_list(:reading).first.connection + + assert_equal rw_conn, ro_conn + end + + private + def config + { "default" => default_config, "readonly" => readonly_config } + end + + def default_config + { "adapter" => "sqlite3", "database" => "test/fixtures/fixture_database.sqlite3" } + end + + def readonly_config + default_config.merge("replica" => true) + end + end + + class MultipleFixtureLegacyConnectionsTest < ActiveRecord::TestCase + include ActiveRecord::TestFixtures + + fixtures :dogs + + def setup + @old_value = ActiveRecord::Base.legacy_connection_handling + ActiveRecord::Base.legacy_connection_handling = true + + @old_handler = ActiveRecord::Base.connection_handler + @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config + db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new(ENV["RAILS_ENV"], "readonly", readonly_config) + + handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new + assert_deprecated do + handler.establish_connection(db_config) + ActiveRecord::Base.connection_handlers = {} end + ActiveRecord::Base.connection_handler = handler + ActiveRecord::Base.connects_to(database: { writing: :default, reading: :readonly }) + + setup_shared_connection_pool + end + + def teardown + ActiveRecord::Base.configurations = @prev_configs + ActiveRecord::Base.connection_handler = @old_handler + ActiveRecord::Base.legacy_connection_handling = false + end + + def test_uses_writing_connection_for_fixtures + ActiveRecord::Base.connected_to(role: :reading) do + Dog.first + + assert_nothing_raised do + ActiveRecord::Base.connected_to(role: :writing) { Dog.create! alias: "Doggo" } + end + end + end + + def test_writing_and_reading_connections_are_the_same_with_legacy_handling + writing = assert_deprecated { ActiveRecord::Base.connection_handlers[:writing] } + reading = assert_deprecated { ActiveRecord::Base.connection_handlers[:reading] } + + rw_conn = assert_deprecated { writing.connection_pool_list.first.connection } + ro_conn = assert_deprecated { reading.connection_pool_list.first.connection } assert_equal rw_conn, ro_conn + ensure + ActiveRecord::Base.legacy_connection_handling = @old_value end private diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 002a5e77e75c7..bafe808d7ef64 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -150,26 +150,9 @@ def disable_extension!(extension, connection) end def clean_up_legacy_connection_handlers - handler = ActiveRecord::Base.default_connection_handler - assert_deprecated do - ActiveRecord::Base.connection_handlers = { ActiveRecord::Base.writing_role => handler } - end - - handler.connection_pool_names.each do |name| - next if ["ActiveRecord::Base", "ARUnit2Model", "Contact", "ContactSti"].include?(name) - - handler.send(:owner_to_pool_manager).delete(name) - end end def clean_up_connection_handler - handler = ActiveRecord::Base.connection_handler - handler.instance_variable_get(:@owner_to_pool_manager).each do |owner, pool_manager| - pool_manager.role_names.each do |role_name| - next if role_name == ActiveRecord::Base.default_role - pool_manager.remove_role(role_name) - end - end end def load_schema