diff options
author | Arthur Neves <arthurnn@gmail.com> | 2016-05-24 23:57:43 -0400 |
---|---|---|
committer | Arthur Neves <arthurnn@gmail.com> | 2016-05-24 23:57:43 -0400 |
commit | 779ccf8a0e6a8bf7bb362c30dac4a340599ab113 (patch) | |
tree | 6ba707f5fca16144290128c4a3eb386ca6a6968c | |
parent | 5167b8f7ffa66c8772c455916b8418844e3b4674 (diff) | |
download | rails-779ccf8a0e6a8bf7bb362c30dac4a340599ab113.tar.gz rails-779ccf8a0e6a8bf7bb362c30dac4a340599ab113.tar.bz2 rails-779ccf8a0e6a8bf7bb362c30dac4a340599ab113.zip |
Remove `name` from `establish_connection`
Instead of passing a separete name variable, we can make the resolver
merge a name on the config, and use that before creating the Specification.
7 files changed, 32 insertions, 33 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index c124f1c4b5..ca04058539 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -837,13 +837,9 @@ module ActiveRecord end alias :connection_pools :connection_pool_list - def establish_connection(spec_or_config, name: "primary") - if spec_or_config.is_a?(ConnectionSpecification) - spec = spec_or_config - else - resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(ActiveRecord::Base.configurations) - spec = resolver.spec(spec_or_config, name) - end + def establish_connection(config) + resolver = ConnectionSpecification::Resolver.new(Base.configurations) + spec = resolver.spec(config) remove_connection(spec.name) owner_to_pool[spec.name] = ConnectionAdapters::ConnectionPool.new(spec) @@ -879,9 +875,9 @@ module ActiveRecord # for (not necessarily the current class). def retrieve_connection(spec_name) #:nodoc: pool = retrieve_connection_pool(spec_name) - raise ConnectionNotEstablished, "No connection pool with id #{spec_name} found." unless pool + raise ConnectionNotEstablished, "No connection pool with id '#{spec_name}' found." unless pool conn = pool.connection - raise ConnectionNotEstablished, "No connection for #{spec_name} in connection pool" unless conn + raise ConnectionNotEstablished, "No connection for '#{spec_name}' in connection pool" unless conn conn end @@ -915,7 +911,8 @@ module ActiveRecord # A connection was established in an ancestor process that must have # subsequently forked. We can't reuse the connection, but we can copy # the specification and establish a new connection with it. - establish_connection(ancestor_pool.spec).tap do |pool| + spec = ancestor_pool.spec + establish_connection(spec.config.merge("name" => spec.name)).tap do |pool| pool.schema_cache = ancestor_pool.schema_cache if ancestor_pool.schema_cache end else diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 7636de067c..0f4bd1c84a 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -164,7 +164,7 @@ module ActiveRecord # spec.config # # => { "host" => "localhost", "database" => "foo", "adapter" => "sqlite3" } # - def spec(config, name = nil) + def spec(config) spec = resolve(config).symbolize_keys raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter) @@ -184,13 +184,7 @@ module ActiveRecord raise AdapterNotFound, "database configuration specifies nonexistent #{spec.config[:adapter]} adapter" end - name ||= - if config.is_a?(Symbol) - config.to_s - else - "primary" - end - ConnectionSpecification.new(name, spec, adapter_method) + ConnectionSpecification.new(spec.delete(:name) || "primary", spec, adapter_method) end private @@ -235,7 +229,7 @@ module ActiveRecord # def resolve_symbol_connection(spec) if config = configurations[spec.to_s] - resolve_connection(config) + resolve_connection(config).merge("name" => spec.to_s) else raise(AdapterNotSpecified, "'#{spec}' database is not configured. Available: #{configurations.keys.inspect}") end diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index b3fd837abe..086c678af5 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -50,7 +50,12 @@ module ActiveRecord config ||= DEFAULT_ENV.call.to_sym spec_name = self == Base ? "primary" : name self.connection_specification_name = spec_name - connection_handler.establish_connection(config, name: spec_name) + + resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(Base.configurations) + spec = resolver.resolve(config).symbolize_keys + spec[:name] = spec_name + + connection_handler.establish_connection(spec) end class MergeAndResolveDefaultUrlConfig # :nodoc: diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 0df46d54df..cc8b2afa42 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -120,7 +120,7 @@ module ActiveRecord old_pool = ActiveRecord::Base.connection_handler.retrieve_connection_pool(ActiveRecord::Base.connection_specification_name) each_local_configuration { |configuration| create configuration } if old_pool - ActiveRecord::Base.connection_handler.establish_connection(old_pool.spec) + ActiveRecord::Base.connection_handler.establish_connection(old_pool.spec.config.merge("name" => old_pool.spec.name)) end end diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 50f942f5aa..039aca73c6 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -5,16 +5,15 @@ module ActiveRecord class ConnectionHandlerTest < ActiveRecord::TestCase def setup @handler = ConnectionHandler.new - resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new Base.configurations @spec_name = "primary" - @pool = @handler.establish_connection(resolver.spec(:arunit, @spec_name)) + @pool = @handler.establish_connection(ActiveRecord::Base.configurations['arunit']) end def test_establish_connection_uses_spec_name config = {"readonly" => {"adapter" => 'sqlite3'}} resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(config) spec = resolver.spec(:readonly) - @handler.establish_connection(spec) + @handler.establish_connection(spec.config.merge("name" => spec.name)) assert_not_nil @handler.retrieve_connection_pool('readonly') ensure diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 9ee92a3cd2..f25b85e8a7 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -27,7 +27,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = resolve_spec(:default_env, config) - expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost" } + expected = { "adapter"=>"postgresql", "database"=>"foo", "host"=>"localhost", "name"=>"default_env" } assert_equal expected, actual end @@ -37,7 +37,7 @@ module ActiveRecord config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = resolve_spec(:foo, config) - expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } + expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost","name"=>"foo" } assert_equal expected, actual end @@ -47,7 +47,7 @@ module ActiveRecord config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } } actual = resolve_spec(:foo, config) - expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost" } + expected = { "adapter" => "postgresql", "database" => "foo", "host" => "localhost","name"=>"foo" } assert_equal expected, actual end @@ -55,7 +55,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "postgres://localhost/foo" config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } actual = resolve_spec(:production, config) - expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost" } + expected = { "adapter"=>"not_postgres", "database"=>"not_foo", "host"=>"localhost", "name"=>"production" } assert_equal expected, actual end @@ -93,7 +93,7 @@ module ActiveRecord ENV['DATABASE_URL'] = "ibm-db://localhost/foo" config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } } actual = resolve_spec(:default_env, config) - expected = { "adapter"=>"ibm_db", "database"=>"foo", "host"=>"localhost" } + expected = { "adapter"=>"ibm_db", "database"=>"foo", "host"=>"localhost", "name"=>"default_env" } assert_equal expected, actual end diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb index 3bddaf32ec..b30a83d9ce 100644 --- a/activerecord/test/cases/connection_specification/resolver_test.rb +++ b/activerecord/test/cases/connection_specification/resolver_test.rb @@ -28,7 +28,8 @@ module ActiveRecord assert_equal({ "adapter" => "abstract", "host" => "foo", - "encoding" => "utf8" }, spec) + "encoding" => "utf8", + "name" => "production"}, spec) end def test_url_sub_key @@ -36,7 +37,8 @@ module ActiveRecord assert_equal({ "adapter" => "abstract", "host" => "foo", - "encoding" => "utf8" }, spec) + "encoding" => "utf8", + "name" => "production"}, spec) end def test_url_sub_key_merges_correctly @@ -46,7 +48,8 @@ module ActiveRecord "adapter" => "abstract", "host" => "foo", "encoding" => "utf8", - "pool" => "3" }, spec) + "pool" => "3", + "name" => "production"}, spec) end def test_url_host_no_db @@ -113,7 +116,8 @@ module ActiveRecord assert_equal({ "adapter" => "sqlite3", "database" => "foo", - "encoding" => "utf8" }, spec) + "encoding" => "utf8", + "name" => "production"}, spec) end def test_spec_name_on_key_lookup |