Merge pull request #7028 from lexmag/join_table_indexes

Add indexes to create_join_table method

5 files changed, 51 insertions, 15 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 3c3f01223c..a246e6ace1 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -206,9 +206,12 @@ module ActiveRecord
         column_options = options.delete(:column_options) || {}
         column_options.reverse_merge!({:null => false})
 
+        t1_column, t2_column = [table_1, table_2].map{ |t| "#{t.to_s.singularize}_id" }
+
         create_table(join_table_name, options.merge!(:id => false)) do |td|
-          td.integer :"#{table_1.to_s.singularize}_id", column_options
-          td.integer :"#{table_2.to_s.singularize}_id", column_options
+          td.integer t1_column, column_options
+          td.integer t2_column, column_options
+          yield td if block_given?
         end
       end
 
diff --git a/activerecord/lib/active_record/migration/join_table.rb b/activerecord/lib/active_record/migration/join_table.rb
index 01a580781b..e880ae97bb 100644
--- a/activerecord/lib/active_record/migration/join_table.rb
+++ b/activerecord/lib/active_record/migration/join_table.rb
@@ -4,13 +4,11 @@ module ActiveRecord
       private
 
       def find_join_table_name(table_1, table_2, options = {})
-        options.delete(:table_name) { join_table_name(table_1, table_2) }
+        options.delete(:table_name) || join_table_name(table_1, table_2)
       end
 
       def join_table_name(table_1, table_2)
-        tables_names = [table_1, table_2].map(&:to_s).sort
-
-        tables_names.join("_").to_sym
+        [table_1, table_2].sort.join("_").to_sym
       end
     end
   end
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 1509e34473..c1f9d8962b 100644
--- a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
+++ b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb
@@ -11,15 +11,28 @@ module ActiveRecord
       end
 
       protected
-        attr_reader :migration_action
+      attr_reader :migration_action, :join_tables
 
-        def set_local_assigns!
-          if file_name =~ /^(add|remove)_.*_(?:to|from)_(.*)/
-            @migration_action = $1
-            @table_name       = $2.pluralize
+      def set_local_assigns!
+        case file_name
+        when /^(add|remove)_.*_(?:to|from)_(.*)/
+          @migration_action = $1
+          @table_name       = $2.pluralize
+        when /join_table/
+          if attributes.length == 2
+            @migration_action = 'join'
+            @join_tables      = attributes.map(&:name)
+
+            set_index_names
           end
         end
+      end
 
+      def set_index_names
+        attributes.each_with_index do |attr, i|
+          attr.index_name = [attr, attributes[i - 1]].map{ |a| :"#{a.name.singularize}_id"}
+        end
+      end
     end
   end
 end
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 34eaffcb0f..d5c07aecd3 100644
--- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb
+++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb
@@ -12,6 +12,14 @@ class <%= migration_class_name %> < ActiveRecord::Migration
   <%- end -%>
 <%- end -%>
   end
+<%- elsif migration_action == 'join' -%>
+  def change
+    create_join_table :<%= join_tables.first %>, :<%= join_tables.second %> do |t|
+    <%- attributes.each do |attribute| -%>
+      <%= '# ' unless attribute.has_index? -%>t.index <%= attribute.index_name %><%= attribute.inject_index_options %>
+    <%- end -%>
+    end
+  end
 <%- else -%>
   def up
 <% attributes.each do |attribute| -%>
@@ -40,4 +48,4 @@ class <%= migration_class_name %> < ActiveRecord::Migration
 <%- end -%>
   end
 <%- end -%>
-end
\ No newline at end of file
+end
diff --git a/activerecord/test/cases/migration/create_join_table_test.rb b/activerecord/test/cases/migration/create_join_table_test.rb
index 8b91b3bc92..cd1b0e8b47 100644
--- a/activerecord/test/cases/migration/create_join_table_test.rb
+++ b/activerecord/test/cases/migration/create_join_table_test.rb
@@ -42,22 +42,36 @@ module ActiveRecord
       end
 
       def test_create_join_table_with_the_table_name
-        connection.create_join_table :artists, :musics, :table_name => :catalog
+        connection.create_join_table :artists, :musics, table_name: :catalog
 
         assert_equal %w(artist_id music_id), connection.columns(:catalog).map(&:name).sort
       end
 
       def test_create_join_table_with_the_table_name_as_string
-        connection.create_join_table :artists, :musics, :table_name => 'catalog'
+        connection.create_join_table :artists, :musics, table_name: 'catalog'
 
         assert_equal %w(artist_id music_id), connection.columns(:catalog).map(&:name).sort
       end
 
       def test_create_join_table_with_column_options
-        connection.create_join_table :artists, :musics, :column_options => {:null => true}
+        connection.create_join_table :artists, :musics, column_options: {null: true}
 
         assert_equal [true, true], connection.columns(:artists_musics).map(&:null)
       end
+
+      def test_create_join_table_without_indexes
+        connection.create_join_table :artists, :musics
+
+        assert connection.indexes(:artists_musics).blank?
+      end
+
+      def test_create_join_table_with_index
+        connection.create_join_table :artists, :musics do |t|
+          t.index [:artist_id, :music_id]
+        end
+
+        assert_equal [%w(artist_id music_id)], connection.indexes(:artists_musics).map(&:columns)
+      end
     end
   end
 end