Move `name` key on configuration hash into `DatabaseConfig`

`name` is used by Rails to find the configuration by connection
specification name, but database adapters don't need to use `name` in
order to establish a connection. This is part of our work to separate
what the database needs to connect (the configuration hash) and the
what Rails needs to find connections (everything else).
Co-authored-by: NJohn Crepezzi <john.crepezzi@gmail.com>

activerecord/CHANGELOG.md

+*   The `:name` key will no longer be returned as part of `DatabaseConfig#configuration_hash`. Please use `DatabaseConfig#owner_name` instead.
+
+    *Eileen M. Uchitelle*, *John Crepezzi*
+
 *   ActiveRecord's `belongs_to_required_by_default` flag can now be set per model.
 
     You can now opt-out/opt-in specific models from having their associations required
     by default.
+
     This change is meant to ease the process of migrating all your models to have
     their association required.
 

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -1188,7 +1188,9 @@ def resolve_pool_config(config)
             raise AdapterNotFound, "database configuration specifies nonexistent #{db_config.adapter} adapter"
           end
 
-          ConnectionAdapters::PoolConfig.new(db_config.configuration_hash.delete(:name) || "primary", db_config)
+          pool_name = db_config.owner_name || "primary"
+          db_config.owner_name = nil
+          ConnectionAdapters::PoolConfig.new(pool_name, db_config)
         end
     end
   end

activerecord/lib/active_record/connection_handling.rb

@@ -253,7 +253,7 @@ def resolve_config_for_connection(config_or_env)
         self.connection_specification_name = pool_name
 
         db_config = Base.configurations.resolve(config_or_env, pool_name)
-        db_config.configuration_hash[:name] = pool_name
+        db_config.owner_name = pool_name
         db_config
       end
 

activerecord/lib/active_record/database_configurations.rb

@@ -182,8 +182,10 @@ def resolve_symbol_connection(env_name, pool_name)
         db_config = find_db_config(env_name)
 
         if db_config
-          config = db_config.configuration_hash.merge(name: pool_name.to_s)
-          DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
+          config = db_config.configuration_hash.dup
+          db_config = DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
+          db_config.owner_name = pool_name.to_s
+          db_config
         else
           raise AdapterNotSpecified, <<~MSG
             The `#{env_name}` database is not configured for the `#{default_env}` environment.

activerecord/lib/active_record/database_configurations/database_config.rb

@@ -7,6 +7,7 @@ class DatabaseConfigurations
     # as this is the parent class for the types of database configuration objects.
     class DatabaseConfig # :nodoc:
       attr_reader :env_name, :spec_name
+      attr_accessor :owner_name
 
       def initialize(env_name, spec_name)
         @env_name = env_name

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -31,9 +31,9 @@ def test_establish_connection_uses_config_hash_with_spec_name
         old_config = ActiveRecord::Base.configurations
         config = { "readonly" => { "adapter" => "sqlite3", "pool" => "5" } }
         ActiveRecord::Base.configurations = config
-        config_hash = ActiveRecord::Base.configurations.resolve(config["readonly"], "readonly").configuration_hash
-        config_hash[:name] = "readonly"
-        @handler.establish_connection(config_hash)
+        db_config = ActiveRecord::Base.configurations.resolve(config["readonly"], "readonly")
+        db_config.owner_name = "readonly"
+        @handler.establish_connection(db_config)
 
         assert_not_nil @handler.retrieve_connection_pool("readonly")
       ensure

activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb

