Pass db_config object around instead of hashes

This is part 1 of removing the need to pass hashes around inside
connection management. This PR creates database config objects every
time when passing a hash, string, or symbol and sends that object to
connection specification. ConnectionSpecification reaches into the
config hash on db_config when needed, but eventually we'll get rid of
that and ConnectionSpecification since it's doing duplicate work with
the DatabaseConfigurations.

We also chose to change the keys to strings because that's what the
database.yml would create and what apps currently expect. While symbols
are nicer, we'd end up having to deprecate the string behavior first.

Co-authored-by: eileencodes <eileencodes@gmail.com>

activerecord/lib/active_record/connection_adapters/connection_specification.rb

@@ -5,22 +5,22 @@
 module ActiveRecord
   module ConnectionAdapters
     class ConnectionSpecification #:nodoc:
-      attr_reader :name, :adapter_method
+      attr_reader :name, :adapter_method, :db_config
 
-      def initialize(name, config, adapter_method)
-        @name, @config, @adapter_method = name, config, adapter_method
+      def initialize(name, db_config, adapter_method)
+        @name, @db_config, @adapter_method = name, db_config, adapter_method
       end
 
       def underlying_configuration_hash
-        @config
+        @db_config.configuration_hash
       end
 
       def initialize_dup(original)
-        @config = original.underlying_configuration_hash.dup
+        @db_config = original.db_config.dup
       end
 
       def to_hash
-        @config.merge(name: @name)
+        underlying_configuration_hash.dup.merge(name: @name)
       end
 
       # Expands a connection string into a hash.
@@ -124,30 +124,6 @@ def initialize(configurations)
           @configurations = configurations
         end
 
-        # Returns a hash with database connection information.
-        #
-        # == Examples
-        #
-        # Full hash Configuration.
-        #
-        #   configurations = { "production" => { "host" => "localhost", "database" => "foo", "adapter" => "sqlite3" } }
-        #   Resolver.new(configurations).resolve(:production)
-        #   # => { host: "localhost", database: "foo", adapter: "sqlite3"}
-        #
-        # Initialized with URL configuration strings.
-        #
-        #   configurations = { "production" => "postgresql://localhost/foo" }
-        #   Resolver.new(configurations).resolve(:production)
-        #   # => { host: "localhost", database: "foo", adapter: "postgresql" }
-        #
-        def resolve(config_or_env, pool_name = nil)
-          if config_or_env
-            resolve_connection config_or_env, pool_name
-          else
-            raise AdapterNotSpecified
-          end
-        end
-
         # Returns an instance of ConnectionSpecification for a given adapter.
         # Accepts a hash one layer deep that contains all connection information.
         #
@@ -163,7 +139,8 @@ def resolve(config_or_env, pool_name = nil)
         def spec(config)
           pool_name = config if config.is_a?(Symbol)
 
-          spec = resolve(config, pool_name).symbolize_keys
+          db_config = resolve(config, pool_name)
+          spec = db_config.configuration_hash
 
           raise(AdapterNotSpecified, "database configuration does not specify adapter") unless spec.key?(:adapter)
 
@@ -194,43 +171,47 @@ def spec(config)
             raise AdapterNotFound, "database configuration specifies nonexistent #{spec[:adapter]} adapter"
           end
 
-          ConnectionSpecification.new(spec.delete(:name) || "primary", spec, adapter_method)
+          ConnectionSpecification.new(spec.delete(:name) || "primary", db_config, adapter_method)
         end
 
-        private
         # Returns fully resolved connection, accepts hash, string or symbol.
-          # Always returns a hash.
+        # Always returns a DatabaseConfiguration::DatabaseConfig
         #
         # == Examples
         #
         # Symbol representing current environment.
         #
-          #   Resolver.new("production" => {}).resolve_connection(:production)
-          #   # => {}
+        #   Resolver.new("production" => {}).resolve(:production)
+        #   # => DatabaseConfigurations::HashConfig.new(env_name: "production", config: {})
         #
         # One layer deep hash of connection values.
         #
-          #   Resolver.new({}).resolve_connection("adapter" => "sqlite3")
-          #   # => { adapter: "sqlite3" }
+        #   Resolver.new({}).resolve("adapter" => "sqlite3")
+        #   # => DatabaseConfigurations::HashConfig.new(config: {"adapter" => "sqlite3"})
         #
         # Connection URL.
         #
-          #   Resolver.new({}).resolve_connection("postgresql://localhost/foo")
-          #   # => { host: "localhost", database: "foo", adapter: "postgresql" }
+        #   Resolver.new({}).resolve("postgresql://localhost/foo")
+        #   # => DatabaseConfigurations::UrlConfig.new(config: {"adapter" => "postgresql", "host" => "localhost", "database" => "foo"})
         #
-          def resolve_connection(config_or_env, pool_name = nil)
+        def resolve(config_or_env, pool_name = nil)
+          env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_s
+
           case config_or_env
           when Symbol
-              resolve_symbol_connection config_or_env, pool_name
+            resolve_symbol_connection(config_or_env, pool_name)
           when String
-              resolve_url_connection config_or_env
+            DatabaseConfigurations::UrlConfig.new(env, "primary", config_or_env)
           when Hash
-              resolve_hash_connection config_or_env
+            DatabaseConfigurations::HashConfig.new(env, "primary", config_or_env)
+          when DatabaseConfigurations::DatabaseConfig
+            config_or_env
           else
             raise TypeError, "Invalid type for configuration. Expected Symbol, String, or Hash. Got #{config_or_env.inspect}"
           end
         end
 
