Ensure a handler is set when using `connected_to`

After looking at #35800 there is definitely an issue in the
`connected_to` method although it's generally behaving. Here are the
details:

1) I added a default connection role - writing - to the connection
handler lookup. I did this because otherwise if you did this:

```
connected_to(databse: :development)
```

And development wasn't a pre-established role it would create a new
handler and connect using that. I don't think this is right so I've
updated it to pick up the default (:writing) unless otherwise specified.
To set a handler when using the database version pass a hash like you
would to `connects_to`:

```
connected_to(database: { readonly_slow: :development })
```

This will connect the `development` database to the `readonly_slow`
handler/role.

2) I updated the tests to match this behavior that we expect.

3) I updated the documentation to clarify that using `connected_to` with
a `database` key will establish a new connection every time. This is
exactly how `establish_connection` behaves and I feel this is correct.
If you want to only establish a connection once you should do that in
the model with `connects_to` and then swap on the role instead of on the
database hash/key.

4) In regards to #35800 this fixes the case where you pass a symbol to
the db and not a hash. But it doesn't fix a case where you may pass an
unknown handler to an abstract class that's not connected. This is
tricky because technical AbstractFoo doesn't have any connections except
for through ApplicationRecord, so in the case of the application that
was shared we should only be swapping connections on ActiveRecord::Base
because there are no other actual connections - AbstractFoo isn't needed
since it's not establishing a new connection. If we need AbstractFoo to
connect to a new handler we should establish that connection with the
handler in AbstractFoo before trying to shard there.

2 files changed, 51 insertions, 17 deletions

diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb
index 53069cd899..6782833c5a 100644
--- a/activerecord/lib/active_record/connection_handling.rb
+++ b/activerecord/lib/active_record/connection_handling.rb
@@ -85,14 +85,14 @@ module ActiveRecord
     # based on the requested role:
     #
     #   ActiveRecord::Base.connected_to(role: :writing) do
-    #     Dog.create! # creates dog using dog connection
+    #     Dog.create! # creates dog using dog writing connection
     #   end
     #
     #   ActiveRecord::Base.connected_to(role: :reading) do
     #     Dog.create! # throws exception because we're on a replica
     #   end
     #
-    #   ActiveRecord::Base.connected_to(role: :unknown_ode) do
+    #   ActiveRecord::Base.connected_to(role: :unknown_role) do
     #     # raises exception due to non-existent role
     #   end
     #
@@ -100,11 +100,20 @@ module ActiveRecord
     # you can use +connected_to+ with a +database+ argument. The +database+ argument
     # expects a symbol that corresponds to the database key in your config.
     #
-    # This will connect to a new database for the queries inside the block.
-    #
     #   ActiveRecord::Base.connected_to(database: :animals_slow_replica) do
     #     Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+
     #   end
+    #
+    # This will connect to a new database for the queries inside the block. By
+    # default the `:writing` role will be used since all connections must be assigned
+    # a role. If you would like to use a different role you can pass a hash to database:
+    #
+    #   ActiveRecord::Base.connected_to(database: { readonly_slow: :animals_slow_replica }) do
+    #     Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+
+    #       using the readonly_slow role.
+    #   end
+    #
+    # When using the database key a new connection will be established every time.
     def connected_to(database: nil, role: nil, &blk)
       if database && role
         raise ArgumentError, "connected_to can only accept a `database` or a `role` argument, but not both arguments."
@@ -112,17 +121,14 @@ module ActiveRecord
         if database.is_a?(Hash)
           role, database = database.first
           role = role.to_sym
-        else
-          role = database.to_sym
         end
 
         config_hash = resolve_config_for_connection(database)
         handler = lookup_connection_handler(role)
 
-        with_handler(role) do
-          handler.establish_connection(config_hash)
-          yield
-        end
+        handler.establish_connection(config_hash)
+
+        with_handler(role, &blk)
       elsif role
         with_handler(role.to_sym, &blk)
       else
@@ -154,6 +160,7 @@ module ActiveRecord
     end
 
     def lookup_connection_handler(handler_key) # :nodoc:
+      handler_key ||= ActiveRecord::Base.writing_role
       connection_handlers[handler_key] ||= ActiveRecord::ConnectionAdapters::ConnectionHandler.new
     end
 
diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb
index 36591097b6..a2d289bf2f 100644
--- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb
+++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb
@@ -203,26 +203,53 @@ module ActiveRecord
           assert_equal "must provide a `database` or a `role`.", error.message
         end
 
-        def test_switching_connections_with_database_symbol
+        def test_switching_connections_with_database_symbol_uses_default_role
           previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env"
 
           config = {
             "default_env" => {
-              "readonly" => { adapter: "sqlite3", database: "db/readonly.sqlite3" },
+              "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" },
               "primary"  => { adapter: "sqlite3", database: "db/primary.sqlite3" }
             }
           }
           @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
 
-          ActiveRecord::Base.connected_to(database: :readonly) do
-            assert_equal :readonly, ActiveRecord::Base.current_role
-            assert ActiveRecord::Base.connected_to?(role: :readonly)
+          ActiveRecord::Base.connected_to(database: :animals) do
+            assert_equal :writing, ActiveRecord::Base.current_role
+            assert ActiveRecord::Base.connected_to?(role: :writing)
+
+            handler = ActiveRecord::Base.connection_handler
+            assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
+
+            assert_not_nil pool = handler.retrieve_connection_pool("primary")
+            assert_equal(config["default_env"]["animals"], pool.spec.config)
+          end
+        ensure
+          ActiveRecord::Base.configurations = @prev_configs
+          ActiveRecord::Base.establish_connection(:arunit)
+          ENV["RAILS_ENV"] = previous_env
+        end
+
+        def test_switching_connections_with_database_hash_uses_passed_role_and_database
+          previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env"
+
+          config = {
+            "default_env" => {
+              "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" },
+              "primary"  => { adapter: "sqlite3", database: "db/primary.sqlite3" }
+            }
+          }
+          @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config
+
+          ActiveRecord::Base.connected_to(database: { writing: :primary }) do
+            assert_equal :writing, ActiveRecord::Base.current_role
+            assert ActiveRecord::Base.connected_to?(role: :writing)
 
             handler = ActiveRecord::Base.connection_handler
-            assert_equal handler, ActiveRecord::Base.connection_handlers[:readonly]
+            assert_equal handler, ActiveRecord::Base.connection_handlers[:writing]
 
             assert_not_nil pool = handler.retrieve_connection_pool("primary")
-            assert_equal(config["default_env"]["readonly"], pool.spec.config)
+            assert_equal(config["default_env"]["primary"], pool.spec.config)
           end
         ensure
           ActiveRecord::Base.configurations = @prev_configs