Fix add_reference options validated on < 7.1

Option validation was [added][1] for 7.1+ Migration classes, and
a compatibility layer was added to ensure that previous Migration
versions do not have their options validated. However, the add_reference
method was missing in the compatibility layer which results in pre 7.1
Migrations validating options passed to add_reference.

This commit fixes the issue by adding add_reference to the compatibility
layer. In addition to adding add_reference to the "no validation"
compatibility test, the test is refactored to run against each previous
migration version to ensure that they all behave consistently.

[1]: e6da3ebd6c
This commit is contained in:
Hartley McGuire 2024-01-09 14:28:13 -05:00 committed by Rafael Mendonça França
parent af2bbd5f66
commit 71b4e22301
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
3 changed files with 51 additions and 27 deletions

View File

@ -1,3 +1,8 @@
* Fix Migrations with versions older than 7.1 validating options given to
`add_reference`.
*Hartley McGuire*
* Add `<role>_types` class method to `ActiveRecord::DelegatedType` so that the delegated types can be instrospected
*JP Rosevear*

View File

@ -63,8 +63,10 @@ module ActiveRecord
column_name.is_a?(String) && /\W/.match?(column_name)
end
end
module TableDefinition
include LegacyIndexName
def column(name, type, **options)
options[:_skip_validate_options] = true
super
@ -97,6 +99,12 @@ module ActiveRecord
super
end
def add_reference(table_name, ref_name, **options)
options[:_skip_validate_options] = true
super
end
alias :add_belongs_to :add_reference
def create_table(table_name, **options)
options[:_uses_legacy_table_name] = true
options[:_skip_validate_options] = true

View File

@ -255,33 +255,6 @@ module ActiveRecord
end
end
def test_options_are_not_validated
migration = Class.new(ActiveRecord::Migration[4.2]) {
def migrate(x)
create_table :tests, wrong_id: false do |t|
t.references :some_table, wrong_primary_key: true
t.integer :some_id, wrong_unique: true
t.string :some_string_column, wrong_null: false
end
add_column :tests, "last_name", :string, wrong_precision: true
change_column :tests, :some_id, :float, wrong_index: true
change_table :tests do |t|
t.change :some_id, :float, null: false, wrong_index: true
t.integer :another_id, wrong_unique: true
end
end
}.new
ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
assert connection.table_exists?(:tests)
ensure
connection.drop_table :tests, if_exists: true
end
def test_create_table_allows_duplicate_column_names
migration = Class.new(ActiveRecord::Migration[5.2]) {
def migrate(x)
@ -681,6 +654,37 @@ module ActiveRecord
end
end
module NoOptionValidationTestCases
def test_options_are_not_validated
migration = Class.new(migration_class) {
def migrate(x)
create_table :tests, wrong_id: false do |t|
t.references :some_table, wrong_primary_key: true
t.integer :some_id, wrong_unique: true
t.string :some_string_column, wrong_null: false
end
add_column :tests, "last_name", :string, wrong_precision: true
change_column :tests, :some_id, :float, wrong_index: true
add_reference :tests, :another_table, invalid_option: :something
change_table :tests do |t|
t.change :some_id, :float, null: false, wrong_index: true
t.integer :another_id, wrong_unique: true
end
end
}.new
ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
assert connection.table_exists?(:tests)
ensure
connection.drop_table :tests, if_exists: true
end
end
module DefaultPrecisionImplicitTestCases
def test_datetime_doesnt_set_precision_on_change_table
create_migration = Class.new(migration_class) {
@ -838,6 +842,7 @@ end
class CompatibilityTest7_0 < BaseCompatibilityTest
include DefaultPrecisionSixTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -847,6 +852,7 @@ end
class CompatibilityTest6_1 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -856,6 +862,7 @@ end
class CompatibilityTest6_0 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -865,6 +872,7 @@ end
class CompatibilityTest5_2 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -874,6 +882,7 @@ end
class CompatibilityTest5_1 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -883,6 +892,7 @@ end
class CompatibilityTest5_0 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class
@ -892,6 +902,7 @@ end
class CompatibilityTest4_2 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
include NoOptionValidationTestCases
private
def migration_class