@@ -22,9 +22,9 @@ def resolve_config(config, env_name = ActiveRecord::ConnectionHandling::DEFAULT_
         configs.configs_for(env_name: env_name, spec_name: "primary")&.configuration_hash
       end
 
-      def resolve_spec(spec, config)
+      def resolve_db_config(spec, config)
         configs = ActiveRecord::DatabaseConfigurations.new(config)
-        configs.resolve(spec, spec).configuration_hash
+        configs.resolve(spec, spec)
       end
 
       def test_invalid_string_config
@@ -45,72 +45,85 @@ def test_invalid_symbol_config
 
       def test_resolver_with_database_uri_and_current_env_symbol_key
         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", name: "default_env" }
-        assert_equal expected, actual
+        config = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
+        actual = resolve_db_config(:default_env, config)
+        expected = { adapter: "postgresql", database: "foo", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "default_env", actual.owner_name
       end
 
       def test_resolver_with_database_uri_and_current_env_symbol_key_and_rails_env
         ENV["DATABASE_URL"] = "postgres://localhost/foo"
         ENV["RAILS_ENV"]    = "foo"
 
-        config   = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } }
-        actual   = resolve_spec(:foo, config)
-        expected = { adapter: "postgresql", database: "foo", host: "localhost", name: "foo" }
-        assert_equal expected, actual
+        config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } }
+        actual = resolve_db_config(:foo, config)
+        expected = { adapter: "postgresql", database: "foo", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "foo", actual.owner_name
       end
 
       def test_resolver_with_nil_database_url_and_current_env
         ENV["RAILS_ENV"] = "foo"
         config = { "foo" => { "adapter" => "postgres", "url" => ENV["DATABASE_URL"] } }
-        actual   = resolve_spec(:foo, config)
-        expected = { adapter: "postgres", url: nil, name: "foo" }
-        assert_equal expected, actual
+        actual = resolve_db_config(:foo, config)
+        expected_config = { adapter: "postgres", url: nil }
+
+        assert_equal expected_config, actual.configuration_hash
+        assert_equal "foo", actual.owner_name
       end
 
       def test_resolver_with_database_uri_and_current_env_symbol_key_and_rack_env
         ENV["DATABASE_URL"] = "postgres://localhost/foo"
         ENV["RACK_ENV"]     = "foo"
 
-        config   = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } }
-        actual   = resolve_spec(:foo, config)
-        expected = { adapter: "postgresql", database: "foo", host: "localhost", name: "foo" }
-        assert_equal expected, actual
+        config = { "not_production" => { "adapter" => "not_postgres", "database" => "not_foo" } }
+        actual = resolve_db_config(:foo, config)
+        expected = { adapter: "postgresql", database: "foo", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "foo", actual.owner_name
       end
 
       def test_resolver_with_database_uri_and_known_key
         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", name: "production" }
-        assert_equal expected, actual
+        config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } }
+        actual = resolve_db_config(:production, config)
+        expected = { adapter: "not_postgres", database: "not_foo", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "production", actual.owner_name
       end
 
       def test_resolver_with_database_uri_and_multiple_envs
         ENV["DATABASE_URL"] = "postgres://localhost"
         ENV["RAILS_ENV"] = "test"
 
-        config   = { "production" => { "adapter" => "postgresql", "database" => "foo_prod" }, "test" => { "adapter" => "postgresql", "database" => "foo_test" } }
-        actual   = resolve_spec(:test, config)
-        expected = { adapter: "postgresql", database: "foo_test", host: "localhost", name: "test" }
-        assert_equal expected, actual
+        config = { "production" => { "adapter" => "postgresql", "database" => "foo_prod" }, "test" => { "adapter" => "postgresql", "database" => "foo_test" } }
+        actual = resolve_db_config(:test, config)
+        expected = { adapter: "postgresql", database: "foo_test", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "test", actual.owner_name
       end
 
       def test_resolver_with_database_uri_and_unknown_symbol_key
         ENV["DATABASE_URL"] = "postgres://localhost/foo"
         config = { "not_production" => {  "adapter" => "not_postgres", "database" => "not_foo" } }
         assert_raises AdapterNotSpecified do
-          resolve_spec(:production, config)
+          resolve_db_config(:production, config)
         end
       end
 
       def test_resolver_with_database_uri_and_supplied_url
         ENV["DATABASE_URL"] = "not-postgres://not-localhost/not_foo"
-        config   = { "production" => {  "adapter" => "also_not_postgres", "database" => "also_not_foo" } }
-        actual   = resolve_spec("postgres://localhost/foo", config)
+        config = { "production" => {  "adapter" => "also_not_postgres", "database" => "also_not_foo" } }
+        actual = resolve_db_config("postgres://localhost/foo", config)
         expected = { adapter: "postgresql", database: "foo", host: "localhost" }
-        assert_equal expected, actual
+
+        assert_equal expected, actual.configuration_hash
       end
 
       def test_jdbc_url
@@ -134,10 +147,12 @@ def test_environment_does_not_exist_in_config_url_does_exist
 
       def test_url_with_hyphenated_scheme
         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", name: "default_env" }
-        assert_equal expected, actual
+        config = { "default_env" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" } }
+        actual = resolve_db_config(:default_env, config)
+        expected = { adapter: "ibm_db", database: "foo", host: "localhost" }
+
+        assert_equal expected, actual.configuration_hash
+        assert_equal "default_env", actual.owner_name
       end
 
       def test_string_connection
@@ -165,10 +180,10 @@ def test_url_sub_key
       end
 
       def test_url_removed_from_hash
-        config   = { "default_env" => { "url" => "postgres://localhost/foo" } }
-        actual   = resolve_spec(:default_env, config)
+        config = { "default_env" => { "url" => "postgres://localhost/foo" } }
+        actual = resolve_db_config(:default_env, config)
 
-        assert_not_includes actual, :url
+        assert_not_includes actual.configuration_hash, :url
       end
 
       def test_url_with_equals_in_query_value
@@ -400,16 +415,18 @@ def test_does_not_change_other_environments
         ENV["DATABASE_URL"] = "postgres://localhost/foo"
         config = { "production" => { "adapter" => "not_postgres", "database" => "not_foo", "host" => "localhost" }, "default_env" => {} }
 
-        actual = resolve_spec(:production, config)
-        assert_equal config["production"].symbolize_keys.merge(name: "production"), actual
+        actual = resolve_db_config(:production, config)
+        assert_equal config["production"].symbolize_keys, actual.configuration_hash
+        assert_equal "production", actual.owner_name
+
+        actual = resolve_db_config(:default_env, config)
 
-        actual = resolve_spec(:default_env, config)
         assert_equal({
           host: "localhost",
           database: "foo",
           adapter: "postgresql",
-          name: "default_env"
-        }, actual)
+        }, actual.configuration_hash)
+        assert_equal "default_env", actual.owner_name
       end
     end
   end

activerecord/test/cases/database_configurations/resolver_test.rb

@@ -6,9 +6,9 @@ module ActiveRecord
   module ConnectionAdapters
     class PoolConfig
       class ResolverTest < ActiveRecord::TestCase
-        def resolve(pool_config, config = {})
+        def resolve_db_config(pool_config, config = {})
           configs = ActiveRecord::DatabaseConfigurations.new(config)
-          configs.resolve(pool_config, pool_config).configuration_hash
+          configs.resolve(pool_config, pool_config)
         end
 
         def test_url_invalid_adapter
@@ -31,107 +31,116 @@ def test_error_if_no_adapter_method
         # checks that the adapter file can be required in.
 
         def test_url_from_environment
-          pool_config = resolve :production, "production" => "abstract://foo?encoding=utf8"
+          pool_config = resolve_db_config :production, "production" => "abstract://foo?encoding=utf8"
+
           assert_equal({
             adapter:  "abstract",
             host:     "foo",
-            encoding: "utf8",
-            name:     "production"
-          }, pool_config)
+            encoding: "utf8"
+          }, pool_config.configuration_hash)
+          assert_equal "production", pool_config.owner_name
         end
 
         def test_url_sub_key
