Generators should raise an error if an attribute has an invalid index type

When passing an invalid index type to a generator, the index is silently
ignored. For example when misspelling the index:

    bin/rails g model post title:string:indxe

Instead of silently ignoring the invalid index, the generator should
raise an error.

This continues the work in d163fcd620
where we started raising errors if the attribute types are invalid.
This commit is contained in:
Petrik 2021-07-04 17:17:49 +02:00
parent 7179dcb269
commit 76f9b7531f
5 changed files with 25 additions and 36 deletions

View File

@ -1,3 +1,7 @@
* Raise an error in generators if an index type is invalid.
*Petrik de Heus*
* `package.json` now uses a strict version constraint for Rails JavaScript packages on new Rails apps.
*Zachary Scott*, *Alex Ghiculescu*

View File

@ -33,12 +33,12 @@ module Rails
class << self
def parse(column_definition)
name, type, has_index = column_definition.split(":")
name, type, index_type = column_definition.split(":")
# if user provided "name:index" instead of "name:string:index"
# type should be set blank so GeneratedAttribute's constructor
# could set it to :string
has_index, type = type, nil if INDEX_OPTIONS.include?(type)
index_type, type = type, nil if valid_index_type?(type)
type, attr_options = *parse_type_and_options(type)
type = type.to_sym if type
@ -47,13 +47,17 @@ module Rails
raise Error, "Could not generate field '#{name}' with unknown type '#{type}'."
end
if index_type && !valid_index_type?(index_type)
raise Error, "Could not generate field '#{name}' with unknown index '#{index_type}'."
end
if type && reference?(type)
if UNIQ_INDEX_OPTIONS.include?(has_index)
if UNIQ_INDEX_OPTIONS.include?(index_type)
attr_options[:index] = { unique: true }
end
end
new(name, type, has_index, attr_options)
new(name, type, index_type, attr_options)
end
def valid_type?(type)
@ -61,6 +65,10 @@ module Rails
ActiveRecord::Base.connection.valid_type?(type)
end
def valid_index_type?(index_type)
INDEX_OPTIONS.include?(index_type.to_s)
end
def reference?(type)
[:references, :belongs_to].include? type
end

View File

@ -68,6 +68,15 @@ class GeneratedAttributeTest < Rails::Generators::TestCase
assert_match message, e.message
end
def test_field_type_with_unknown_index_type_raises_error
index_type = :unknown
e = assert_raise Rails::Generators::Error do
create_generated_attribute "string", "name", index_type
end
message = "Could not generate field 'name' with unknown index 'unknown'"
assert_match message, e.message
end
def test_default_value_is_integer
assert_field_default_value :integer, 1
end

View File

@ -159,21 +159,6 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
end
end
def test_add_migration_with_attributes_and_wrong_index_declaration
migration = "add_title_and_content_to_books"
run_generator [migration, "title:string:inex", "content:text", "user_id:integer:unik"]
assert_migration "db/migrate/#{migration}.rb" do |content|
assert_method :change, content do |change|
assert_match(/add_column :books, :title, :string/, change)
assert_match(/add_column :books, :content, :text/, change)
assert_match(/add_column :books, :user_id, :integer/, change)
end
assert_no_match(/add_index :books, :title/, content)
assert_no_match(/add_index :books, :user_id/, content)
end
end
def test_add_migration_with_attributes_without_type_and_index
migration = "add_title_with_index_and_body_to_posts"
run_generator [migration, "title:index", "body:text", "user_uuid:uniq"]

View File

@ -223,23 +223,6 @@ class ModelGeneratorTest < Rails::Generators::TestCase
end
end
def test_migration_with_attributes_and_with_wrong_index_declaration
run_generator ["product", "name:string", "supplier_id:integer:inex", "user_id:integer:unqu"]
assert_migration "db/migrate/create_products.rb" do |m|
assert_method :change, m do |up|
assert_match(/create_table :products/, up)
assert_match(/t\.string :name/, up)
assert_match(/t\.integer :supplier_id/, up)
assert_match(/t\.integer :user_id/, up)
assert_no_match(/add_index :products, :name/, up)
assert_no_match(/add_index :products, :supplier_id/, up)
assert_no_match(/add_index :products, :user_id/, up)
end
end
end
def test_migration_with_missing_attribute_type_and_with_index
run_generator ["product", "name:index", "supplier_id:integer:index", "year:integer"]