From 436277da88507f9aae0874e62f3e61a8546b9683 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Fri, 26 Aug 2022 10:47:00 -0400 Subject: [PATCH] Move SchemaMigration to an independent object Previously, SchemaMigration inherited from ActiveRecord::Base. This is problematic for multiple databases and resulted in building the code in AbstractAdapter that was previously there. Rather than hacking around the fact that SchemaMigration inherits from Base, this PR makes SchemaMigration an independent object. Then each connection can get it's own SchemaMigration object. This change required defining the methods that SchemaMigration was depending on ActiveRecord::Base for (ex create!). I reimplemented only the methods called by the framework as this class is no-doc's so it doesn't need to implement anything beyond that. Now each connection gets it's own SchemaMigration object which stores the connection. I also decided to update the method names (create -> create_version, delete_by -> delete_version, delete_all -> delete_all_versions) to be more explicit. This change also required adding a NullSchemaMigraiton class for cases when we don't have a connection yet but still need to copy migrations from the MigrationContext. Ultimately I think this is a little weird - we need to do so much work to pick up a set of files? Maybe something to explore in the future. Aside from removing the hack we added back in #36439 this change will enable my work to stop clobbering and depending directly on Base.connection in the rake tasks. While working on this I discovered that we always have a `ActiveRecord::SchemaMigration` because the connection is always on `Base` in the rake tasks. This will free us up to do less hacky stuff in the migrations and tasks. --- activerecord/CHANGELOG.md | 6 + .../abstract/schema_statements.rb | 2 +- .../connection_adapters/abstract_adapter.rb | 16 +-- activerecord/lib/active_record/migration.rb | 35 +++--- .../lib/active_record/schema_migration.rb | 107 ++++++++++++------ .../lib/active_record/tasks/database_tasks.rb | 2 +- .../test/cases/active_record_schema_test.rb | 6 +- .../adapters/mysql2/schema_migrations_test.rb | 8 +- .../adapters/mysql2/table_options_test.rb | 2 +- .../postgresql/extension_migration_test.rb | 6 +- .../cases/adapters/postgresql/uuid_test.rb | 4 +- .../cases/migration/compatibility_test.rb | 6 +- .../test/cases/migration/logger_test.rb | 2 +- activerecord/test/cases/migration_test.rb | 57 ++++++---- activerecord/test/cases/migrator_test.rb | 44 +++---- .../test/cases/multi_db_migrator_test.rb | 26 +++-- activerecord/test/cases/schema_dumper_test.rb | 9 +- .../test/cases/tasks/database_tasks_test.rb | 28 ++--- railties/test/application/loading_test.rb | 4 +- railties/test/railties/engine_test.rb | 2 +- 20 files changed, 210 insertions(+), 162 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4a8c9314db..a0ca6d9c14 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Move `ActiveRecord::SchemaMigration` to an independent object. + + `ActiveRecord::SchemaMigration` no longer inherits from `ActiveRecord::Base` and is now an independent object that should be instantiated with a `connection`. This class is private and should not be used by applications directly. If you want to interact with the schema migrations table, please access it on the connection directly, for example: `ActiveRecord::Base.connection.schema_migration`. + + *Eileen M. Uchitelle* + * Deprecate `all_connection_pools` and make `connection_pool_list` more explicit. Following on #45924 `all_connection_pools` is now deprecated. `connection_pool_list` will either take an explicit role or applications can opt into the new behavior by passing `:all`. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index d4fcaaa399..50b09884dc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1270,7 +1270,7 @@ def check_constraint_exists?(table_name, **options) end def dump_schema_information # :nodoc: - versions = schema_migration.all_versions + versions = schema_migration.versions insert_versions_sql(versions) if versions.any? end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index e88b02544b..f32c1c0a97 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -193,21 +193,7 @@ def migration_context # :nodoc: end def schema_migration # :nodoc: - @schema_migration ||= begin - conn = self - connection_name = conn.pool.pool_config.connection_name - - return ActiveRecord::SchemaMigration if connection_name == "ActiveRecord::Base" - - schema_migration_name = "#{connection_name}::SchemaMigration" - - Class.new(ActiveRecord::SchemaMigration) do - define_singleton_method(:name) { schema_migration_name } - define_singleton_method(:to_s) { schema_migration_name } - - self.connection_specification_name = connection_name - end - end + SchemaMigration.new(self) end def internal_metadata # :nodoc: diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 2be7aff302..e152a110c4 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -950,14 +950,13 @@ def method_missing(method, *arguments, &block) def copy(destination, sources, options = {}) copied = [] - schema_migration = options[:schema_migration] || ActiveRecord::SchemaMigration FileUtils.mkdir_p(destination) unless File.exist?(destination) - destination_migrations = ActiveRecord::MigrationContext.new(destination, schema_migration).migrations + destination_migrations = ActiveRecord::MigrationContext.new(destination, SchemaMigration::NullSchemaMigration.new).migrations last = destination_migrations.last sources.each do |scope, path| - source_migrations = ActiveRecord::MigrationContext.new(path, schema_migration).migrations + source_migrations = ActiveRecord::MigrationContext.new(path, SchemaMigration::NullSchemaMigration.new).migrations source_migrations.each do |migration| source = File.binread(migration.filename) @@ -1018,7 +1017,7 @@ def next_migration_number(number) if ActiveRecord.timestamped_migrations [Time.now.utc.strftime("%Y%m%d%H%M%S"), "%.14d" % number].max else - SchemaMigration.normalize_migration_number(number) + "%.3d" % number.to_i end end @@ -1076,15 +1075,21 @@ def load_migration # # A migration context requires the path to the migrations is set # in the +migrations_paths+ parameter. Optionally a +schema_migration+ - # class can be provided. For most applications, +SchemaMigration+ is - # sufficient. Multiple database applications need a +SchemaMigration+ - # per primary database. + # class can be provided. Multiple database applications will instantiate + # a +SchemaMigration+ object per database. From the Rake tasks, Rails will + # handle this for you. class MigrationContext attr_reader :migrations_paths, :schema_migration, :internal_metadata - def initialize(migrations_paths, schema_migration = SchemaMigration, internal_metadata = InternalMetadata) + def initialize(migrations_paths, schema_migration = nil, internal_metadata = InternalMetadata) + if schema_migration == SchemaMigration + schema_migration = SchemaMigration.new(ActiveRecord::Base.connection) + + ActiveSupport::Deprecation.warn("SchemaMigration no longer inherits from ActiveRecord::Base. Please instaniate a new SchemaMigration object with the desired connection, ie `ActiveRecord::SchemaMigration.new(ActiveRecord::Base.connection)`.") + end + @migrations_paths = migrations_paths - @schema_migration = schema_migration + @schema_migration = schema_migration || SchemaMigration.new(ActiveRecord::Base.connection) @internal_metadata = internal_metadata end @@ -1152,7 +1157,7 @@ def open # :nodoc: def get_all_versions # :nodoc: if schema_migration.table_exists? - schema_migration.all_versions.map(&:to_i) + schema_migration.integer_versions else [] end @@ -1257,7 +1262,9 @@ class << self # For cases where a table doesn't exist like loading from schema cache def current_version - MigrationContext.new(migrations_paths, SchemaMigration, InternalMetadata).current_version + schema_migration = SchemaMigration.new(ActiveRecord::Base.connection) + + MigrationContext.new(migrations_paths, schema_migration, InternalMetadata).current_version end end @@ -1327,7 +1334,7 @@ def migrated end def load_migrated - @migrated_versions = Set.new(@schema_migration.all_versions.map(&:to_i)) + @migrated_versions = Set.new(@schema_migration.integer_versions) end private @@ -1406,10 +1413,10 @@ def validate(migrations) def record_version_state_after_migrating(version) if down? migrated.delete(version) - @schema_migration.delete_by(version: version.to_s) + @schema_migration.delete_version(version.to_s) else migrated << version - @schema_migration.create!(version: version.to_s) + @schema_migration.create_version(version.to_s) end end diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb index 624b49586f..7b977f005f 100644 --- a/activerecord/lib/active_record/schema_migration.rb +++ b/activerecord/lib/active_record/schema_migration.rb @@ -1,54 +1,87 @@ # frozen_string_literal: true -require "active_record/scoping/default" -require "active_record/scoping/named" - module ActiveRecord # This class is used to create a table that keeps track of which migrations # have been applied to a given database. When a migration is run, its schema - # number is inserted in to the `SchemaMigration.table_name` so it doesn't need + # number is inserted in to the schema migrations table so it doesn't need # to be executed the next time. - class SchemaMigration < ActiveRecord::Base # :nodoc: - class << self - def primary_key - "version" - end + class SchemaMigration # :nodoc: + attr_reader :connection, :arel_table - def table_name - "#{table_name_prefix}#{schema_migrations_table_name}#{table_name_suffix}" - end + def initialize(connection) + @connection = connection + @arel_table = Arel::Table.new(table_name) + end - def create_table - unless connection.table_exists?(table_name) - connection.create_table(table_name, id: false) do |t| - t.string :version, **connection.internal_string_options_for_primary_key - end - end - end + def create_version(version) + im = Arel::InsertManager.new(arel_table) + im.insert(arel_table[primary_key] => version) + connection.insert(im, "#{self} Create", primary_key, version) + end - def drop_table - connection.drop_table table_name, if_exists: true - end + def delete_version(version) + dm = Arel::DeleteManager.new(arel_table) + dm.wheres = [arel_table[primary_key].eq(version)] - def normalize_migration_number(number) - "%.3d" % number.to_i - end + connection.delete(dm, "#{self} Destroy") + end - def normalized_versions - all_versions.map { |v| normalize_migration_number v } - end - - def all_versions - order(:version).pluck(:version) - end - - def table_exists? - connection.data_source_exists?(table_name) + def delete_all_versions + versions.each do |version| + delete_version(version) end end - def version - super.to_i + def primary_key + "version" + end + + def table_name + "#{ActiveRecord::Base.table_name_prefix}#{ActiveRecord::Base.schema_migrations_table_name}#{ActiveRecord::Base.table_name_suffix}" + end + + def create_table + unless connection.table_exists?(table_name) + connection.create_table(table_name, id: false) do |t| + t.string :version, **connection.internal_string_options_for_primary_key + end + end + end + + def drop_table + connection.drop_table table_name, if_exists: true + end + + def normalize_migration_number(number) + "%.3d" % number.to_i + end + + def normalized_versions + versions.map { |v| normalize_migration_number v } + end + + def versions + sm = Arel::SelectManager.new(arel_table) + sm.project(arel_table[primary_key]) + sm.order(arel_table[primary_key].asc) + connection.select_values(sm) + end + + def integer_versions + versions.map(&:to_i) + end + + def count + sm = Arel::SelectManager.new(arel_table) + sm.project(*Arel::Nodes::Count.new([Arel.star])) + connection.select_values(sm).first + end + + def table_exists? + connection.data_source_exists?(table_name) + end + + class NullSchemaMigration end end end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index bbb7494bef..e8485adbf4 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -192,7 +192,7 @@ def prepare_all ActiveRecord::Base.establish_connection(db_config) begin - database_initialized = ActiveRecord::SchemaMigration.table_exists? + database_initialized = ActiveRecord::Base.connection.schema_migration.table_exists? rescue ActiveRecord::NoDatabaseError create(db_config) retry diff --git a/activerecord/test/cases/active_record_schema_test.rb b/activerecord/test/cases/active_record_schema_test.rb index cffe187bb1..6c7b5cc65c 100644 --- a/activerecord/test/cases/active_record_schema_test.rb +++ b/activerecord/test/cases/active_record_schema_test.rb @@ -20,7 +20,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase @connection.drop_table :nep_schema_migrations rescue nil @connection.drop_table :has_timestamps rescue nil @connection.drop_table :multiple_indexes rescue nil - @schema_migration.delete_all rescue nil + @schema_migration.delete_all_versions rescue nil ActiveRecord::Migration.verbose = @original_verbose end @@ -31,7 +31,7 @@ def test_has_primary_key @schema_migration.create_table assert_difference "@schema_migration.count", 1 do - @schema_migration.create version: 12 + @schema_migration.create_version(12) end ensure @schema_migration.drop_table @@ -69,7 +69,6 @@ def test_schema_define def test_schema_define_with_table_name_prefix old_table_name_prefix = ActiveRecord::Base.table_name_prefix ActiveRecord::Base.table_name_prefix = "nep_" - @schema_migration.reset_table_name @internal_metadata.reset_table_name ActiveRecord::Schema.define(version: 7) do create_table :fruits do |t| @@ -82,7 +81,6 @@ def test_schema_define_with_table_name_prefix assert_equal 7, @connection.migration_context.current_version ensure ActiveRecord::Base.table_name_prefix = old_table_name_prefix - @schema_migration.reset_table_name @internal_metadata.reset_table_name end diff --git a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb index 4b8c00fa3c..436bda9258 100644 --- a/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb +++ b/activerecord/test/cases/adapters/mysql2/schema_migrations_test.rb @@ -5,6 +5,10 @@ class SchemaMigrationsTest < ActiveRecord::Mysql2TestCase self.use_transactional_tests = false + def setup + @schema_migration = ActiveRecord::Base.connection.schema_migration + end + def test_renaming_index_on_foreign_key connection.add_index "engines", "car_id" connection.add_foreign_key :engines, :cars, name: "fk_engines_cars" @@ -17,10 +21,10 @@ def test_renaming_index_on_foreign_key def test_initializes_schema_migrations_for_encoding_utf8mb4 with_encoding_utf8mb4 do - table_name = ActiveRecord::SchemaMigration.table_name + table_name = @schema_migration.table_name connection.drop_table table_name, if_exists: true - ActiveRecord::SchemaMigration.create_table + @schema_migration.create_table assert connection.column_exists?(table_name, :version, :string) end diff --git a/activerecord/test/cases/adapters/mysql2/table_options_test.rb b/activerecord/test/cases/adapters/mysql2/table_options_test.rb index 6edbe769e7..101dd76ff4 100644 --- a/activerecord/test/cases/adapters/mysql2/table_options_test.rb +++ b/activerecord/test/cases/adapters/mysql2/table_options_test.rb @@ -94,7 +94,7 @@ def teardown ActiveRecord::Base.logger = @logger_was ActiveRecord::Migration.verbose = @verbose_was ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true - ActiveRecord::SchemaMigration.delete_all rescue nil + ActiveRecord::Base.connection.schema_migration.delete_all_versions rescue nil end test "new migrations do not contain default ENGINE=InnoDB option" do diff --git a/activerecord/test/cases/adapters/postgresql/extension_migration_test.rb b/activerecord/test/cases/adapters/postgresql/extension_migration_test.rb index 908a1d2f2b..21fae532d1 100644 --- a/activerecord/test/cases/adapters/postgresql/extension_migration_test.rb +++ b/activerecord/test/cases/adapters/postgresql/extension_migration_test.rb @@ -27,20 +27,18 @@ def setup ActiveRecord::Base.table_name_prefix = "p_" ActiveRecord::Base.table_name_suffix = "_s" - @connection.schema_migration.reset_table_name @connection.internal_metadata.reset_table_name - @connection.schema_migration.delete_all rescue nil + @connection.schema_migration.delete_all_versions rescue nil ActiveRecord::Migration.verbose = false end def teardown - @connection.schema_migration.delete_all rescue nil + @connection.schema_migration.delete_all_versions rescue nil ActiveRecord::Migration.verbose = true ActiveRecord::Base.table_name_prefix = @old_table_name_prefix ActiveRecord::Base.table_name_suffix = @old_table_name_suffix - @connection.schema_migration.reset_table_name @connection.internal_metadata.reset_table_name super diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 82041a5f23..ea34014a6d 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -308,7 +308,7 @@ def migrate(x) ensure drop_table "pg_uuids_4" ActiveRecord::Migration.verbose = @verbose_was - ActiveRecord::Base.connection.schema_migration.delete_all + ActiveRecord::Base.connection.schema_migration.delete_all_versions end uses_transaction :test_schema_dumper_for_uuid_primary_key_default_in_legacy_migration end @@ -360,7 +360,7 @@ def migrate(x) ensure drop_table "pg_uuids_4" ActiveRecord::Migration.verbose = @verbose_was - ActiveRecord::Base.connection.schema_migration.delete_all + ActiveRecord::Base.connection.schema_migration.delete_all_versions end uses_transaction :test_schema_dumper_for_uuid_primary_key_with_default_nil_in_legacy_migration end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 1fbd0adf5d..49026d38ff 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -30,7 +30,7 @@ def setup teardown do connection.drop_table :testings rescue nil ActiveRecord::Migration.verbose = @verbose_was - @schema_migration.delete_all rescue nil + @schema_migration.delete_all_versions rescue nil end def test_migration_doesnt_remove_named_index @@ -685,7 +685,7 @@ def setup def teardown ActiveRecord::Migration.verbose = @verbose_was - @schema_migration.delete_all rescue nil + @schema_migration.delete_all_versions rescue nil connection.drop_table :testings rescue nil end @@ -812,7 +812,7 @@ def setup def teardown @migration.migrate(:down) if @migration ActiveRecord::Migration.verbose = @verbose_was - ActiveRecord::SchemaMigration.delete_all rescue nil + ActiveRecord::Base.connection.schema_migration.delete_all_versions rescue nil LegacyPrimaryKey.reset_column_information end diff --git a/activerecord/test/cases/migration/logger_test.rb b/activerecord/test/cases/migration/logger_test.rb index ed4e8d853c..92836f69e6 100644 --- a/activerecord/test/cases/migration/logger_test.rb +++ b/activerecord/test/cases/migration/logger_test.rb @@ -19,7 +19,7 @@ def setup super @schema_migration = ActiveRecord::Base.connection.schema_migration @schema_migration.create_table - @schema_migration.delete_all + @schema_migration.delete_all_versions @internal_metadata = ActiveRecord::Base.connection.internal_metadata end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 915f6fa32c..471ff885bb 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -53,8 +53,8 @@ def setup ActiveRecord::Base.table_name_prefix = "" ActiveRecord::Base.table_name_suffix = "" - ActiveRecord::SchemaMigration.create_table - ActiveRecord::SchemaMigration.delete_all + @schema_migration.create_table + @schema_migration.delete_all_versions %w(things awesome_things prefix_things_suffix p_awesome_things_s).each do |table| Thing.connection.drop_table(table) rescue nil @@ -79,6 +79,22 @@ def setup ActiveRecord::Migration.verbose = @verbose_was end + def test_passing_a_schema_migration_class_to_migration_context_is_deprecated + migrations_path = MIGRATIONS_ROOT + "/valid" + migrator = assert_deprecated { ActiveRecord::MigrationContext.new(migrations_path, ActiveRecord::SchemaMigration) } + migrator.up + + assert_equal 3, migrator.current_version + assert_equal false, migrator.needs_migration? + + migrator.down + assert_equal 0, migrator.current_version + assert_equal true, migrator.needs_migration? + + @schema_migration.create_version(3) + assert_equal true, migrator.needs_migration? + end + def test_migration_context_with_default_schema_migration migrations_path = MIGRATIONS_ROOT + "/valid" migrator = ActiveRecord::MigrationContext.new(migrations_path) @@ -91,7 +107,7 @@ def test_migration_context_with_default_schema_migration assert_equal 0, migrator.current_version assert_equal true, migrator.needs_migration? - ActiveRecord::SchemaMigration.create!(version: 3) + @schema_migration.create_version(3) assert_equal true, migrator.needs_migration? end @@ -111,7 +127,7 @@ def test_migrator_versions assert_equal 0, migrator.current_version assert_equal true, migrator.needs_migration? - ActiveRecord::SchemaMigration.create!(version: 3) + @schema_migration.create_version(3) assert_equal true, migrator.needs_migration? end @@ -635,21 +651,20 @@ def migrate(x) def test_schema_migrations_table_name original_schema_migrations_table_name = ActiveRecord::Base.schema_migrations_table_name - assert_equal "schema_migrations", ActiveRecord::SchemaMigration.table_name + assert_equal "schema_migrations", @schema_migration.table_name ActiveRecord::Base.table_name_prefix = "prefix_" ActiveRecord::Base.table_name_suffix = "_suffix" Reminder.reset_table_name - assert_equal "prefix_schema_migrations_suffix", ActiveRecord::SchemaMigration.table_name + assert_equal "prefix_schema_migrations_suffix", @schema_migration.table_name ActiveRecord::Base.schema_migrations_table_name = "changed" Reminder.reset_table_name - assert_equal "prefix_changed_suffix", ActiveRecord::SchemaMigration.table_name + assert_equal "prefix_changed_suffix", @schema_migration.table_name ActiveRecord::Base.table_name_prefix = "" ActiveRecord::Base.table_name_suffix = "" Reminder.reset_table_name - assert_equal "changed", ActiveRecord::SchemaMigration.table_name + assert_equal "changed", @schema_migration.table_name ensure ActiveRecord::Base.schema_migrations_table_name = original_schema_migrations_table_name - ActiveRecord::SchemaMigration.reset_table_name Reminder.reset_table_name end @@ -774,28 +789,26 @@ def test_internal_metadata_create_table_wont_be_affected_by_schema_cache end def test_schema_migration_create_table_wont_be_affected_by_schema_cache - ActiveRecord::SchemaMigration.drop_table - assert_not_predicate ActiveRecord::SchemaMigration, :table_exists? + @schema_migration.drop_table + assert_not_predicate @schema_migration, :table_exists? - ActiveRecord::SchemaMigration.connection.transaction do - ActiveRecord::SchemaMigration.create_table - assert_predicate ActiveRecord::SchemaMigration, :table_exists? + @schema_migration.connection.transaction do + @schema_migration.create_table + assert_predicate @schema_migration, :table_exists? - schema_migration = ActiveRecord::SchemaMigration.create!(version: "foo") - assert_equal "foo", schema_migration[:version] + assert_equal "foo", @schema_migration.create_version("foo") raise ActiveRecord::Rollback end - ActiveRecord::SchemaMigration.connection.transaction do - ActiveRecord::SchemaMigration.create_table - assert_predicate ActiveRecord::SchemaMigration, :table_exists? + @schema_migration.connection.transaction do + @schema_migration.create_table + assert_predicate @schema_migration, :table_exists? - schema_migration = ActiveRecord::SchemaMigration.create!(version: "bar") - assert_equal "bar", schema_migration[:version] + assert_equal "bar", @schema_migration.create_version("bar") raise ActiveRecord::Rollback end ensure - ActiveRecord::SchemaMigration.create_table + @schema_migration.create_table end def test_proper_table_name_on_migration diff --git a/activerecord/test/cases/migrator_test.rb b/activerecord/test/cases/migrator_test.rb index 3652355a74..8e49dc68f1 100644 --- a/activerecord/test/cases/migrator_test.rb +++ b/activerecord/test/cases/migrator_test.rb @@ -25,7 +25,7 @@ def setup super @schema_migration = ActiveRecord::Base.connection.schema_migration @schema_migration.create_table - @schema_migration.delete_all rescue nil + @schema_migration.delete_all_versions rescue nil @internal_metadata = ActiveRecord::Base.connection.internal_metadata @verbose_was = ActiveRecord::Migration.verbose ActiveRecord::Migration.message_count = 0 @@ -38,7 +38,7 @@ def puts(*) end teardown do - @schema_migration.delete_all rescue nil + @schema_migration.delete_all_versions rescue nil ActiveRecord::Migration.verbose = @verbose_was ActiveRecord::Migration.class_eval do undef :puts @@ -145,7 +145,7 @@ def test_relative_migrations end def test_finds_pending_migrations - @schema_migration.create!(version: "1") + @schema_migration.create_version("1") migration_list = [ActiveRecord::Migration.new("foo", 1), ActiveRecord::Migration.new("bar", 3)] migrations = ActiveRecord::Migrator.new(:up, migration_list, @schema_migration, @internal_metadata).pending_migrations @@ -157,8 +157,8 @@ def test_migrations_status path = MIGRATIONS_ROOT + "/valid" schema_migration = ActiveRecord::Base.connection.schema_migration - @schema_migration.create(version: 2) - @schema_migration.create(version: 10) + @schema_migration.create_version(2) + @schema_migration.create_version(10) assert_equal [ ["down", "001", "Valid people have last names"], @@ -172,10 +172,10 @@ def test_migrations_status_order_new_and_old_version path = MIGRATIONS_ROOT + "/old_and_new_versions" schema_migration = ActiveRecord::Base.connection.schema_migration - @schema_migration.create(version: 230) - @schema_migration.create(version: 231) - @schema_migration.create(version: 20210716122844) - @schema_migration.create(version: 20210716123013) + @schema_migration.create_version(230) + @schema_migration.create_version(231) + @schema_migration.create_version(20210716122844) + @schema_migration.create_version(20210716123013) assert_equal [ ["up", "230", "Add people hobby"], @@ -189,14 +189,14 @@ def test_migrations_status_order_new_and_old_version_applied_out_of_order path = MIGRATIONS_ROOT + "/old_and_new_versions" schema_migration = ActiveRecord::Base.connection.schema_migration - @schema_migration.create(version: 230) - @schema_migration.create(version: 231) + @schema_migration.create_version(230) + @schema_migration.create_version(231) # "Apply" a newer migration and not an older to simulate out-of-order # migration application which should not affect ordering in status and is # possible if a branch is merged which contains a migration which has an # earlier version but is judged to be compatible with existing migrations. - @schema_migration.create(version: 20210716123013) + @schema_migration.create_version(20210716123013) assert_equal [ ["up", "230", "Add people hobby"], @@ -210,8 +210,8 @@ def test_migrations_status_in_subdirectories path = MIGRATIONS_ROOT + "/valid_with_subdirectories" schema_migration = ActiveRecord::Base.connection.schema_migration - @schema_migration.create(version: 2) - @schema_migration.create(version: 10) + @schema_migration.create_version(2) + @schema_migration.create_version(10) assert_equal [ ["down", "001", "Valid people have last names"], @@ -243,8 +243,8 @@ def test_migrations_status_from_two_directories paths = [MIGRATIONS_ROOT + "/valid_with_timestamps", MIGRATIONS_ROOT + "/to_copy_with_timestamps"] schema_migration = ActiveRecord::Base.connection.schema_migration - @schema_migration.create(version: "20100101010101") - @schema_migration.create(version: "20160528010101") + @schema_migration.create_version("20100101010101") + @schema_migration.create_version("20160528010101") assert_equal [ ["down", "20090101010101", "People have hobbies"], @@ -300,7 +300,7 @@ def test_down_calls_down end def test_current_version - @schema_migration.create!(version: "1000") + @schema_migration.create_version("1000") schema_migration = ActiveRecord::Base.connection.schema_migration migrator = ActiveRecord::MigrationContext.new("db/migrate", schema_migration) assert_equal 1000, migrator.current_version @@ -455,7 +455,7 @@ def test_migrator_output_when_running_single_migration result = migrator.run(:up, 1) - assert_equal(1, result.version) + assert_equal("1", result) end def test_migrator_rollback @@ -484,10 +484,10 @@ def test_migrator_db_has_no_schema_migrations_table _, migrator = migrator_class(3) migrator = migrator.new("valid", schema_migration) - ActiveRecord::SchemaMigration.drop_table - assert_not_predicate ActiveRecord::SchemaMigration, :table_exists? + @schema_migration.drop_table + assert_not_predicate @schema_migration, :table_exists? migrator.migrate(1) - assert_predicate ActiveRecord::SchemaMigration, :table_exists? + assert_predicate @schema_migration, :table_exists? end def test_migrator_forward @@ -506,7 +506,7 @@ def test_migrator_forward def test_only_loads_pending_migrations # migrate up to 1 - @schema_migration.create!(version: "1") + @schema_migration.create_version("1") schema_migration = ActiveRecord::Base.connection.schema_migration calls, migrator = migrator_class(3) diff --git a/activerecord/test/cases/multi_db_migrator_test.rb b/activerecord/test/cases/multi_db_migrator_test.rb index b85661c8f8..6f211388c1 100644 --- a/activerecord/test/cases/multi_db_migrator_test.rb +++ b/activerecord/test/cases/multi_db_migrator_test.rb @@ -29,8 +29,8 @@ def setup @connection_a.schema_migration.create_table @connection_b.schema_migration.create_table - @connection_a.schema_migration.delete_all rescue nil - @connection_b.schema_migration.delete_all rescue nil + @connection_a.schema_migration.delete_all_versions rescue nil + @connection_b.schema_migration.delete_all_versions rescue nil @path_a = MIGRATIONS_ROOT + "/valid" @path_b = MIGRATIONS_ROOT + "/to_copy" @@ -57,8 +57,8 @@ def puts(*) end teardown do - @connection_a.schema_migration.delete_all rescue nil - @connection_b.schema_migration.delete_all rescue nil + @connection_a.schema_migration.delete_all_versions rescue nil + @connection_b.schema_migration.delete_all_versions rescue nil ActiveRecord::Migration.verbose = @verbose_was ActiveRecord::Migration.class_eval do @@ -69,9 +69,11 @@ def puts(*) end end - def test_schema_migration_class_names - assert_equal "ActiveRecord::SchemaMigration", @schema_migration_a.name - assert_equal "ARUnit2Model::SchemaMigration", @schema_migration_b.name + def test_schema_migration_is_different_for_different_connections + assert_not_equal @schema_migration_a, @schema_migration_b + assert_not_equal @schema_migration_a.connection, @schema_migration_b.connection + assert "ActiveRecord::Base", @schema_migration_a.connection.pool.pool_config.connection_name + assert "Arunit2", @schema_migration_b.connection.pool.pool_config.connection_name end def test_finds_migrations @@ -87,8 +89,8 @@ def test_finds_migrations end def test_migrations_status - @schema_migration_a.create(version: 2) - @schema_migration_a.create(version: 10) + @schema_migration_a.create_version(2) + @schema_migration_a.create_version(10) assert_equal [ ["down", "001", "Valid people have last names"], @@ -97,7 +99,7 @@ def test_migrations_status ["up", "010", "********** NO FILE **********"], ], ActiveRecord::MigrationContext.new(@path_a, @schema_migration_a).migrations_status - @schema_migration_b.create(version: 4) + @schema_migration_b.create_version(4) assert_equal [ ["down", "001", "People have hobbies"], @@ -136,14 +138,14 @@ def test_get_all_versions end def test_finds_pending_migrations - @schema_migration_a.create!(version: "1") + @schema_migration_a.create_version("1") migration_list_a = [ActiveRecord::Migration.new("foo", 1), ActiveRecord::Migration.new("bar", 3)] migrations_a = ActiveRecord::Migrator.new(:up, migration_list_a, @schema_migration_a, @internal_metadata_a).pending_migrations assert_equal 1, migrations_a.size assert_equal migration_list_a.last, migrations_a.first - @schema_migration_b.create!(version: "1") + @schema_migration_b.create_version("1") migration_list_b = [ActiveRecord::Migration.new("foo", 1), ActiveRecord::Migration.new("bar", 3)] migrations_b = ActiveRecord::Migrator.new(:up, migration_list_b, @schema_migration_b, @internal_metadata_b).pending_migrations diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 964f50940f..b4f7707424 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -8,7 +8,8 @@ class SchemaDumperTest < ActiveRecord::TestCase self.use_transactional_tests = false setup do - ActiveRecord::SchemaMigration.create_table + @schema_migration = ActiveRecord::Base.connection.schema_migration + @schema_migration.create_table end def standard_dump @@ -16,7 +17,7 @@ def standard_dump end def test_dump_schema_information_with_empty_versions - ActiveRecord::SchemaMigration.delete_all + @schema_migration.delete_all_versions schema_info = ActiveRecord::Base.connection.dump_schema_information assert_no_match(/INSERT INTO/, schema_info) end @@ -24,7 +25,7 @@ def test_dump_schema_information_with_empty_versions def test_dump_schema_information_outputs_lexically_reverse_ordered_versions_regardless_of_database_order versions = %w{ 20100101010101 20100201010101 20100301010101 } versions.shuffle.each do |v| - ActiveRecord::SchemaMigration.create!(version: v) + @schema_migration.create_version(v) end schema_info = ActiveRecord::Base.connection.dump_schema_information @@ -37,7 +38,7 @@ def test_dump_schema_information_outputs_lexically_reverse_ordered_versions_rega STR assert_equal expected, schema_info ensure - ActiveRecord::SchemaMigration.delete_all + @schema_migration.delete_all_versions end def test_schema_dump_include_migration_version diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index 25f39e518f..64888093aa 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -1031,14 +1031,18 @@ def test_migrate_using_empty_scope_and_verbose_mode end class DatabaseTasksMigrateStatusTest < DatabaseTasksMigrationTestCase + def setup + @schema_migration = ActiveRecord::Base.connection.schema_migration + end + def test_migrate_status_table - ActiveRecord::SchemaMigration.create_table + @schema_migration.create_table output = capture_migration_status assert_match(/database: :memory:/, output) assert_match(/down 001 Valid people have last names/, output) assert_match(/down 002 We need reminders/, output) assert_match(/down 003 Innocent jointable/, output) - ActiveRecord::SchemaMigration.drop_table + @schema_migration.drop_table end private @@ -1169,25 +1173,27 @@ class DatabaseTasksTruncateAllTest < ActiveRecord::TestCase fixtures :authors, :author_addresses def setup - SchemaMigration.create_table - SchemaMigration.create!(version: SchemaMigration.table_name) + @schema_migration = ActiveRecord::Base.connection.schema_migration + @schema_migration.create_table + @schema_migration.create_version(@schema_migration.table_name) InternalMetadata.create_table InternalMetadata.create!(key: InternalMetadata.table_name) + @old_configurations = ActiveRecord::Base.configurations end def teardown - SchemaMigration.delete_all + @schema_migration.delete_all_versions InternalMetadata.delete_all clean_up_connection_handler + ActiveRecord::Base.configurations = @old_configurations end def test_truncate_tables - assert_operator SchemaMigration.count, :>, 0 + assert_operator @schema_migration.count, :>, 0 assert_operator InternalMetadata.count, :>, 0 assert_operator Author.count, :>, 0 assert_operator AuthorAddress.count, :>, 0 - old_configurations = ActiveRecord::Base.configurations db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary") configurations = { development: db_config.configuration_hash } ActiveRecord::Base.configurations = configurations @@ -1198,12 +1204,10 @@ def test_truncate_tables ) end - assert_operator SchemaMigration.count, :>, 0 + assert_operator @schema_migration.count, :>, 0 assert_operator InternalMetadata.count, :>, 0 assert_equal 0, Author.count assert_equal 0, AuthorAddress.count - ensure - ActiveRecord::Base.configurations = old_configurations end end @@ -1211,14 +1215,12 @@ class DatabaseTasksTruncateAllWithPrefixTest < DatabaseTasksTruncateAllTest setup do ActiveRecord::Base.table_name_prefix = "p_" - SchemaMigration.reset_table_name InternalMetadata.reset_table_name end teardown do ActiveRecord::Base.table_name_prefix = nil - SchemaMigration.reset_table_name InternalMetadata.reset_table_name end end @@ -1227,14 +1229,12 @@ class DatabaseTasksTruncateAllWithSuffixTest < DatabaseTasksTruncateAllTest setup do ActiveRecord::Base.table_name_suffix = "_s" - SchemaMigration.reset_table_name InternalMetadata.reset_table_name end teardown do ActiveRecord::Base.table_name_suffix = nil - SchemaMigration.reset_table_name InternalMetadata.reset_table_name end end diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index 2015171385..af11368b53 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -135,14 +135,14 @@ class Post < ApplicationRecord initial = [ ActiveStorage::Record, ActiveStorage::Blob, ActiveStorage::Attachment, - ActiveRecord::SchemaMigration, ActiveRecord::InternalMetadata, ApplicationRecord + 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"], ActiveRecord::Base.descendants.collect(&:to_s).sort.uniq + assert_equal ["ActiveRecord::InternalMetadata"], ActiveRecord::Base.descendants.collect(&:to_s).sort.uniq end test "initialize can't be called twice" do diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 5c16a18ae0..8bc60d5349 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -34,7 +34,7 @@ def boot_rails def migrations migration_root = File.expand_path(ActiveRecord::Migrator.migrations_paths.first, app_path) - ActiveRecord::MigrationContext.new(migration_root, ActiveRecord::SchemaMigration).migrations + ActiveRecord::MigrationContext.new(migration_root, ActiveRecord::SchemaMigration::NullSchemaMigration.new).migrations end test "serving sprocket's assets" do