-          pool_config = resolve :production, "production" => { "url" => "abstract://foo?encoding=utf8" }
+          pool_config = resolve_db_config :production, "production" => { "url" => "abstract://foo?encoding=utf8" }
+
           assert_equal({
             adapter:  "abstract",
             host:     "foo",
-            encoding: "utf8",
-            name:     "production"
-          }, pool_config)
+            encoding: "utf8"
+          }, pool_config.configuration_hash)
+          assert_equal "production", pool_config.owner_name
         end
 
         def test_url_sub_key_merges_correctly
           hash = { "url" => "abstract://foo?encoding=utf8&", "adapter" => "sqlite3", "host" => "bar", "pool" => "3" }
-          pool_config = resolve :production, "production" => hash
+          pool_config = resolve_db_config :production, "production" => hash
+
           assert_equal({
             adapter:  "abstract",
             host:     "foo",
             encoding: "utf8",
-            pool:     "3",
-            name:     "production"
-          }, pool_config)
+            pool:     "3"
+          }, pool_config.configuration_hash)
+          assert_equal "production", pool_config.owner_name
         end
 
         def test_url_host_no_db
-          pool_config = resolve "abstract://foo?encoding=utf8"
+          pool_config = resolve_db_config "abstract://foo?encoding=utf8"
           assert_equal({
             adapter:  "abstract",
             host:     "foo",
             encoding: "utf8"
-          }, pool_config)
+          }, pool_config.configuration_hash)
         end
 
         def test_url_missing_scheme
           assert_raises ActiveRecord::DatabaseConfigurations::InvalidConfigurationError do
