diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 222e803a3e..52aa1b265f 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -3,6 +3,7 @@ require "benchmark" require "set" require "zlib" +require "active_support/core_ext/enumerable" require "active_support/core_ext/module/attribute_accessors" require "active_support/actionable_error" @@ -553,21 +554,37 @@ module ActiveRecord # This class is used to verify that all migrations have been run before # loading a web page if config.active_record.migration_error is set to :page_load class CheckPending - def initialize(app) + def initialize(app, file_watcher: ActiveSupport::FileUpdateChecker) @app = app - @last_check = 0 + @needs_check = true + @mutex = Mutex.new + @file_watcher = file_watcher end def call(env) - mtime = ActiveRecord::Base.connection.migration_context.last_migration.mtime.to_i - if @last_check < mtime - ActiveRecord::Migration.check_pending!(connection) - @last_check = mtime + @mutex.synchronize do + @watcher ||= build_watcher do + @needs_check = true + ActiveRecord::Migration.check_pending!(connection) + @needs_check = false + end + + if @needs_check + @watcher.execute + else + @watcher.execute_if_updated + end end + @app.call(env) end private + def build_watcher(&block) + paths = Array(connection.migration_context.migrations_paths) + @file_watcher.new([], paths.index_with(["rb"]), &block) + end + def connection ActiveRecord::Base.connection end @@ -993,10 +1010,6 @@ module ActiveRecord File.basename(filename) end - def mtime - File.mtime filename - end - delegate :migrate, :announce, :write, :disable_ddl_transaction, to: :migration private @@ -1010,16 +1023,6 @@ module ActiveRecord end end - class NullMigration < MigrationProxy #:nodoc: - def initialize - super(nil, 0, nil, nil) - end - - def mtime - 0 - end - end - class MigrationContext #:nodoc: attr_reader :migrations_paths, :schema_migration @@ -1098,10 +1101,6 @@ module ActiveRecord migrations.any? end - def last_migration #:nodoc: - migrations.last || NullMigration.new - end - def migrations migrations = migration_files.map do |file| version, name, scope = parse_migration_filename(file) diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index f06a98bd98..c0998e8bc6 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -82,10 +82,11 @@ module ActiveRecord ActiveSupport.on_load(:active_record) { LogSubscriber.backtrace_cleaner = ::Rails.backtrace_cleaner } end - initializer "active_record.migration_error" do + initializer "active_record.migration_error" do |app| if config.active_record.delete(:migration_error) == :page_load config.app_middleware.insert_after ::ActionDispatch::Callbacks, - ActiveRecord::Migration::CheckPending + ActiveRecord::Migration::CheckPending, + file_watcher: app.config.file_watcher end end diff --git a/activerecord/test/cases/migration/pending_migrations_test.rb b/activerecord/test/cases/migration/pending_migrations_test.rb index 119bfd372a..dd6f88493f 100644 --- a/activerecord/test/cases/migration/pending_migrations_test.rb +++ b/activerecord/test/cases/migration/pending_migrations_test.rb @@ -7,34 +7,98 @@ module ActiveRecord if current_adapter?(:SQLite3Adapter) && !in_memory_db? class PendingMigrationsTest < ActiveRecord::TestCase setup do + @migration_dir = Dir.mktmpdir("activerecord-migrations-") + file = ActiveRecord::Base.connection.raw_connection.filename - @conn = ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:", migrations_paths: MIGRATIONS_ROOT + "/valid" + @conn = ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:", migrations_paths: @migration_dir source_db = SQLite3::Database.new file dest_db = ActiveRecord::Base.connection.raw_connection backup = SQLite3::Backup.new(dest_db, "main", source_db, "main") backup.step(-1) backup.finish + + ActiveRecord::Base.connection.drop_table "schema_migrations", if_exists: true + + @app = Minitest::Mock.new end teardown do @conn.release_connection if @conn ActiveRecord::Base.establish_connection :arunit + FileUtils.rm_rf(@migration_dir) + end + + def run_migrations + migrator = Base.connection.migration_context + capture(:stdout) { migrator.migrate } + end + + def create_migration(number, name) + filename = "#{number}_#{name.underscore}.rb" + File.write(File.join(@migration_dir, filename), <<~RUBY) + class #{name.classify} < ActiveRecord::Migration::Current + end + RUBY end def test_errors_if_pending - ActiveRecord::Base.connection.drop_table "schema_migrations", if_exists: true + create_migration "01", "create_foo" assert_raises ActiveRecord::PendingMigrationError do - CheckPending.new(Proc.new { }).call({}) + CheckPending.new(@app).call({}) + end + + # Continues failing + assert_raises ActiveRecord::PendingMigrationError do + CheckPending.new(@app).call({}) end end def test_checks_if_supported - ActiveRecord::SchemaMigration.create_table - migrator = Base.connection.migration_context - capture(:stdout) { migrator.migrate } + run_migrations - assert_nil CheckPending.new(Proc.new { }).call({}) + check_pending = CheckPending.new(@app) + + @app.expect :call, nil, [{}] + check_pending.call({}) + @app.verify + + # With cached result + @app.expect :call, nil, [{}] + check_pending.call({}) + @app.verify + end + + def test_okay_with_no_migrations + check_pending = CheckPending.new(@app) + + @app.expect :call, nil, [{}] + check_pending.call({}) + @app.verify + + # With cached result + @app.expect :call, nil, [{}] + check_pending.call({}) + @app.verify + end + + # Regression test for https://github.com/rails/rails/pull/29759 + def test_understands_migrations_created_out_of_order + # With a prior file before even initialization + create_migration "05", "create_bar" + run_migrations + + check_pending = CheckPending.new(@app) + + @app.expect :call, nil, [{}] + check_pending.call({}) + @app.verify + + # It understands the new migration created at 01 + create_migration "01", "create_foo" + assert_raises ActiveRecord::PendingMigrationError do + check_pending.call({}) + end end end end