Add active_record.config.validate_migration_timestamps
config option.
#50400
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Background
Take two of #50205, reverted here.
As discussed in the revert PR, forward-dated migrations are the main reason hand-edited migrations can be problematic.
Rather than validating that a timestamp is a "real" timestamp in the form we expect, as proposed in the first PR, we reject any migrations with timestamp prefixes greater than "one day from the current timestamp". The
1.day
allows us to forward date migrations within a narrow range, when e.g. copying engine migrations, while preventing prefixes that would actually pose problems for future migration generation.Detail
Adds an
active_record.config.validate_migration_timestamps
config option. When set, anActiveRecord::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.It is turned off by default, but will be turned on for applications starting in Rails 7.2.
Additional information
There was some discussion in #50231 about string vs numeric based maxes. We are inconsistent with which one we use:
#next_migration_version
uses a string-based max:rails/activerecord/lib/active_record/migration.rb
Lines 1118 to 1120 in b0048c7
But
#current_version
uses a numeric one:rails/activerecord/lib/active_record/migration.rb
Lines 1302 to 1305 in b0048c7
For the purposes of this PR, I believe we should compare using numeric-based maxes: this ensures that migration prefixes that are too long (ie. more than 14 digits) will be flagged as invalid. There was a previous PR that changed
#next_migration_version
to use an integer-based maxed, but it was reverted because it broke existing migrations with timestamps that were too long (one of the cases we're trying to prevent with this PR). We may want to consider bringing in those changes so that we consistently assume "current migration" to mean "migration with numeric-based max version".It's also worth noting that it's hard for existing applications to opt-into this if they already have problematic migrations (as we do at Shopify). Should we enforce that applications roll these migrations up, or modify the filenames?
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
cc @eileencodes @matthewd @fxn