Make all reads on configuration_hash use methods

Convert all uses of `db_config.configuration_hash[*]` to use methods
defined on an implementation of `DatabaseConfigurations::DatabaseConfig`.

Since we want to get away from accessing properties directly on the
underlying configuration hash, we'll move here to accessing those values
via the implementations on `DatabaseConfig` (or more specifically,
`HashConfig`).

There are still codepaths that are passing around `configuration_hash`,
and follow-on PRs will address those with the goal of using
configuration objects everywhere up until the point we pass a resolved
hash over to the underlying client.

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

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

@@ -385,14 +385,9 @@ module ActiveRecord
 
         @spec = spec
 
-        @checkout_timeout = (spec.db_config.configuration_hash[:checkout_timeout] && spec.db_config.configuration_hash[:checkout_timeout].to_f) || 5
+        @checkout_timeout = spec.db_config.checkout_timeout
-        if @idle_timeout = spec.db_config.configuration_hash.fetch(:idle_timeout, 300)
+        @idle_timeout = spec.db_config.idle_timeout
-          @idle_timeout = @idle_timeout.to_f
+        @size = spec.db_config.pool
-          @idle_timeout = nil if @idle_timeout <= 0
-        end
-
-        # default max pool size to 5
-        @size = (spec.db_config.configuration_hash[:pool] && spec.db_config.configuration_hash[:pool].to_i) || 5
 
         # This variable tracks the cache of threads mapped to reserved connections, with the
         # sole purpose of speeding up the +connection+ method. It is not the authoritative
@@ -420,10 +415,7 @@ module ActiveRecord
 
         @lock_thread = false
 
-        # +reaping_frequency+ is configurable mostly for historical reasons, but it could
+        @reaper = Reaper.new(self, spec.db_config.reaping_frequency)
-        # also be useful if someone wants a very low +idle_timeout+.
-        reaping_frequency = spec.db_config.configuration_hash.fetch(:reaping_frequency, 60)
-        @reaper = Reaper.new(self, reaping_frequency && reaping_frequency.to_f)
         @reaper.run
       end
 
@@ -505,7 +497,7 @@ module ActiveRecord
       # Raises:
       # - ActiveRecord::ExclusiveConnectionTimeoutError if unable to gain ownership of all
       #   connections in the pool within a timeout interval (default duration is
-      #   <tt>spec.db_config.configuration_hash[:checkout_timeout] * 2</tt> seconds).
+      #   <tt>spec.db_config.checkout_timeout * 2</tt> seconds).
       def disconnect(raise_on_acquisition_timeout = true)
         with_exclusively_acquired_all_connections(raise_on_acquisition_timeout) do
           synchronize do
@@ -526,7 +518,7 @@ module ActiveRecord
       #
       # The pool first tries to gain ownership of all connections. If unable to
       # do so within a timeout interval (default duration is
-      # <tt>spec.db_config.configuration_hash[:checkout_timeout] * 2</tt> seconds), then the pool is forcefully
+      # <tt>spec.db_config.checkout_timeout * 2</tt> seconds), then the pool is forcefully
       # disconnected without any regard for other connection owning threads.
       def disconnect!
         disconnect(false)
@@ -557,7 +549,7 @@ module ActiveRecord
       # Raises:
       # - ActiveRecord::ExclusiveConnectionTimeoutError if unable to gain ownership of all
       #   connections in the pool within a timeout interval (default duration is
-      #   <tt>spec.db_config.configuration_hash[:checkout_timeout] * 2</tt> seconds).
+      #   <tt>spec.db_config.checkout_timeout * 2</tt> seconds).
       def clear_reloadable_connections(raise_on_acquisition_timeout = true)
         with_exclusively_acquired_all_connections(raise_on_acquisition_timeout) do
           synchronize do
@@ -579,7 +571,7 @@ module ActiveRecord
       #
       # The pool first tries to gain ownership of all connections. If unable to
       # do so within a timeout interval (default duration is
-      # <tt>spec.db_config.configuration_hash[:checkout_timeout] * 2</tt> seconds), then the pool forcefully
+      # <tt>spec.db_config.checkout_timeout * 2</tt> seconds), then the pool forcefully
       # clears the cache and reloads connections without any regard for other
       # connection owning threads.
       def clear_reloadable_connections!

activerecord/lib/active_record/database_configurations/database_config.rb

@@ -21,10 +21,30 @@ module ActiveRecord
         "#{adapter}_connection"
       end
 
+      def database
+        raise NotImplementedError
+      end
+
       def adapter
         raise NotImplementedError
       end
 
+      def pool
+        raise NotImplementedError
+      end
+
+      def checkout_timeout
+        raise NotImplementedError
+      end
+
+      def reaping_frequency
+        raise NotImplementedError
+      end
+
+      def idle_timeout
+        raise NotImplementedError
+      end
+
       def replica?
         raise NotImplementedError
       end

