Improve the derivation of HABTM assocation join table names

Improve the derivation of HABTM join table name to take account of nesting.
It now takes the table names of the two models, sorts them lexically and
then joins them, stripping any common prefix from the second table name.

Some examples:

  Top level models
  (Category <=> Product)
  Old: categories_products
  New: categories_products

  Top level models with a global table_name_prefix
  (Category <=> Product)
  Old: site_categories_products
  New: site_categories_products

  Nested models in a module without a table_name_prefix method
  (Admin::Category <=> Admin::Product)
  Old: categories_products
  New: categories_products

  Nested models in a module with a table_name_prefix method
  (Admin::Category <=> Admin::Product)
  Old: categories_products
  New: admin_categories_products

  Nested models in a parent model
  (Catalog::Category <=> Catalog::Product)
  Old: categories_products
  New: catalog_categories_products

  Nested models in different parent models
  (Catalog::Category <=> Content::Page)
  Old: categories_pages
  New: catalog_categories_content_pages

Also as part of this commit the validity checks for HABTM assocations have
been moved to ActiveRecord::Reflection One side effect of this is to move when
the exceptions are raised from the point of declaration to when the association
is built. This is consistant with other association validity checks.

6 files changed, 20 insertions, 34 deletions

diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index 30fc44b4c2..f7656ecd47 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -6,7 +6,6 @@ module ActiveRecord::Associations::Builder
 
     def build
       reflection = super
-      check_validity(reflection)
       define_destroy_hook
       reflection
     end
@@ -24,34 +23,5 @@ module ActiveRecord::Associations::Builder
           RUBY
         })
       end
-
-      # TODO: These checks should probably be moved into the Reflection, and we should not be
-      #       redefining the options[:join_table] value - instead we should define a
-      #       reflection.join_table method.
-      def check_validity(reflection)
-        if reflection.association_foreign_key == reflection.foreign_key
-          raise ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection)
-        end
-
-        reflection.options[:join_table] ||= join_table_name(
-          model.send(:undecorated_table_name, model.to_s),
-          model.send(:undecorated_table_name, reflection.class_name)
-        )
-      end
-
-      # Generates a join table name from two provided table names.
-      # The names in the join table names end up in lexicographic order.
-      #
-      #   join_table_name("members", "clubs")         # => "clubs_members"
-      #   join_table_name("members", "special_clubs") # => "members_special_clubs"
-      def join_table_name(first_table_name, second_table_name)
-        if first_table_name < second_table_name
-          join_table = "#{first_table_name}_#{second_table_name}"
-        else
-          join_table = "#{second_table_name}_#{first_table_name}"
-        end
-
-        model.table_name_prefix + join_table + model.table_name_suffix
-      end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 58d041ec1d..93618721bb 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -5,7 +5,7 @@ module ActiveRecord
       attr_reader :join_table
 
       def initialize(owner, reflection)
-        @join_table = Arel::Table.new(reflection.options[:join_table])
+        @join_table = Arel::Table.new(reflection.join_table)
         super
       end
 
diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb
index cea6ad6944..5a41b40c8f 100644
--- a/activerecord/lib/active_record/associations/join_helper.rb
+++ b/activerecord/lib/active_record/associations/join_helper.rb
@@ -19,7 +19,7 @@ module ActiveRecord
 
           if reflection.source_macro == :has_and_belongs_to_many
             tables << alias_tracker.aliased_table_for(
-              (reflection.source_reflection || reflection).options[:join_table],
+              (reflection.source_reflection || reflection).join_table,
               table_alias_for(reflection, true)
             )
           end
diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb
index b77b667219..8e8925f0a9 100644
--- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb
@@ -6,7 +6,7 @@ module ActiveRecord
 
         def initialize(klass, records, reflection, preload_options)
           super
-          @join_table = Arel::Table.new(options[:join_table]).alias('t0')
+          @join_table = Arel::Table.new(reflection.join_table).alias('t0')
         end
 
         # Unlike the other associations, we want to get a raw array of rows so that we can
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 7e6512501c..96d24b72b3 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -594,7 +594,7 @@ module ActiveRecord
             when :has_and_belongs_to_many
               if (targets = row.delete(association.name.to_s))
                 targets = targets.is_a?(Array) ? targets : targets.split(/\s*,\s*/)
-                table_name = association.options[:join_table]
+                table_name = association.join_table
                 rows[table_name].concat targets.map { |target|
                   { association.foreign_key             => row[primary_key_name],
                     association.association_foreign_key => ActiveRecord::Fixtures.identify(target) }
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index e9a8e90538..0d9534acd6 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -161,6 +161,10 @@ module ActiveRecord
         @quoted_table_name ||= klass.quoted_table_name
       end
 
+      def join_table
+        @join_table ||= options[:join_table] || derive_join_table
+      end
+
       def foreign_key
         @foreign_key ||= options[:foreign_key] || derive_foreign_key
       end
@@ -208,6 +212,10 @@ module ActiveRecord
 
       def check_validity!
         check_validity_of_inverse!
+
+        if has_and_belongs_to_many? && association_foreign_key == foreign_key
+          raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(self)
+        end
       end
 
       def check_validity_of_inverse!
@@ -290,6 +298,10 @@ module ActiveRecord
         macro == :belongs_to
       end
 
+      def has_and_belongs_to_many?
+        macro == :has_and_belongs_to_many
+      end
+
       def association_class
         case macro
         when :belongs_to
@@ -332,6 +344,10 @@ module ActiveRecord
           end
         end
 
+        def derive_join_table
+          [active_record.table_name, klass.table_name].sort.join("\0").gsub(/^(.*_)(.+)\0\1(.+)/, '\1\2_\3').gsub("\0", "_")
+        end
+
         def primary_key(klass)
           klass.primary_key || raise(UnknownPrimaryKey.new(klass))
         end