Skip to content

Commit

Permalink
Fix missed establish_connection
Browse files Browse the repository at this point in the history
In `connected_to` one of the deprecated arguments wasn't well tested so
the incorrect methods signature wasn't caught by the tests.

This change updates the caller when `connected_to` uses the database
key.

I've also cleaned up a few arguments that weren't necessary. Since
the handler methods set defaults for the `shard` key, we don't need to
pass that in `establish_connection` when not using the sharding API.
  • Loading branch information
eileencodes committed Aug 10, 2020
1 parent 64915e5 commit 919eb6d
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/connection_handling.rb
Expand Up @@ -49,7 +49,7 @@ module ConnectionHandling
def establish_connection(config_or_env = nil)
config_or_env ||= DEFAULT_ENV.call.to_sym
db_config, owner_name = resolve_config_for_connection(config_or_env)
connection_handler.establish_connection(db_config, owner_name: owner_name, shard: current_shard)
connection_handler.establish_connection(db_config, owner_name: owner_name)
end

# Connects a model to the databases specified. The +database+ keyword
Expand Down Expand Up @@ -89,7 +89,7 @@ def connects_to(database: {}, shards: {})
db_config, owner_name = resolve_config_for_connection(database_key)
handler = lookup_connection_handler(role.to_sym)

connections << handler.establish_connection(db_config, owner_name: owner_name, shard: default_shard)
connections << handler.establish_connection(db_config, owner_name: owner_name)
end

shards.each do |shard, database_keys|
Expand Down Expand Up @@ -154,7 +154,7 @@ def connected_to(database: nil, role: nil, shard: nil, prevent_writes: false, &b
db_config, owner_name = resolve_config_for_connection(database)
handler = lookup_connection_handler(role)

handler.establish_connection(db_config, default_shard, owner_name)
handler.establish_connection(db_config, owner_name: owner_name)

with_handler(role, &blk)
elsif shard
Expand Down
Expand Up @@ -217,6 +217,14 @@ def test_switching_connections_with_database_and_role_raises
assert_equal "`connected_to` cannot accept a `database` argument with any other arguments.", error.message
end

def test_database_argument_is_deprecated
assert_deprecated do
ActiveRecord::Base.connected_to(database: { writing: { adapter: "sqlite3", database: "test/db/primary.sqlite3" } }) { }
end
ensure
ActiveRecord::Base.establish_connection(:arunit)
end

def test_switching_connections_without_database_and_role_raises
error = assert_raises(ArgumentError) do
ActiveRecord::Base.connected_to { }
Expand Down

0 comments on commit 919eb6d

Please sign in to comment.