added ability to specify from cli when generating a model/migration whether particular property should be an index like this 'rails g model person name:string:index profile:string'

12 files changed, 200 insertions, 18 deletions

diff --git a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
index f6159deeeb..2db950fefb 100644
--- a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
+++ b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
@@ -3,7 +3,7 @@ require 'rails/generators/active_record'
 module ActiveRecord
   module Generators
     class MigrationGenerator < Base
-      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type"
+      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type field:type:index"
 
       def create_migration_file
         set_local_assigns!
diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb
index ce8d7eed42..53018ffc9a 100644
--- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb
+++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb
@@ -2,14 +2,20 @@ class <%= migration_class_name %> < ActiveRecord::Migration
 <%- if migration_action == 'add' -%>
   def change
 <% attributes.each do |attribute| -%>
-    add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %>
+    add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %>
+    <%- if attribute.has_index? -%>
+    add_index :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_index_options %>
+    <%- end %>
 <%- end -%>
   end
 <%- else -%>
   def up
 <% attributes.each do |attribute| -%>
   <%- if migration_action -%>
-    <%= migration_action %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'add' %>, :<%= attribute.type %><% end %>
+    <%= migration_action %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'add' %>, :<%= attribute.type %><%= attribute.inject_options %><% end %>
+    <% if attribute.has_index? && migration_action == 'add' %>
+    add_index :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_index_options %>
+    <% end -%>
   <%- end -%>
 <%- end -%>
   end
@@ -17,7 +23,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration
   def down
 <% attributes.reverse.each do |attribute| -%>
   <%- if migration_action -%>
-    <%= migration_action == 'add' ? 'remove' : 'add' %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'remove' %>, :<%= attribute.type %><% end %>
+    <%= migration_action == 'add' ? 'remove' : 'add' %>_column :<%= table_name %>, :<%= attribute.name %><% if migration_action == 'remove' %>, :<%= attribute.type %><%= attribute.inject_options %><% end %>
   <%- end -%>
 <%- end -%>
   end
diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb
index f7caa43ac8..c0527e231f 100644
--- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb
+++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb
@@ -3,7 +3,7 @@ require 'rails/generators/active_record'
 module ActiveRecord
   module Generators
     class ModelGenerator < Base
-      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type"
+      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type field:type:index"
 
       check_class_collision
 
diff --git a/activerecord/lib/rails/generators/active_record/model/templates/migration.rb b/activerecord/lib/rails/generators/active_record/model/templates/migration.rb
index 851930344a..0715d0262b 100644
--- a/activerecord/lib/rails/generators/active_record/model/templates/migration.rb
+++ b/activerecord/lib/rails/generators/active_record/model/templates/migration.rb
@@ -2,7 +2,7 @@ class <%= migration_class_name %> < ActiveRecord::Migration
   def change
     create_table :<%= table_name %> do |t|
 <% attributes.each do |attribute| -%>
-      t.<%= attribute.type %> :<%= attribute.name %>
+      t.<%= attribute.type %> :<%= attribute.name %><%= attribute.inject_options %>
 <% end -%>
 <% if options[:timestamps] %>
       t.timestamps
@@ -10,8 +10,11 @@ class <%= migration_class_name %> < ActiveRecord::Migration
     end
 <% if options[:indexes] -%>
 <% attributes.select {|attr| attr.reference? }.each do |attribute| -%>
-    add_index :<%= table_name %>, :<%= attribute.name %>_id
+    add_index :<%= table_name %>, :<%= attribute.name %>_id<%= attribute.inject_index_options %>
 <% end -%>
 <% end -%>
+<% attributes.select {|attr| attr.has_index? }.each do |attribute| -%>
+    add_index :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_index_options %>
+<% end -%>
   end
 end
diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb
index 816d82cac3..5ba37737a2 100644
--- a/railties/lib/rails/generators/generated_attribute.rb
+++ b/railties/lib/rails/generators/generated_attribute.rb
@@ -4,11 +4,10 @@ require 'active_support/core_ext/object/inclusion'
 module Rails
   module Generators
     class GeneratedAttribute
-      attr_accessor :name, :type
+      attr_accessor :name, :type, :has_index, :attr_options
 
-      def initialize(name, type)
-        type = :string if type.blank?
-        @name, @type = name, type.to_sym
+      def initialize(column_definition)
+        parse column_definition
       end
 
       def field_type
@@ -48,6 +47,51 @@ module Rails
       def reference?
         self.type.in?([:references, :belongs_to])
       end