activerecord/lib/active_record/database_configurations/hash_config.rb

@@ -53,6 +53,29 @@ module ActiveRecord
         configuration_hash[:migrations_paths]
       end
 
+      def database
+        configuration_hash[:database]
+      end
+
+      def pool
+        configuration_hash.fetch(:pool, 5).to_i
+      end
+
+      def checkout_timeout
+        configuration_hash.fetch(:checkout_timeout, 5).to_f
+      end
+
+      # +reaping_frequency+ is configurable mostly for historical reasons, but it could
+      # also be useful if someone wants a very low +idle_timeout+.
+      def reaping_frequency
+        configuration_hash.fetch(:reaping_frequency, 60).to_f
+      end
+
+      def idle_timeout
+        timeout = configuration_hash.fetch(:idle_timeout, 300).to_f
+        timeout if timeout > 0
+      end
+
       def adapter
         configuration_hash[:adapter]
       end

activerecord/lib/active_record/tasks/database_tasks.rb

@@ -125,13 +125,13 @@ module ActiveRecord
 
       def create(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).create
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).create
-        $stdout.puts "Created database '#{db_config.configuration_hash[:database]}'" if verbose?
+        $stdout.puts "Created database '#{db_config.database}'" if verbose?
       rescue DatabaseAlreadyExists
-        $stderr.puts "Database '#{db_config.configuration_hash[:database]}' already exists" if verbose?
+        $stderr.puts "Database '#{db_config.database}' already exists" if verbose?
       rescue Exception => error
         $stderr.puts error
-        $stderr.puts "Couldn't create '#{db_config.configuration_hash[:database]}' database. Please check your configuration."
+        $stderr.puts "Couldn't create '#{db_config.database}' database. Please check your configuration."
         raise
       end
 
@@ -189,13 +189,13 @@ module ActiveRecord
 
       def drop(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).drop
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).drop
-        $stdout.puts "Dropped database '#{db_config.configuration_hash[:database]}'" if verbose?
+        $stdout.puts "Dropped database '#{db_config.database}'" if verbose?
       rescue ActiveRecord::NoDatabaseError
-        $stderr.puts "Database '#{db_config.configuration_hash[:database]}' does not exist"
+        $stderr.puts "Database '#{db_config.database}' does not exist"
       rescue Exception => error
         $stderr.puts error
-        $stderr.puts "Couldn't drop database '#{db_config.configuration_hash[:database]}'"
+        $stderr.puts "Couldn't drop database '#{db_config.database}'"
         raise
       end
 
@@ -274,7 +274,7 @@ module ActiveRecord
 
       def charset(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).charset
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).charset
       end
 
       def collation_current(env_name = env, spec_name = spec)
@@ -284,12 +284,12 @@ module ActiveRecord
 
       def collation(configuration, *arguments)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).collation
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).collation
       end
 
       def purge(configuration)
         db_config = resolve_configuration(configuration)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash).purge
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash).purge
       end
 
       def purge_all
@@ -304,13 +304,13 @@ module ActiveRecord
       def structure_dump(configuration, *arguments)
         db_config = resolve_configuration(configuration)
         filename = arguments.delete_at(0)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).structure_dump(filename, structure_dump_flags)
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).structure_dump(filename, structure_dump_flags)
       end
 
       def structure_load(configuration, *arguments)
         db_config = resolve_configuration(configuration)
         filename = arguments.delete_at(0)
-        class_for_adapter(db_config.configuration_hash[:adapter]).new(db_config.configuration_hash, *arguments).structure_load(filename, structure_load_flags)
+        class_for_adapter(db_config.adapter).new(db_config.configuration_hash, *arguments).structure_load(filename, structure_load_flags)
       end
 
       def load_schema(db_config, format = ActiveRecord::Base.schema_format, file = nil) # :nodoc:
@@ -493,12 +493,12 @@ module ActiveRecord
 
         def each_local_configuration
           ActiveRecord::Base.configurations.configs_for.each do |db_config|
-            next unless db_config.configuration_hash[:database]
+            next unless db_config.database
 
             if local_database?(db_config)
               yield db_config
             else
-              $stderr.puts "This task only modifies local databases. #{db_config.configuration_hash[:database]} is on a remote host."
+              $stderr.puts "This task only modifies local databases. #{db_config.database} is on a remote host."
             end
           end
         end

activerecord/test/cases/adapters/mysql2/schema_test.rb

@@ -11,7 +11,7 @@ module ActiveRecord
 
       def setup
         @connection = ActiveRecord::Base.connection
-        db          = Post.connection_pool.spec.db_config.configuration_hash[:database]
+        db          = Post.connection_pool.spec.db_config.database
         table       = Post.table_name
         @db_name    = db
 

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -63,13 +63,13 @@ module ActiveRecord
         @handler.establish_connection(:readonly)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("readonly")
