Skip to content

Commit 901828f

Browse files
committed
Move calls on Base connection to methods for rake tasks
This PR aims to contain calls to `Base.connection` and `Base.establish_connection` in active record rake tasks and the migration code. In a follow up PR I will swap out the `Base` class for a temporary class which will allow us to stop clobbering `Base` in the active record rails tasks. This work is an important step to achieve moving away from Active Record's dependence on `Base.connection` and `Base.establish_connection`. The reliance on `Base.connection` is problematic for sharding support and calling `Base.establish_connection` in the Rake tasks (without any warning message) indicates it's ok for applications to do the same (outside these tasks, it's not). I've vetted the approach of swapping out `Base` for a temporary class in another branch but decided that it would be easier to demonstrate and contain the changes if I first contained these calls. The major changes in this PR are: * Contain `Base.connection` in `Tasks.migration_connection` and replace calls to each * Contain `Base.establish_connection` in `Tasks.establish_connection` and replace calls to each * Add a `with_temporary_connection_for_each` method for cases where we need to loop over each config and set the connection back afterwards * Add a `with_temporary_connection(db_config)` method for cases where we have one config but need to establish a new connection and set the old one back. * Update every place we were looping through configs to establish a connection with the new temporary connection methods. There are a lot of changes here but I've pulled out everything that didn't need to be in this commit into other PRs. Once this is merged, I'll create the next PR that replaces `Base` the new methods in `Tasks` with a temporary connection and then we will officially be no longer clobbering `Base` in these tasks. This also reduces complexity because we won't need to ensure we set `Base.connection` back at the end. Once that is working and all internal methods are using the new temp class I'll deprecate calls on `Base.connection` in these methods. Most applications just override the task but not the actual methods in `Tasks` so my hope is this will be smooth-ish. However, nothing will stop applications from still using `Base.connection` for a very long time if they still want to clobber.
1 parent a02c235 commit 901828f

File tree

4 files changed

+215
-202
lines changed

4 files changed

+215
-202
lines changed

activerecord/lib/active_record/migration.rb

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,15 @@ class PendingMigrationError < MigrationError # :nodoc:
137137
ActiveRecord::Tasks::DatabaseTasks.migrate
138138

139139
if ActiveRecord.dump_schema_after_migration
140-
ActiveRecord::Tasks::DatabaseTasks.dump_schema(
141-
ActiveRecord::Base.connection_db_config
142-
)
140+
connection = ActiveRecord::Tasks::DatabaseTasks.migration_connection
141+
ActiveRecord::Tasks::DatabaseTasks.dump_schema(connection.pool.db_config)
143142
end
144143
end
145144

146145
def initialize(message = nil, pending_migrations: nil)
147146
if pending_migrations.nil?
148-
pending_migrations = ActiveRecord::Base.connection.migration_context.open.pending_migrations
147+
connection = ActiveRecord::Tasks::DatabaseTasks.migration_connection
148+
pending_migrations = connection.migration_context.open.pending_migrations
149149
end
150150

151151
super(message || detailed_migration_message(pending_migrations))
@@ -165,6 +165,10 @@ def detailed_migration_message(pending_migrations)
165165

166166
message
167167
end
168+
169+
def connection
170+
ActiveRecord::Tasks::DatabaseTasks.migration_connection
171+
end
168172
end
169173

170174
class ConcurrentMigrationError < MigrationError # :nodoc:
@@ -624,6 +628,10 @@ def build_watcher(&block)
624628
paths = all_configs.flat_map { |config| config.migrations_paths || Migrator.migrations_paths }.uniq
625629
@file_watcher.new([], paths.index_with(["rb"]), &block)
626630
end
631+
632+
def connection
633+
ActiveRecord::Tasks::DatabaseTasks.migration_connection
634+
end
627635
end
628636

629637
class << self
@@ -635,7 +643,7 @@ def nearest_delegate # :nodoc:
635643
end
636644

637645
# Raises <tt>ActiveRecord::PendingMigrationError</tt> error if any migrations are pending.
638-
def check_pending!(connection = Base.connection)
646+
def check_pending!(connection = ActiveRecord::Tasks::DatabaseTasks.migration_connection)
639647
if pending_migrations = connection.migration_context.pending_migrations
640648
raise ActiveRecord::PendingMigrationError.new(pending_migrations: pending_migrations)
641649
end
@@ -645,6 +653,7 @@ def load_schema_if_pending!
645653
if any_schema_needs_update?
646654
# Roundtrip to Rake to allow plugins to hook into database initialization.
647655
root = defined?(ENGINE_ROOT) ? ENGINE_ROOT : Rails.root
656+
648657
FileUtils.cd(root) do
649658
Base.connection_handler.clear_all_connections!(:all)
650659
system("bin/rails db:test:prepare")
@@ -692,25 +701,24 @@ def any_schema_needs_update?
692701
end
693702
end
694703

704+
def db_configs_in_current_env
705+
ActiveRecord::Base.configurations.configs_for(env_name: env)
706+
end
707+
695708
def pending_migrations
696-
prev_db_config = Base.connection_db_config
697709
pending_migrations = []
698710

699-
db_configs_in_current_env.each do |db_config|
700-
conn = Base.establish_connection(db_config).connection
701-
if pending = conn.migration_context.open.pending_migrations
711+
ActiveRecord::Tasks::DatabaseTasks.with_temporary_connection_for_each(env: env) do |connection|
712+
if pending = connection.migration_context.open.pending_migrations
702713
pending_migrations << pending
703714
end
704715
end
705716

706717
pending_migrations.flatten
707-
ensure
708-
Base.establish_connection(prev_db_config)
709718
end
710719

711-
def db_configs_in_current_env
712-
current_environment = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
713-
ActiveRecord::Base.configurations.configs_for(env_name: current_environment)
720+
def env
721+
ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
714722
end
715723
end
716724

@@ -893,7 +901,7 @@ def migrate(direction)
893901
end
894902

895903
time = nil
896-
ActiveRecord::Base.connection_pool.with_connection do |conn|
904+
ActiveRecord::Tasks::DatabaseTasks.migration_connection.pool.with_connection do |conn|
897905
time = Benchmark.measure do
898906
exec_migration(conn, direction)
899907
end
@@ -957,7 +965,7 @@ def suppress_messages
957965
end
958966

959967
def connection
960-
@connection || ActiveRecord::Base.connection
968+
@connection || ActiveRecord::Tasks::DatabaseTasks.migration_connection
961969
end
962970

963971
def method_missing(method, *arguments, &block)
@@ -1138,8 +1146,8 @@ def initialize(migrations_paths, schema_migration = nil, internal_metadata = nil
11381146
end
11391147

11401148
@migrations_paths = migrations_paths
1141-
@schema_migration = schema_migration || SchemaMigration.new(ActiveRecord::Base.connection)
1142-
@internal_metadata = internal_metadata || InternalMetadata.new(ActiveRecord::Base.connection)
1149+
@schema_migration = schema_migration || SchemaMigration.new(connection)
1150+
@internal_metadata = internal_metadata || InternalMetadata.new(connection)
11431151
end
11441152

11451153
# Runs the migrations in the +migrations_path+.
@@ -1265,7 +1273,6 @@ def protected_environment? # :nodoc:
12651273
end
12661274

12671275
def last_stored_environment # :nodoc:
1268-
connection = ActiveRecord::Base.connection
12691276
return nil unless connection.internal_metadata.enabled?
12701277
return nil if current_version == 0
12711278
raise NoEnvironmentInSchemaError unless connection.internal_metadata.table_exists?
@@ -1276,6 +1283,10 @@ def last_stored_environment # :nodoc:
12761283
end
12771284

12781285
private
1286+
def connection
1287+
ActiveRecord::Tasks::DatabaseTasks.migration_connection
1288+
end
1289+
12791290
def migration_files
12801291
paths = Array(migrations_paths)
12811292
Dir[*paths.flat_map { |path| "#{path}/**/[0-9]*_*.rb" }]
@@ -1311,8 +1322,9 @@ class << self
13111322

13121323
# For cases where a table doesn't exist like loading from schema cache
13131324
def current_version
1314-
schema_migration = SchemaMigration.new(ActiveRecord::Base.connection)
1315-
internal_metadata = InternalMetadata.new(ActiveRecord::Base.connection)
1325+
connection = ActiveRecord::Tasks::DatabaseTasks.migration_connection
1326+
schema_migration = SchemaMigration.new(connection)
1327+
internal_metadata = InternalMetadata.new(connection)
13161328

13171329
MigrationContext.new(migrations_paths, schema_migration, internal_metadata).current_version
13181330
end
@@ -1388,6 +1400,10 @@ def load_migrated
13881400
end
13891401

13901402
private
1403+
def connection
1404+
ActiveRecord::Tasks::DatabaseTasks.migration_connection
1405+
end
1406+
13911407
# Used for running a specific migration.
13921408
def run_without_lock
13931409
migration = migrations.detect { |m| m.version == @target_version }
@@ -1411,7 +1427,7 @@ def migrate_without_lock
14111427
def record_environment
14121428
return if down?
14131429

1414-
@internal_metadata[:environment] = ActiveRecord::Base.connection.migration_context.current_environment
1430+
@internal_metadata[:environment] = connection.migration_context.current_environment
14151431
end
14161432

14171433
def ran?(migration)
@@ -1481,23 +1497,22 @@ def down?
14811497
# Wrap the migration in a transaction only if supported by the adapter.
14821498
def ddl_transaction(migration, &block)
14831499
if use_transaction?(migration)
1484-
Base.transaction(&block)
1500+
connection.transaction(&block)
14851501
else
14861502
yield
14871503
end
14881504
end
14891505

14901506
def use_transaction?(migration)
1491-
!migration.disable_ddl_transaction && Base.connection.supports_ddl_transactions?
1507+
!migration.disable_ddl_transaction && connection.supports_ddl_transactions?
14921508
end
14931509

14941510
def use_advisory_lock?
1495-
Base.connection.advisory_locks_enabled?
1511+
connection.advisory_locks_enabled?
14961512
end
14971513

14981514
def with_advisory_lock
14991515
lock_id = generate_migrator_advisory_lock_id
1500-
connection = ActiveRecord::Base.connection
15011516

15021517
got_lock = connection.get_advisory_lock(lock_id)
15031518
raise ConcurrentMigrationError unless got_lock
@@ -1513,7 +1528,7 @@ def with_advisory_lock
15131528

15141529
MIGRATOR_SALT = 2053462845
15151530
def generate_migrator_advisory_lock_id
1516-
db_name_hash = Zlib.crc32(Base.connection.current_database)
1531+
db_name_hash = Zlib.crc32(connection.current_database)
15171532
MIGRATOR_SALT * db_name_hash
15181533
end
15191534
end

0 commit comments

Comments
 (0)