+        private
           # Takes the environment such as +:production+ or +:development+ and a
           # pool name the corresponds to the name given by the connection pool
           # to the connection. That pool name is merged into the hash with the
@@ -246,12 +227,13 @@ def resolve_connection(config_or_env, pool_name = nil)
           #       ]>
           #
           #   Resolver.new(configurations).resolve_symbol_connection(:production, "primary")
-          #   # => { database: "my_db" }
+          #   # => DatabaseConfigurations::HashConfig(config: database: "my_db", env_name: "production", spec_name: "primary")
           def resolve_symbol_connection(env_name, pool_name)
             db_config = configurations.find_db_config(env_name)
 
             if db_config
-              resolve_connection(db_config.configuration_hash).merge(name: pool_name.to_s)
+              config = db_config.configuration_hash.merge(name: pool_name.to_s)
+              DatabaseConfigurations::HashConfig.new(db_config.env_name, db_config.spec_name, config)
             else
               raise AdapterNotSpecified, <<~MSG
                 The `#{env_name}` database is not configured for the `#{ActiveRecord::ConnectionHandling::DEFAULT_ENV.call}` environment.
@@ -275,27 +257,6 @@ def build_configuration_sentence # :nodoc:
               end
             end.join("\n")
           end
-
-          # Accepts a hash. Expands the "url" key that contains a
-          # URL database connection to a full connection
-          # hash and merges with the rest of the hash.
-          # Connection details inside of the "url" key win any merge conflicts
-          def resolve_hash_connection(spec)
-            if spec[:url] && !spec[:url].match?(/^jdbc:/)
-              connection_hash = resolve_url_connection(spec.delete(:url))
-              spec.merge!(connection_hash)
-            end
-            spec
-          end
-
-          # Takes a connection URL.
-          #
-          #   Resolver.new({}).resolve_url_connection("postgresql://localhost/foo")
-          #   # => { host: "localhost", database: "foo", adapter: "postgresql" }
-          #
-          def resolve_url_connection(url)
-            ConnectionUrlResolver.new(url).to_hash
-          end
       end
     end
   end

activerecord/lib/active_record/connection_handling.rb

@@ -184,7 +184,7 @@ def resolve_config_for_connection(config_or_env) # :nodoc:
       self.connection_specification_name = pool_name
 
       resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(Base.configurations)
-      config_hash = resolver.resolve(config_or_env, pool_name).symbolize_keys
+      config_hash = resolver.resolve(config_or_env, pool_name).configuration_hash
       config_hash[:name] = pool_name
 
       config_hash

activerecord/lib/active_record/database_configurations/database_config.rb

@@ -18,6 +18,10 @@ def config
         configuration_hash.stringify_keys
       end
 
+      def initialize_dup(original)
+        @config = original.configuration_hash.dup
+      end
+
       def replica?
         raise NotImplementedError
       end

activerecord/lib/active_record/database_configurations/hash_config.rb

@@ -28,6 +28,8 @@ class HashConfig < DatabaseConfig
       def initialize(env_name, spec_name, config)
         super(env_name, spec_name)
         @config = config.symbolize_keys
+
+        resolve_url_key
       end
 
       def configuration_hash
@@ -47,6 +49,14 @@ def replica?
       def migrations_paths
         configuration_hash[:migrations_paths]
       end
+
+      private
+        def resolve_url_key
+          if configuration_hash[:url] && !configuration_hash[:url].match?(/^jdbc:/)
+            connection_hash = ActiveRecord::ConnectionAdapters::ConnectionSpecification::ConnectionUrlResolver.new(configuration_hash[:url]).to_hash
+            configuration_hash.merge!(connection_hash)
+          end
+        end
     end
   end
 end

activerecord/test/cases/connection_adapters/adapter_leasing_test.rb

@@ -39,7 +39,8 @@ def test_expire_mutates_in_use
       end
 
       def test_close
-        pool = Pool.new(ConnectionSpecification.new("primary", {}, nil))
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("test", "primary", {})
+        pool = Pool.new(ConnectionSpecification.new("primary", db_config, nil))
         pool.insert_connection_for_test! @adapter
         @adapter.pool = pool
 

activerecord/test/cases/connection_adapters/connection_specification_test.rb

@@ -6,7 +6,8 @@ module ActiveRecord
   module ConnectionAdapters
     class ConnectionSpecificationTest < ActiveRecord::TestCase
       def test_dup_deep_copy_config
-        spec = ConnectionSpecification.new("primary", { a: :b }, "bar")
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("development", "primary", { a: :b })
+        spec = ConnectionSpecification.new("primary", db_config, "bar")
         assert_not_equal(spec.underlying_configuration_hash.object_id, spec.dup.underlying_configuration_hash.object_id)
       end
     end

activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb

@@ -25,7 +25,7 @@ def resolve_config(config)
       def resolve_spec(spec, config)
         configs = ActiveRecord::DatabaseConfigurations.new(config)
         resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(configs)
-        resolver.resolve(spec, spec)
+        resolver.resolve(spec, spec).configuration_hash
       end
 
       def test_invalid_string_config

activerecord/test/cases/connection_specification/resolver_test.rb

@@ -9,7 +9,7 @@ class ResolverTest < ActiveRecord::TestCase
         def resolve(spec, config = {})
           configs = ActiveRecord::DatabaseConfigurations.new(config)
           resolver = ConnectionAdapters::ConnectionSpecification::Resolver.new(configs)
-          resolver.resolve(spec, spec)
+          resolver.resolve(spec, spec).configuration_hash
         end
 
         def spec(spec, config = {})