mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Refactor schema migration on connection
This method was jumping through extra hoops to find the name of the class the connection is stored on when we can get it from the connection itself. Since we already have the connection we don't need to loop through the pools. In addition, this was using the wrong class name. The class name for the schema migration should come from the connection owner class, not from the `db_config.name`. In this case, `db_config.name` is the name of the configuration in the database.yml. Rails uses the class name to lookup connections, not the db config name, so we should be consistent here. While working on this I noticed that we were generating an extra schema migration class for `ActiveRecord::Base`. Since `ActiveRecord::Base` can and should use the default and we don't want to create a new one for single db applications, we should skip creating this if the spec name is `ActiveRecord::Base`. I added an additional test that ensures the class generation is correct.
This commit is contained in:
parent
e00f9c8aae
commit
095f1bfaa0
4 changed files with 14 additions and 11 deletions
|
@ -130,19 +130,17 @@ module ActiveRecord
|
|||
def schema_migration # :nodoc:
|
||||
@schema_migration ||= begin
|
||||
conn = self
|
||||
name = conn.pool.db_config.name
|
||||
schema_migration_name = "#{name}::SchemaMigration"
|
||||
spec_name = conn.pool.pool_config.connection_specification_name
|
||||
|
||||
return ActiveRecord::SchemaMigration if spec_name == "ActiveRecord::Base"
|
||||
|
||||
schema_migration_name = "#{spec_name}::SchemaMigration"
|
||||
|
||||
Class.new(ActiveRecord::SchemaMigration) do
|
||||
define_singleton_method(:name) { schema_migration_name }
|
||||
define_singleton_method(:to_s) { schema_migration_name }
|
||||
|
||||
connection_handler.connection_pool_names.each do |pool_name|
|
||||
if conn.pool == connection_handler.retrieve_connection_pool(pool_name)
|
||||
self.connection_specification_name = pool_name
|
||||
break
|
||||
end
|
||||
end
|
||||
self.connection_specification_name = spec_name
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -67,6 +67,11 @@ class MultiDbMigratorTest < ActiveRecord::TestCase
|
|||
end
|
||||
end
|
||||
|
||||
def test_schema_migration_class_names
|
||||
assert_equal "ActiveRecord::SchemaMigration", @schema_migration_a.name
|
||||
assert_equal "ARUnit2Model::SchemaMigration", @schema_migration_b.name
|
||||
end
|
||||
|
||||
def test_finds_migrations
|
||||
@migrations_a_list.each_with_index do |pair, i|
|
||||
assert_equal @migrations_a[i].version, pair.first
|
||||
|
|
|
@ -133,12 +133,12 @@ class LoadingTest < ActiveSupport::TestCase
|
|||
require "#{rails_root}/config/environment"
|
||||
setup_ar!
|
||||
|
||||
initial = [ActiveStorage::Blob, ActiveStorage::Attachment, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata, ApplicationRecord, "primary::SchemaMigration"].collect(&:to_s).sort
|
||||
initial = [ActiveStorage::Blob, ActiveStorage::Attachment, ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata, ApplicationRecord].collect(&:to_s).sort
|
||||
assert_equal initial, ActiveRecord::Base.descendants.collect(&:to_s).sort.uniq
|
||||
get "/load"
|
||||
assert_equal [Post].collect(&:to_s).sort, ActiveRecord::Base.descendants.collect(&:to_s).sort - initial
|
||||
get "/unload"
|
||||
assert_equal ["ActiveRecord::InternalMetadata", "ActiveRecord::SchemaMigration", "primary::SchemaMigration"], ActiveRecord::Base.descendants.collect(&:to_s).sort.uniq
|
||||
assert_equal ["ActiveRecord::InternalMetadata", "ActiveRecord::SchemaMigration"], ActiveRecord::Base.descendants.collect(&:to_s).sort.uniq
|
||||
end
|
||||
|
||||
test "initialize cant be called twice" do
|
||||
|
|
|
@ -101,7 +101,7 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase
|
|||
test "autoloaded? and overridden class names" do
|
||||
invalid_constant_name = Module.new do
|
||||
def self.name
|
||||
"primary::SchemaMigration"
|
||||
"MyModule::SchemaMigration"
|
||||
end
|
||||
end
|
||||
assert_not deps.autoloaded?(invalid_constant_name)
|
||||
|
|
Loading…
Reference in a new issue