-        assert_equal "db/readonly.sqlite3", pool.spec.db_config.configuration_hash[:database]
+        assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
 
         assert_not_nil pool = @handler.retrieve_connection_pool("primary")
-        assert_equal "db/primary.sqlite3", pool.spec.db_config.configuration_hash[:database]
+        assert_equal "db/primary.sqlite3", pool.spec.db_config.database
 
         assert_not_nil pool = @handler.retrieve_connection_pool("common")
-        assert_equal "db/common.sqlite3", pool.spec.db_config.configuration_hash[:database]
+        assert_equal "db/common.sqlite3", pool.spec.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
         ENV["RAILS_ENV"] = previous_env
@@ -93,7 +93,7 @@ module ActiveRecord
 
           ActiveRecord::Base.establish_connection
 
-          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.configuration_hash[:database]
+          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ENV["RAILS_ENV"] = previous_env
@@ -116,7 +116,7 @@ module ActiveRecord
 
           ActiveRecord::Base.establish_connection
 
-          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.configuration_hash[:database]
+          assert_match "db/primary.sqlite3", ActiveRecord::Base.connection.pool.spec.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ENV["RAILS_ENV"] = previous_env
@@ -132,7 +132,7 @@ module ActiveRecord
         @handler.establish_connection(:development)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("development")
-        assert_equal "db/primary.sqlite3", pool.spec.db_config.configuration_hash[:database]
+        assert_equal "db/primary.sqlite3", pool.spec.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
       end
@@ -147,7 +147,7 @@ module ActiveRecord
         @handler.establish_connection(:development_readonly)
 
         assert_not_nil pool = @handler.retrieve_connection_pool("development_readonly")
-        assert_equal "db/readonly.sqlite3", pool.spec.db_config.configuration_hash[:database]
+        assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
       ensure
         ActiveRecord::Base.configurations = @prev_configs
       end

activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb

@@ -82,10 +82,10 @@ module ActiveRecord
           ActiveRecord::Base.connects_to(database: { writing: :primary, reading: :readonly })
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:writing].retrieve_connection_pool("primary")
-          assert_equal "db/primary.sqlite3", pool.spec.db_config.configuration_hash[:database]
+          assert_equal "db/primary.sqlite3", pool.spec.db_config.database
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.configuration_hash[:database]
+          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)
@@ -140,10 +140,10 @@ module ActiveRecord
           ActiveRecord::Base.connects_to(database: { default: :primary, readonly: :readonly })
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:default].retrieve_connection_pool("primary")
-          assert_equal "db/primary.sqlite3", pool.spec.db_config.configuration_hash[:database]
+          assert_equal "db/primary.sqlite3", pool.spec.db_config.database
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:readonly].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.configuration_hash[:database]
+          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)
@@ -284,7 +284,7 @@ module ActiveRecord
           ActiveRecord::Base.connects_to database: { writing: :development, reading: :development_readonly }
 
           assert_not_nil pool = ActiveRecord::Base.connection_handlers[:reading].retrieve_connection_pool("primary")
-          assert_equal "db/readonly.sqlite3", pool.spec.db_config.configuration_hash[:database]
+          assert_equal "db/readonly.sqlite3", pool.spec.db_config.database
         ensure
           ActiveRecord::Base.configurations = @prev_configs
           ActiveRecord::Base.establish_connection(:arunit)

activerecord/test/cases/helper.rb

@@ -37,7 +37,7 @@ end
 
 def in_memory_db?
   current_adapter?(:SQLite3Adapter) &&
-  ActiveRecord::Base.connection_pool.spec.db_config.configuration_hash[:database] == ":memory:"
+  ActiveRecord::Base.connection_pool.spec.db_config.database == ":memory:"
 end
 
 def subsecond_precision_supported?

railties/test/application/rake/multi_dbs_test.rb

@@ -220,14 +220,14 @@ module ApplicationTests
       test "db:create and db:drop works on all databases for env" do
         require "#{app_path}/config/environment"
         ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
-          db_create_and_drop db_config.spec_name, db_config.configuration_hash[:database]
+          db_create_and_drop db_config.spec_name, db_config.database
         end
       end
 
       test "db:create:namespace and db:drop:namespace works on specified databases" do
         require "#{app_path}/config/environment"
         ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
-          db_create_and_drop_namespace db_config.spec_name, db_config.configuration_hash[:database]
+          db_create_and_drop_namespace db_config.spec_name, db_config.database
         end
       end
 
@@ -359,7 +359,7 @@ module ApplicationTests
         db_migrate_and_schema_dump_and_load "schema"
 
         app_file "db/seeds.rb", <<-RUBY
-          print Book.connection.pool.spec.db_config.configuration_hash[:database]
+          print Book.connection.pool.spec.db_config.database
         RUBY
 
         output = rails("db:seed")