From 76f9b7531f7c643f7dca42260080d15aea87a718 Mon Sep 17 00:00:00 2001 From: Petrik Date: Sun, 4 Jul 2021 17:17:49 +0200 Subject: [PATCH] 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 d163fcd6208c9b0507746888c7fb4a6f934303ce where we started raising errors if the attribute types are invalid. --- railties/CHANGELOG.md | 4 ++++ .../lib/rails/generators/generated_attribute.rb | 16 ++++++++++++---- .../test/generators/generated_attribute_test.rb | 9 +++++++++ .../test/generators/migration_generator_test.rb | 15 --------------- .../test/generators/model_generator_test.rb | 17 ----------------- 5 files changed, 25 insertions(+), 36 deletions(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 1fa3b1d99b..1cdff9c945 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -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* diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 8112a06bfe..6867d029b9 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -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 diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb index 5064bd51b2..895cb74bb9 100644 --- a/railties/test/generators/generated_attribute_test.rb +++ b/railties/test/generators/generated_attribute_test.rb @@ -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 diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index 9bf4a680d4..97c63958d5 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -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"] diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index bc3df7d8ef..0e6981c72e 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -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"]