+      
+      def has_index?
+        @has_index
+      end
+
+      def has_uniq_index?
+        @has_uniq_index
+      end
+
+      def parse(column_definition)
+        name, type, has_index = 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
+        if type =~ /index|uniq|unique/i
+          has_index = type
+          type = nil
+        end
+        type = :string if type.blank?
+
+        @name = name
+        @type, @attr_options = *parse_type_and_options(type)
+        @has_index = ['index','uniq','unique'].include?(has_index)
+        @has_uniq_index = ['uniq','unique'].include?(has_index)
+      end
+
+      # parse possible attribute options like :limit for string/text/binary/integer or :precision/:scale for decimals
+      # when declaring options curly brackets should be used
+      def parse_type_and_options(type)
+        attribute_options = case type 
+          when /(string|text|binary|integer){(\d+)}/
+            {:limit => $2.to_i}
+          when /decimal{(\d+),(\d+)}/
+            {:precision => $1.to_i, :scale => $2.to_i}
+          else; {}
+        end
+        [type.to_s.gsub(/{.*}/,'').to_sym, attribute_options]
+      end
+
+      def inject_options
+        @attr_options.blank? ? '' : ", #{@attr_options.to_s.gsub(/[{}]/, '')}"
+      end
+
+      def inject_index_options
+        has_uniq_index? ? ", :unique => true" : ''
+      end
     end
   end
 end
diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb
index e96fc63ee9..baf7cb7dca 100644
--- a/railties/lib/rails/generators/named_base.rb
+++ b/railties/lib/rails/generators/named_base.rb
@@ -150,9 +150,8 @@ module Rails
 
         # Convert attributes array into GeneratedAttribute objects.
         def parse_attributes! #:nodoc:
-          self.attributes = (attributes || []).map do |key_value|
-            name, type = key_value.split(':')
-            Rails::Generators::GeneratedAttribute.new(name, type)
+          self.attributes = (attributes || []).map do |attr|
+            Rails::Generators::GeneratedAttribute.new(attr)
           end
         end
 
diff --git a/railties/lib/rails/generators/rails/migration/migration_generator.rb b/railties/lib/rails/generators/rails/migration/migration_generator.rb
index 39fa5b63b1..ecf0a77b76 100644
--- a/railties/lib/rails/generators/rails/migration/migration_generator.rb
+++ b/railties/lib/rails/generators/rails/migration/migration_generator.rb
@@ -1,7 +1,7 @@
 module Rails
   module Generators
     class MigrationGenerator < NamedBase #metagenerator
-      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type"
+      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type field:type:index"
       hook_for :orm, :required => true
     end
   end
diff --git a/railties/lib/rails/generators/rails/model/model_generator.rb b/railties/lib/rails/generators/rails/model/model_generator.rb
index 629d5eed3f..2a882981c3 100644
--- a/railties/lib/rails/generators/rails/model/model_generator.rb
+++ b/railties/lib/rails/generators/rails/model/model_generator.rb
@@ -1,7 +1,7 @@
 module Rails
   module Generators
     class ModelGenerator < NamedBase #metagenerator
-      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type"
+      argument :attributes, :type => :array, :default => [], :banner => "field:type field:type field:type:index"
       hook_for :orm, :required => true
     end
   end
diff --git a/railties/lib/rails/generators/test_case.rb b/railties/lib/rails/generators/test_case.rb
index 7319fb79f6..ab55031d07 100644
--- a/railties/lib/rails/generators/test_case.rb
+++ b/railties/lib/rails/generators/test_case.rb
@@ -219,7 +219,7 @@ module Rails
       #   create_generated_attribute(:string, 'name')
       #
       def create_generated_attribute(attribute_type, name = 'test')
-        Rails::Generators::GeneratedAttribute.new(name, attribute_type.to_s)
+        Rails::Generators::GeneratedAttribute.new([name, attribute_type.to_s].join(':'))
       end
 
       protected
diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb
index a85829085c..ea6c6e4e2c 100644
--- a/railties/test/generators/generated_attribute_test.rb
+++ b/railties/test/generators/generated_attribute_test.rb
@@ -69,7 +69,7 @@ class GeneratedAttributeTest < Rails::Generators::TestCase
   end
 
   def test_default_value_for_type
-    att = Rails::Generators::GeneratedAttribute.new("type", "string")
+    att = Rails::Generators::GeneratedAttribute.new("type:string")
     assert_equal("", att.default)
   end
 
diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb
index 337257df7d..6b6c9dee7c 100644
--- a/railties/test/generators/migration_generator_test.rb
+++ b/railties/test/generators/migration_generator_test.rb
@@ -58,6 +58,68 @@ class MigrationGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_add_migration_with_attributes_and_indices
+    migration = "add_title_with_index_and_body_to_posts"
+    run_generator [migration, "title:string:index", "body:text", "user_id:integer:unique"]
+
+    assert_migration "db/migrate/#{migration}.rb" do |content|
+      assert_method :change, content do |up|
+        assert_match(/add_column :posts, :title, :string/, up)
+        assert_match(/add_column :posts, :body, :text/, up)
+        assert_match(/add_column :posts, :user_id, :integer/, up)
+      end
+      assert_match(/add_index :posts, :title/, content)
+      assert_match(/add_index :posts, :user_id, :unique => true/, content)
+    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 |up|
+        assert_match(/add_column :books, :title, :string/, up)
+        assert_match(/add_column :books, :content, :text/, up)
+        assert_match(/add_column :books, :user_id, :integer/, up)
+      end
+      assert_not_match(/add_index :books, :title/, content)
+      assert_not_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"]
+
+    assert_migration "db/migrate/#{migration}.rb" do |content|
+      assert_method :change, content do |up|
+        assert_match(/add_column :posts, :title, :string/, up)
+        assert_match(/add_column :posts, :body, :text/, up)
+        assert_match(/add_column :posts, :user_uuid, :string/, up)
+      end
+      assert_match(/add_index :posts, :title/, content)
+      assert_match(/add_index :posts, :user_uuid, :unique => true/, content)
+    end
+  end
+
+  def test_add_migration_with_attributes_index_declaration_and_attribute_options
+    migration = "add_title_and_content_to_books"
+    run_generator [migration, "title:string{40}:index", "content:string{255}", "price:decimal{5,2}:index", "discount:decimal{3,2}:uniq"]
+
+    assert_migration "db/migrate/#{migration}.rb" do |content|
+      assert_method :change, content do |up|
+        assert_match(/add_column :books, :title, :string, :limit=>40/, up)
+        assert_match(/add_column :books, :content, :string, :limit=>255/, up)
+        assert_match(/add_column :books, :price, :decimal, :precision=>5, :scale=>2/, up)
+        assert_match(/add_column :books, :discount, :decimal, :precision=>3, :scale=>2/, up)
+      end
+      assert_match(/add_index :books, :title/, content)
+      assert_match(/add_index :books, :price/, content)
+      assert_match(/add_index :books, :discount, :unique => true/, content)
+    end
+  end
+
   def test_should_create_empty_migrations_if_name_not_start_with_add_or_remove
     migration = "create_books"
     run_generator [migration, "title:string", "content:text"]
diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb
index 1b0cb425c6..7a8cf1d8a7 100644
--- a/railties/test/generators/model_generator_test.rb
+++ b/railties/test/generators/model_generator_test.rb
@@ -113,6 +113,74 @@ class ModelGeneratorTest < Rails::Generators::TestCase
     end
   end
 
+  def test_migration_with_attributes_and_with_index
+    run_generator ["product", "name:string:index", "supplier_id:integer:index", "user_id:integer:uniq", "order_id:unique"]
+
+    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_match(/t\.string :order_id/, up)
+
+        assert_match(/add_index :products, :name/, up)
+        assert_match(/add_index :products, :supplier_id/, up)
+        assert_match(/add_index :products, :user_id, :unique => true/, up)
+        assert_match(/add_index :products, :order_id, :unique => true/, up)
+      end
+    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_not_match(/add_index :products, :name/, up)
+        assert_not_match(/add_index :products, :supplier_id/, up)
+        assert_not_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"]
+
+    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(/add_index :products, :name/, up)
+        assert_match(/add_index :products, :supplier_id/, up)
+        assert_not_match(/add_index :products, :year/, up)
+      end
+    end
+  end
+
+  def test_add_migration_with_attributes_index_declaration_and_attribute_options
+    run_generator ["product", "title:string{40}:index", "content:string{255}", "price:decimal{5,2}:index", "discount:decimal{5,2}:uniq"]
+
+    assert_migration "db/migrate/create_products.rb" do |content|
+      assert_method :change, content do |up|
+        assert_match(/create_table :products/, up)
+        assert_match(/t.string :title, :limit=>40/, up)
+        assert_match(/t.string :content, :limit=>255/, up)
+        assert_match(/t.decimal :price, :precision=>5, :scale=>2/, up)
+      end
+      assert_match(/add_index :products, :title/, content)
+      assert_match(/add_index :products, :price/, content)
+      assert_match(/add_index :products, :discount, :unique => true/, content)
+    end
+  end
+  
   def test_migration_without_timestamps
     ActiveRecord::Base.timestamped_migrations = false
     run_generator ["account"]