Merge pull request #38029 from seejohnrun/freeze-configuration_hash

Don't allow mutations on configuration_hash

activerecord/lib/active_record/database_configurations/hash_config.rb

@@ -36,7 +36,7 @@ def config
       end
 
       def configuration_hash
-        @config
+        @config.freeze
       end
 
       # Determines whether a database configuration is for a replica / readonly

activerecord/lib/active_record/test_databases.rb

@@ -5,18 +5,25 @@
 module ActiveRecord
   module TestDatabases # :nodoc:
     ActiveSupport::Testing::Parallelization.after_fork_hook do |i|
-      create_and_load_schema(i, env_name: Rails.env)
+      create_and_load_schema(i, env_name: ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_sym)
     end
 
     def self.create_and_load_schema(i, env_name:)
       old, ENV["VERBOSE"] = ENV["VERBOSE"], "false"
 
       ActiveRecord::Base.configurations.configs_for(env_name: env_name).each do |db_config|
-        db_config.configuration_hash[:database] += "-#{i}"
-        ActiveRecord::Tasks::DatabaseTasks.reconstruct_from_schema(db_config, ActiveRecord::Base.schema_format, nil)
+        database = "#{db_config.database}-#{i}"
+
+        db_config_copy = ActiveRecord::DatabaseConfigurations::HashConfig.new(
+          env_name,
+          db_config.spec_name,
+          db_config.configuration_hash.merge(database: database)
+        )
+
+        ActiveRecord::Tasks::DatabaseTasks.reconstruct_from_schema(db_config_copy, ActiveRecord::Base.schema_format, nil)
       end
     ensure
-      ActiveRecord::Base.establish_connection(Rails.env.to_sym)
+      ActiveRecord::Base.establish_connection(ActiveRecord::ConnectionHandling::DEFAULT_ENV.call.to_sym)
       ENV["VERBOSE"] = old
     end
   end

activerecord/test/cases/connection_adapters/connection_handler_test.rb

@@ -385,8 +385,8 @@ def test_pool_from_any_process_for_uses_most_recent_spec
           wr.binmode
 
           pid = fork do
-            ActiveRecord::Base.configurations["arunit"]["database"] = file.path
-            ActiveRecord::Base.establish_connection(:arunit)
+            config_hash = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary").configuration_hash.merge(database: file.path)
+            ActiveRecord::Base.establish_connection(config_hash)
 
             pid2 = fork do
               wr.write ActiveRecord::Base.connection_db_config.database

activerecord/test/cases/connection_pool_test.rb

@@ -200,8 +200,11 @@ def test_reap_inactive
 
       def test_idle_timeout_configuration
         @pool.disconnect!
-        @db_config.configuration_hash.merge!(idle_timeout: "0.02")
-        pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", @db_config)
+
+        config = @db_config.configuration_hash.merge(idle_timeout: "0.02")
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new(@db_config.env_name, @db_config.spec_name, config)
+
+        pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", db_config)
         @pool = ConnectionPool.new(pool_config)
         idle_conn = @pool.checkout
         @pool.checkin(idle_conn)
@@ -225,8 +228,10 @@ def test_idle_timeout_configuration
 
       def test_disable_flush
         @pool.disconnect!
-        @db_config.configuration_hash.merge!(idle_timeout: -5)
-        pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", @db_config)
+
+        config = @db_config.configuration_hash.merge(idle_timeout: -5)
+        db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new(@db_config.env_name, @db_config.spec_name, config)
+        pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", db_config)
         @pool = ConnectionPool.new(pool_config)
         idle_conn = @pool.checkout
         @pool.checkin(idle_conn)
@@ -720,12 +725,10 @@ def test_public_connections_access_threadsafe
 
       private
         def with_single_connection_pool
-          old_config = @db_config.configuration_hash
-          db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup)
-
-          db_config.configuration_hash[:pool] = 1 # this is safe to do, because .dupped PoolConfig also auto-dups its config
-
+          config = @db_config.configuration_hash.merge(pool: 1)
+          db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", config)
           pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new("primary", db_config)
+
           yield(pool = ConnectionPool.new(pool_config))
         ensure
           pool.disconnect! if pool

activerecord/test/cases/reaper_test.rb

@@ -68,9 +68,7 @@ def test_pool_has_reaper
       end
 
       def test_reaping_frequency_configuration
-        pool_config = duplicated_pool_config
-        pool_config.db_config.configuration_hash[:reaping_frequency] = "10.01"
-
+        pool_config = duplicated_pool_config(reaping_frequency: "10.01")
         pool = ConnectionPool.new(pool_config)
 
         assert_equal 10.01, pool.reaper.frequency
@@ -79,9 +77,7 @@ def test_reaping_frequency_configuration
       end
 
       def test_connection_pool_starts_reaper
-        pool_config = duplicated_pool_config
-        pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001"
-
+        pool_config = duplicated_pool_config(reaping_frequency: "0.0001")
         pool = ConnectionPool.new(pool_config)
 
         conn, child = new_conn_in_thread(pool)
@@ -97,8 +93,7 @@ def test_connection_pool_starts_reaper
       end
 
       def test_reaper_works_after_pool_discard
-        pool_config = duplicated_pool_config
-        pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001"
+        pool_config = duplicated_pool_config(reaping_frequency: "0.0001")
 
         2.times do
           pool = ConnectionPool.new(pool_config)
@@ -129,9 +124,7 @@ def test_reap_flush_on_discarded_pool
 
       if Process.respond_to?(:fork)
         def test_connection_pool_starts_reaper_in_fork
-          pool_config = duplicated_pool_config
-          pool_config.db_config.configuration_hash[:reaping_frequency] = "0.0001"
-
+          pool_config = duplicated_pool_config(reaping_frequency: "0.0001")
           pool = ConnectionPool.new(pool_config)
           pool.checkout
 
@@ -175,8 +168,8 @@ def test_reaper_does_not_reap_discarded_connection_pools
       end
 
       private
-        def duplicated_pool_config
-          old_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash
+        def duplicated_pool_config(merge_config_options = {})
+          old_config = ActiveRecord::Base.connection_pool.db_config.configuration_hash.merge(merge_config_options)
           db_config = ActiveRecord::DatabaseConfigurations::HashConfig.new("arunit", "primary", old_config.dup)
           PoolConfig.new("primary", db_config)
         end

activerecord/test/cases/test_databases_test.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require "cases/helper"
+
+class TestDatabasesTest < ActiveRecord::TestCase
+  unless in_memory_db?
+    def test_databases_are_created
+      previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "arunit"
+
+      base_db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", spec_name: "primary")
+      expected_database = "#{base_db_config.database}-2"
+
+      ActiveRecord::Tasks::DatabaseTasks.stub(:reconstruct_from_schema, ->(db_config, _, _) {
+        assert_equal expected_database, db_config.database
+      }) do
+        ActiveRecord::TestDatabases.create_and_load_schema(2, env_name: "arunit")
+      end
+    ensure
+      ENV["RAILS_ENV"] = previous_env
+    end
+  end
+end