diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 955f01023ff5c..6e9df8ea34e5a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Add `active_record.config.validate_migration_timestamps` option for validating migration timestamps. + + When set, validates that the timestamp prefix for a migration is no more than a day ahead of + the timestamp associated with the current time. This is designed to prevent migrations prefixes + from being hand-edited to future timestamps, which impacts migration generation and other + migration commands. + + *Adrianna Chang* + * Add support for generated columns in SQLite3 adapter Generated columns (both stored and dynamic) are supported since version 3.31.0 of SQLite. diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 41aa1de0f396c..a94fa007a89ed 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -370,6 +370,14 @@ def self.global_executor_concurrency # :nodoc: singleton_class.attr_accessor :timestamped_migrations self.timestamped_migrations = true + ## + # :singleton-method: + # Specify whether or not to validate migration timestamps. When set, an error + # will be raised if a timestamp is more than a day ahead of the timestamp + # associated with the current time. +timestamped_migrations+ must be set to true. + singleton_class.attr_accessor :validate_migration_timestamps + self.validate_migration_timestamps = false + ## # :singleton-method: # Specify strategy to use for executing migrations. diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 36831a15762d7..18614c2c3eb50 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -131,6 +131,22 @@ def initialize(name = nil) end end + class InvalidMigrationTimestampError < MigrationError # :nodoc: + def initialize(version = nil, name = nil) + if version && name + super(<<-MSG.squish) + Invalid timestamp #{version} for migration file: #{name}.\nTimestamp must be in form YYYYMMDDHHMMSS, + and less than #{(Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S")}. + MSG + else + super(<<-MSG.squish) + Invalid timestamp for migration.\nTimestamp must be in form YYYYMMDDHHMMSS, + and less than #{(Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S")}. + MSG + end + end + end + class PendingMigrationError < MigrationError # :nodoc: include ActiveSupport::ActionableError @@ -493,7 +509,11 @@ def initialize # # 20080717013526_your_migration_name.rb # - # The prefix is a generation timestamp (in UTC). + # The prefix is a generation timestamp (in UTC). Timestamps should not be + # modified manually. To validate that migration timestamps adhere to the + # format Active Record expects, you can use the following configuration option: + # + # config.active_record.validate_migration_timestamps = true # # If you'd prefer to use numeric prefixes, you can turn timestamped migrations # off by setting: @@ -1316,6 +1336,9 @@ def migrations # :nodoc: migrations = migration_files.map do |file| version, name, scope = parse_migration_filename(file) raise IllegalMigrationNameError.new(file) unless version + if validate_timestamp? && !valid_migration_timestamp?(version) + raise InvalidMigrationTimestampError.new(version, name) + end version = version.to_i name = name.camelize @@ -1331,6 +1354,9 @@ def migrations_status # :nodoc: file_list = migration_files.filter_map do |file| version, name, scope = parse_migration_filename(file) raise IllegalMigrationNameError.new(file) unless version + if validate_timestamp? && !valid_migration_timestamp?(version) + raise InvalidMigrationTimestampError.new(version, name) + end version = schema_migration.normalize_migration_number(version) status = db_list.delete(version) ? "up" : "down" [status, version, (name + scope).humanize] @@ -1375,6 +1401,14 @@ def parse_migration_filename(filename) File.basename(filename).scan(Migration::MigrationFilenameRegexp).first end + def validate_timestamp? + ActiveRecord.timestamped_migrations && ActiveRecord.validate_migration_timestamps + end + + def valid_migration_timestamp?(version) + version.to_i < (Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S").to_i + end + def move(direction, steps) migrator = Migrator.new(direction, migrations, schema_migration, internal_metadata) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 7b23230371b11..5a1439c4858c6 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -1807,6 +1807,8 @@ def setup @verbose_was, ActiveRecord::Migration.verbose = ActiveRecord::Migration.verbose, false @schema_migration = ActiveRecord::Base.connection.schema_migration @internal_metadata = ActiveRecord::Base.connection.internal_metadata + @active_record_validate_timestamps_was = ActiveRecord.validate_migration_timestamps + ActiveRecord.validate_migration_timestamps = true ActiveRecord::Base.connection.schema_cache.clear! @migrations_path = MIGRATIONS_ROOT + "/temp" @@ -1816,15 +1818,69 @@ def setup def teardown @schema_migration.create_table @schema_migration.delete_all_versions + ActiveRecord.validate_migration_timestamps = @active_record_validate_timestamps_was ActiveRecord::Migration.verbose = @verbose_was end + def test_migration_raises_if_timestamp_greater_than_14_digits + with_temp_migration_files(["201801010101010000_test_migration.rb"]) do + error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do + @migrator.up(201801010101010000) + end + assert_match(/Invalid timestamp 201801010101010000 for migration file: test_migration/, error.message) + end + end + + def test_migration_raises_if_timestamp_is_future_date + timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i + with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do + error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do + @migrator.up(timestamp) + end + assert_match(/Invalid timestamp #{timestamp} for migration file: test_migration/, error.message) + end + end + + def test_migration_succeeds_if_timestamp_is_less_than_one_day_in_the_future + timestamp = (Time.now.utc + 1.minute).strftime("%Y%m%d%H%M%S").to_i + with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do + @migrator.up(timestamp) + assert_equal timestamp, @migrator.current_version + end + end + + def test_migration_succeeds_despite_future_timestamp_if_validate_timestamps_is_false + validate_migration_timestamps_was = ActiveRecord.validate_migration_timestamps + ActiveRecord.validate_migration_timestamps = false + + timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i + with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do + @migrator.up(timestamp) + assert_equal timestamp, @migrator.current_version + end + ensure + ActiveRecord.validate_migration_timestamps = validate_migration_timestamps_was + end + + def test_migration_succeeds_despite_future_timestamp_if_timestamped_migrations_is_false + timestamped_migrations_was = ActiveRecord.timestamped_migrations + ActiveRecord.timestamped_migrations = false + + timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i + with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do + @migrator.up(timestamp) + assert_equal timestamp, @migrator.current_version + end + ensure + ActiveRecord.timestamped_migrations = timestamped_migrations_was + end + def test_copied_migrations_at_timestamp_boundary_are_valid migrations_path_source = MIGRATIONS_ROOT + "/temp_source" migrations_path_dest = MIGRATIONS_ROOT + "/temp_dest" migrations = ["20180101010101_test_migration.rb", "20180101010102_test_migration_two.rb", "20180101010103_test_migration_three.rb"] - with_temp_migration_files(migrations_path_source, migrations) do + with_temp_migration_files(migrations, migrations_path_source) do travel_to(Time.utc(2023, 12, 1, 10, 10, 59)) do ActiveRecord::Migration.copy(migrations_path_dest, temp: migrations_path_source) @@ -1847,7 +1903,7 @@ def test_copied_migrations_at_timestamp_boundary_are_valid end private - def with_temp_migration_files(migrations_dir, filenames) + def with_temp_migration_files(filenames, migrations_dir = @migrations_path) Dir.mkdir(migrations_dir) unless Dir.exist?(migrations_dir) paths = [] diff --git a/guides/source/configuring.md b/guides/source/configuring.md index fbe877d4a10cb..ca0c3d68e0d61 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -58,6 +58,10 @@ NOTE: If you need to apply configuration directly to a class, use a [lazy load h Below are the default values associated with each target version. In cases of conflicting values, newer versions take precedence over older versions. +#### Default Values for Target Version 7.2 + +- [`config.active_record.validate_migration_timestamps`](#config-active-record-validate-migration-timestamps): `true` + #### Default Values for Target Version 7.1 - [`config.action_dispatch.debug_exception_log_level`](#config-action-dispatch-debug-exception-log-level): `:error` @@ -1047,6 +1051,20 @@ Specifies if an error should be raised if the order of a query is ignored during Controls whether migrations are numbered with serial integers or with timestamps. The default is `true`, to use timestamps, which are preferred if there are multiple developers working on the same application. +#### `config.active_record.validate_migration_timestamps` + +Controls whether to validate migration timestamps. When set, an error will be raised if the +timestamp prefix for a migration is more than a day ahead of the timestamp associated with the +current time. This is done to prevent forward-dating of migration files, which can impact migration +generation and other migration commands. `config.active_record.timestamped_migrations` must be set to `true`. + +The default value depends on the `config.load_defaults` target version: + +| Starting with version | The default value is | +| --------------------- | -------------------- | +| (original) | `false` | +| 7.2 | `true` | + #### `config.active_record.db_warnings_action` Controls the action to be taken when a SQL query produces a warning. The following options are available: diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index aa279063849e5..7e388575eddeb 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -323,6 +323,10 @@ def load_defaults(target_version) end when "7.2" load_defaults "7.1" + + if respond_to?(:active_record) + active_record.validate_migration_timestamps = true + end else raise "Unknown version #{target_version.to_s.inspect}" end diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt index fcf2024ecf0ce..23412933a9eba 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt @@ -8,3 +8,14 @@ # # Read the Guide for Upgrading Ruby on Rails for more info on each option. # https://guides.rubyonrails.org/upgrading_ruby_on_rails.html + +### +# Enable validation of migration timestamps. When set, an ActiveRecord::InvalidMigrationTimestampError +# will be raised if the timestamp prefix for a migration is more than a day ahead of the timestamp +# associated with the current time. This is done to prevent forward-dating of migration files, which can +# impact migration generation and other migration commands. +# +# Applications with existing timestamped migrations that do not adhere to the +# expected format can disable validation by setting this config to `false`. +#++ +# Rails.application.config.active_record.validate_migration_timestamps = true diff --git a/railties/test/application/loading_test.rb b/railties/test/application/loading_test.rb index a733952779136..913931589666c 100644 --- a/railties/test/application/loading_test.rb +++ b/railties/test/application/loading_test.rb @@ -319,6 +319,7 @@ class User test "columns migrations also trigger reloading" do add_to_config <<-RUBY config.enable_reloading = true + config.active_record.timestamped_migrations = false RUBY app_file "config/routes.rb", <<-RUBY diff --git a/railties/test/application/rake/migrations_test.rb b/railties/test/application/rake/migrations_test.rb index 11fcfcc5b91fb..e2e625b85850a 100644 --- a/railties/test/application/rake/migrations_test.rb +++ b/railties/test/application/rake/migrations_test.rb @@ -8,6 +8,7 @@ class RakeMigrationsTest < ActiveSupport::TestCase def setup build_app FileUtils.rm_rf("#{app_path}/config/environments") + add_to_config("config.active_record.timestamped_migrations = false") end def teardown @@ -17,7 +18,7 @@ def teardown test "running migrations with given scope" do rails "generate", "model", "user", "username:string", "password:string" - app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION + app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION class AMigration < ActiveRecord::Migration::Current end MIGRATION @@ -200,6 +201,8 @@ class TwoMigration < ActiveRecord::Migration::Current end test "migration status" do + remove_from_config("config.active_record.timestamped_migrations = false") + rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -217,7 +220,7 @@ class TwoMigration < ActiveRecord::Migration::Current end test "migration status without timestamps" do - add_to_config("config.active_record.timestamped_migrations = false") + remove_from_config("config.active_record.timestamped_migrations = false") rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" @@ -236,6 +239,8 @@ class TwoMigration < ActiveRecord::Migration::Current end test "migration status after rollback and redo" do + remove_from_config("config.active_record.timestamped_migrations = false") + rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -259,6 +264,8 @@ class TwoMigration < ActiveRecord::Migration::Current end test "migration status after rollback and forward" do + remove_from_config("config.active_record.timestamped_migrations = false") + rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -283,6 +290,8 @@ class TwoMigration < ActiveRecord::Migration::Current test "raise error on any move when current migration does not exist" do Dir.chdir(app_path) do + remove_from_config("config.active_record.timestamped_migrations = false") + rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -382,8 +391,6 @@ class TwoMigration < ActiveRecord::Migration::Current end test "migration status after rollback and redo without timestamps" do - add_to_config("config.active_record.timestamped_migrations = false") - rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -497,6 +504,8 @@ class TwoMigration < ActiveRecord::Migration::Current test "migration status migrated file is deleted" do Dir.chdir(app_path) do + remove_from_config("config.active_record.timestamped_migrations = false") + rails "generate", "model", "user", "username:string", "password:string" rails "generate", "migration", "add_email_to_users", "email:string" rails "db:migrate" @@ -523,7 +532,7 @@ class ApplicationRecord < ActiveRecord::Base rails("db:migrate") - app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION + app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION class AMigration < ActiveRecord::Migration::Current def change User.first diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index 34282f52b5af7..f652ac1dd562b 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -10,6 +10,7 @@ class RakeMultiDbsTest < ActiveSupport::TestCase def setup build_app(multi_db: true) FileUtils.rm_rf("#{app_path}/config/environments") + add_to_config("config.active_record.timestamped_migrations = false") end def teardown @@ -779,11 +780,13 @@ class TwoMigration < ActiveRecord::Migration::Current end test "db:migrate:status works on all databases" do + remove_from_config("config.active_record.timestamped_migrations = false") require "#{app_path}/config/environment" db_migrate_and_migrate_status end test "db:migrate:status:namespace works" do + remove_from_config("config.active_record.timestamped_migrations = false") require "#{app_path}/config/environment" ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config| db_migrate_namespaced db_config.name diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index a1cdf295088fb..43b8bd3535224 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -74,6 +74,8 @@ def migrations(database = nil) end test "copying migrations" do + add_to_config("config.active_record.timestamped_migrations = false") + @plugin.write "db/migrate/1_create_users.rb", <<-RUBY class CreateUsers < ActiveRecord::Migration::Current end @@ -125,6 +127,8 @@ def up end test "copying migrations to specific database" do + add_to_config("config.active_record.timestamped_migrations = false") + @plugin.write "db/migrate/1_create_users.rb", <<-RUBY class CreateUsers < ActiveRecord::Migration::Current end @@ -185,6 +189,8 @@ class Engine < ::Rails::Engine RUBY end + add_to_config("config.active_record.timestamped_migrations = false") + @plugin.write "db/migrate/1_create_users.rb", <<-RUBY class CreateUsers < ActiveRecord::Migration::Current end @@ -225,6 +231,8 @@ class Engine < ::Rails::Engine; end RUBY end + add_to_config("config.active_record.timestamped_migrations = false") + @core.write "db/migrate/1_create_users.rb", <<-RUBY class CreateUsers < ActiveRecord::Migration::Current; end RUBY