mirror of https://github.com/rails/rails
Revert "Add config for validating migration timestamps"
This commit is contained in:
parent
d6197c5efc
commit
2854e378c8
|
@ -1,10 +1,3 @@
|
|||
* Add `active_record.config.validate_migration_timestamps` option for validating migration timestamps.
|
||||
|
||||
When set, validates that the timestamp prefix for a migration is in the form YYYYMMDDHHMMSS.
|
||||
This is designed to prevent migration timestamps from being modified by hand.
|
||||
|
||||
*Adrianna Chang*
|
||||
|
||||
* When using a `DATABASE_URL`, allow for a configuration to map the protocol in the URL to a specific database
|
||||
adapter. This allows decoupling the adapter the application chooses to use from the database connection details
|
||||
set in the deployment environment.
|
||||
|
|
|
@ -370,14 +370,6 @@ module ActiveRecord
|
|||
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 not a valid timestamp in the form
|
||||
# YYYYMMDDHHMMSS. +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.
|
||||
|
|
|
@ -131,16 +131,6 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
class InvalidMigrationTimestampError < MigrationError # :nodoc:
|
||||
def initialize(version = nil, name = nil)
|
||||
if version && name
|
||||
super("Invalid timestamp #{version} for migration file: #{name}.\nTimestamp should be in form YYYYMMDDHHMMSS.")
|
||||
else
|
||||
super("Invalid timestamp for migration. Timestamp should be in form YYYYMMDDHHMMSS.")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class PendingMigrationError < MigrationError # :nodoc:
|
||||
include ActiveSupport::ActionableError
|
||||
|
||||
|
@ -503,11 +493,7 @@ module ActiveRecord
|
|||
#
|
||||
# 20080717013526_your_migration_name.rb
|
||||
#
|
||||
# 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
|
||||
# The prefix is a generation timestamp (in UTC).
|
||||
#
|
||||
# If you'd prefer to use numeric prefixes, you can turn timestamped migrations
|
||||
# off by setting:
|
||||
|
@ -1330,9 +1316,6 @@ module ActiveRecord
|
|||
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
|
||||
|
||||
|
@ -1348,9 +1331,6 @@ module ActiveRecord
|
|||
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]
|
||||
|
@ -1395,22 +1375,6 @@ module ActiveRecord
|
|||
File.basename(filename).scan(Migration::MigrationFilenameRegexp).first
|
||||
end
|
||||
|
||||
def validate_timestamp?
|
||||
ActiveRecord.timestamped_migrations && ActiveRecord.validate_migration_timestamps
|
||||
end
|
||||
|
||||
def valid_migration_timestamp?(version)
|
||||
timestamp = "#{version} UTC"
|
||||
timestamp_format = "%Y%m%d%H%M%S %Z"
|
||||
time = Time.strptime(timestamp, timestamp_format)
|
||||
|
||||
# Time.strptime will wrap if a day between 1-31 is specified, even if the day doesn't exist
|
||||
# for the given month. Comparing against the original timestamp guards against this.
|
||||
time.strftime(timestamp_format) == timestamp
|
||||
rescue ArgumentError
|
||||
false
|
||||
end
|
||||
|
||||
def move(direction, steps)
|
||||
migrator = Migrator.new(direction, migrations, schema_migration, internal_metadata)
|
||||
|
||||
|
|
|
@ -1802,102 +1802,3 @@ class CopyMigrationsTest < ActiveRecord::TestCase
|
|||
assert_raise(ArgumentError) { ActiveRecord::Migration[1.0] }
|
||||
end
|
||||
end
|
||||
|
||||
class MigrationValidationTest < ActiveRecord::TestCase
|
||||
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"
|
||||
@migrator = ActiveRecord::MigrationContext.new(@migrations_path, @schema_migration, @internal_metadata)
|
||||
end
|
||||
|
||||
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_not_14_digits
|
||||
with_temp_migration_file(@migrations_path, "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
|
||||
|
||||
with_temp_migration_file(@migrations_path, "201801010_test_migration.rb") do
|
||||
error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do
|
||||
@migrator.up(201801010)
|
||||
end
|
||||
assert_match(/Invalid timestamp 201801010 for migration file: test_migration/, error.message)
|
||||
end
|
||||
end
|
||||
|
||||
def test_migration_raises_if_timestamp_is_invalid_date
|
||||
# 2023-15-26 is an invalid date
|
||||
with_temp_migration_file(@migrations_path, "20231526101010_test_migration.rb") do
|
||||
error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do
|
||||
@migrator.up(20231526101010)
|
||||
end
|
||||
assert_match(/Invalid timestamp 20231526101010 for migration file: test_migration/, error.message)
|
||||
end
|
||||
end
|
||||
|
||||
def test_migration_raises_if_timestamp_is_invalid_day_month_combo
|
||||
# 2023-02-31 is an invalid date; 02 and 31 are valid month / days individually,
|
||||
# but February 31 does not exist.
|
||||
with_temp_migration_file(@migrations_path, "20230231101010_test_migration.rb") do
|
||||
error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do
|
||||
@migrator.up(20230231101010)
|
||||
end
|
||||
assert_match(/Invalid timestamp 20230231101010 for migration file: test_migration/, error.message)
|
||||
end
|
||||
end
|
||||
|
||||
def test_migration_succeeds_despite_invalid_timestamp_if_validate_timestamps_is_false
|
||||
validate_migration_timestamps_was = ActiveRecord.validate_migration_timestamps
|
||||
ActiveRecord.validate_migration_timestamps = false
|
||||
with_temp_migration_file(@migrations_path, "201801010_test_migration.rb") do
|
||||
@migrator.up(201801010)
|
||||
assert_equal 201801010, @migrator.current_version
|
||||
end
|
||||
ensure
|
||||
ActiveRecord.validate_migration_timestamps = validate_migration_timestamps_was
|
||||
end
|
||||
|
||||
def test_migration_succeeds_despite_invalid_timestamp_if_timestamped_migrations_is_false
|
||||
timestamped_migrations_was = ActiveRecord.timestamped_migrations
|
||||
ActiveRecord.timestamped_migrations = false
|
||||
with_temp_migration_file(@migrations_path, "201801010_test_migration.rb") do
|
||||
@migrator.up(201801010)
|
||||
assert_equal 201801010, @migrator.current_version
|
||||
end
|
||||
ensure
|
||||
ActiveRecord.timestamped_migrations = timestamped_migrations_was
|
||||
end
|
||||
|
||||
private
|
||||
def with_temp_migration_file(migrations_dir, filename)
|
||||
Dir.mkdir(migrations_dir) unless Dir.exist?(migrations_dir)
|
||||
path = File.join(migrations_dir, filename)
|
||||
|
||||
File.open(path, "w+") do |file|
|
||||
file << <<-MIGRATION
|
||||
class TestMigration < ActiveRecord::Migration::Current
|
||||
def change; end
|
||||
end
|
||||
MIGRATION
|
||||
end
|
||||
|
||||
yield
|
||||
ensure
|
||||
File.delete(path) if File.exist?(path)
|
||||
Dir.rmdir(migrations_dir) if Dir.exist?(migrations_dir)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -58,10 +58,6 @@ 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`
|
||||
|
@ -1051,17 +1047,6 @@ 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 a timestamp is not a valid timestamp in the form YYYYMMDDHHMMSS. `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:
|
||||
|
|
|
@ -323,10 +323,6 @@ module Rails
|
|||
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
|
||||
|
|
|
@ -8,13 +8,3 @@
|
|||
#
|
||||
# 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. Migration timestamps must be
|
||||
# in the form YYYYMMDDHHMMSS, or an ActiveRecord::InvalidMigrationTimestampError
|
||||
# will be raised.
|
||||
#
|
||||
# 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
|
||||
|
|
|
@ -319,7 +319,6 @@ class LoadingTest < ActiveSupport::TestCase
|
|||
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
|
||||
|
|
|
@ -8,7 +8,6 @@ module ApplicationTests
|
|||
def setup
|
||||
build_app
|
||||
FileUtils.rm_rf("#{app_path}/config/environments")
|
||||
add_to_config("config.active_record.timestamped_migrations = false")
|
||||
end
|
||||
|
||||
def teardown
|
||||
|
@ -18,7 +17,7 @@ module ApplicationTests
|
|||
test "running migrations with given scope" do
|
||||
rails "generate", "model", "user", "username:string", "password:string"
|
||||
|
||||
app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION
|
||||
app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION
|
||||
class AMigration < ActiveRecord::Migration::Current
|
||||
end
|
||||
MIGRATION
|
||||
|
@ -201,8 +200,6 @@ module ApplicationTests
|
|||
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"
|
||||
|
@ -220,7 +217,7 @@ module ApplicationTests
|
|||
end
|
||||
|
||||
test "migration status without timestamps" do
|
||||
remove_from_config("config.active_record.timestamped_migrations = false")
|
||||
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"
|
||||
|
@ -239,8 +236,6 @@ module ApplicationTests
|
|||
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"
|
||||
|
@ -264,8 +259,6 @@ module ApplicationTests
|
|||
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"
|
||||
|
@ -290,8 +283,6 @@ module ApplicationTests
|
|||
|
||||
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"
|
||||
|
@ -391,6 +382,8 @@ module ApplicationTests
|
|||
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"
|
||||
|
@ -504,8 +497,6 @@ module ApplicationTests
|
|||
|
||||
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"
|
||||
|
@ -532,7 +523,7 @@ module ApplicationTests
|
|||
|
||||
rails("db:migrate")
|
||||
|
||||
app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION
|
||||
app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION
|
||||
class AMigration < ActiveRecord::Migration::Current
|
||||
def change
|
||||
User.first
|
||||
|
|
|
@ -10,7 +10,6 @@ module ApplicationTests
|
|||
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
|
||||
|
@ -780,13 +779,11 @@ module ApplicationTests
|
|||
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
|
||||
|
|
|
@ -74,8 +74,6 @@ module RailtiesTest
|
|||
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
|
||||
|
@ -127,8 +125,6 @@ module RailtiesTest
|
|||
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
|
||||
|
@ -189,8 +185,6 @@ module RailtiesTest
|
|||
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
|
||||
|
@ -231,8 +225,6 @@ module RailtiesTest
|
|||
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
|
||||
|
|
Loading…
Reference in New Issue