-            resolve "foo"
+            resolve_db_config "foo"
           end
         end
 
         def test_url_host_db
-          pool_config = resolve "abstract://foo/bar?encoding=utf8"
+          pool_config = resolve_db_config "abstract://foo/bar?encoding=utf8"
           assert_equal({
             adapter:  "abstract",
             database: "bar",
             host:     "foo",
             encoding: "utf8"
-          }, pool_config)
+          }, pool_config.configuration_hash)
         end
 
         def test_url_port
-          pool_config = resolve "abstract://foo:123?encoding=utf8"
+          pool_config = resolve_db_config "abstract://foo:123?encoding=utf8"
+
           assert_equal({
             adapter:  "abstract",
             port:     123,
             host:     "foo",
             encoding: "utf8"
-          }, pool_config)
+          }, pool_config.configuration_hash)
         end
 
         def test_encoded_password
           password = "am@z1ng_p@ssw0rd#!"
           encoded_password = URI.encode_www_form_component(password)
-          pool_config = resolve "abstract://foo:#{encoded_password}@localhost/bar"
-          assert_equal password, pool_config[:password]
+          pool_config = resolve_db_config "abstract://foo:#{encoded_password}@localhost/bar"
+
+          assert_equal password, pool_config.configuration_hash[:password]
         end
 
         def test_url_with_authority_for_sqlite3
-          pool_config = resolve "sqlite3:///foo_test"
-          assert_equal("/foo_test", pool_config[:database])
+          pool_config = resolve_db_config "sqlite3:///foo_test"
+
+          assert_equal("/foo_test", pool_config.database)
         end
 
         def test_url_absolute_path_for_sqlite3
-          pool_config = resolve "sqlite3:/foo_test"
-          assert_equal("/foo_test", pool_config[:database])
+          pool_config = resolve_db_config "sqlite3:/foo_test"
+
+          assert_equal("/foo_test", pool_config.database)
         end
 
         def test_url_relative_path_for_sqlite3
-          pool_config = resolve "sqlite3:foo_test"
-          assert_equal("foo_test", pool_config[:database])
+          pool_config = resolve_db_config "sqlite3:foo_test"
+
+          assert_equal("foo_test", pool_config.database)
         end
 
         def test_url_memory_db_for_sqlite3
-          pool_config = resolve "sqlite3::memory:"
-          assert_equal(":memory:", pool_config[:database])
+          pool_config = resolve_db_config "sqlite3::memory:"
+          assert_equal(":memory:", pool_config.database)
         end
 
         def test_url_sub_key_for_sqlite3
-          pool_config = resolve :production, "production" => { "url" => "sqlite3:foo?encoding=utf8" }
+          pool_config = resolve_db_config :production, "production" => { "url" => "sqlite3:foo?encoding=utf8" }
+
           assert_equal({
             adapter:  "sqlite3",
             database: "foo",
-            encoding: "utf8",
-            name:     "production"
-          }, pool_config)
+            encoding: "utf8"
+          }, pool_config.configuration_hash)
+          assert_equal "production", pool_config.owner_name
         end
 
         def test_pool_config_with_invalid_type

activerecord/test/cases/query_cache_test.rb

@@ -444,17 +444,15 @@ def test_cache_is_available_when_connection_is_connected
   def test_cache_is_available_when_using_a_not_connected_connection
     skip "In-Memory DB can't test for using a not connected connection" if in_memory_db?
     with_temporary_connection_pool do
-      spec_name = Task.connection_specification_name
-      conf = ActiveRecord::Base.configurations["arunit"].merge("name" => "test2")
-      ActiveRecord::Base.connection_handler.establish_connection(conf)
-      Task.connection_specification_name = "test2"
+      db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary").dup
+      db_config.owner_name = "test2"
+      ActiveRecord::Base.connection_handler.establish_connection(db_config)
       assert_not_predicate Task, :connected?
 
       Task.cache do
         assert_queries(1) { Task.find(1); Task.find(1) }
       ensure
-        ActiveRecord::Base.connection_handler.remove_connection(Task.connection_specification_name)
-        Task.connection_specification_name = spec_name
+        ActiveRecord::Base.connection_handler.remove_connection(db_config.owner_name)
       end
     end
   end