Abstract some common code from AssociationScope and JoinDependency::JoinAssociation into a JoinHelper module

4 files changed, 83 insertions, 97 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 90745112b1..08fb6bf3c4 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -144,6 +144,7 @@ module ActiveRecord
     autoload :JoinDependency,   'active_record/associations/join_dependency'
     autoload :AssociationScope, 'active_record/associations/association_scope'
     autoload :AliasTracker,     'active_record/associations/alias_tracker'
+    autoload :JoinHelper,       'active_record/associations/join_helper'
 
     # Clears out the association cache.
     def clear_association_cache #:nodoc:
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index cf859bbdee..b9bbeed4d5 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -1,10 +1,12 @@
 module ActiveRecord
   module Associations
     class AssociationScope #:nodoc:
+      include JoinHelper
+
       attr_reader :association, :alias_tracker
 
       delegate :klass, :owner, :reflection, :interpolate, :to => :association
-      delegate :chain, :conditions, :options, :source_options, :to => :reflection
+      delegate :chain, :conditions, :options, :source_options, :active_record, :to => :reflection
 
       def initialize(association)
         @association   = association
@@ -56,8 +58,8 @@ module ActiveRecord
           if reflection.source_macro == :has_and_belongs_to_many
             join_table = tables.shift
 
-            scope = scope.joins(inner_join(
-              join_table, reflection,
+            scope = scope.joins(join(
+              join_table,
               table[reflection.active_record_primary_key].
                 eq(join_table[reflection.association_foreign_key])
             ))
@@ -84,32 +86,19 @@ module ActiveRecord
               scope = scope.where(interpolate(condition))
             end
           else
-            constraint = table[key].eq foreign_table[foreign_key]
-
-            join  = inner_join(foreign_table, reflection, constraint, *conditions[i])
-            scope = scope.joins(join)
+            scope = scope.joins(join(
+              foreign_table,
+              table[key].eq(foreign_table[foreign_key]),
+              *conditions[i]
+            ))
           end
         end
 
         scope
       end
 
-      def construct_tables
-        tables = []
-        chain.each do |reflection|
-          tables << alias_tracker.aliased_table_for(
-            table_name_for(reflection),
-            table_alias_for(reflection, reflection != self.reflection)
-          )
-
-          if reflection.source_macro == :has_and_belongs_to_many
-            tables << alias_tracker.aliased_table_for(
-              (reflection.source_reflection || reflection).options[:join_table],
-              table_alias_for(reflection, true)
-            )
-          end
-        end
-        tables
+      def alias_suffix
+        reflection.name
       end
 
       def table_name_for(reflection)
@@ -123,27 +112,6 @@ module ActiveRecord
         end
       end
 
-      def table_alias_for(reflection, join = false)
-        name = alias_tracker.pluralize(reflection.name)
-        name << "_#{self.reflection.name}"
-        name << "_join" if join
-        name
-      end
-
-      def inner_join(table, reflection, *conditions)
-        conditions = sanitize_conditions(reflection, conditions)
-        table.create_join(table, table.create_on(conditions))
-      end
-
-      def sanitize_conditions(reflection, conditions)
-        conditions = conditions.map do |condition|
-          condition = reflection.klass.send(:sanitize_sql, interpolate(condition), reflection.table_name)
-          condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
-          condition
-        end
-
-        conditions.length == 1 ? conditions.first : Arel::Nodes::And.new(conditions)
-      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
index 750f03a120..7dc6beeede 100644
--- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb
+++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -2,6 +2,8 @@ module ActiveRecord
   module Associations
     class JoinDependency # :nodoc:
       class JoinAssociation < JoinPart # :nodoc:
+        include JoinHelper
+
         # The reflection of the association represented
         attr_reader :reflection
 
@@ -26,6 +28,8 @@ module ActiveRecord
         delegate :table, :table_name, :to => :parent, :prefix => :parent
         delegate :alias_tracker, :to => :join_dependency
 
+        alias :alias_suffix :parent_table_name
+
         def initialize(reflection, join_dependency, parent = nil)
           reflection.check_validity!
 
@@ -40,8 +44,7 @@ module ActiveRecord
           @parent          = parent
           @join_type       = Arel::InnerJoin
           @aliased_prefix  = "t#{ join_dependency.join_parts.size }"
-
-          setup_tables
+          @tables          = construct_tables.reverse
         end
 
         def ==(other)
@@ -86,14 +89,17 @@ module ActiveRecord
               foreign_key = reflection.active_record_primary_key
             end
 
-            conditions = self.conditions[i].dup
-            conditions << table[key].eq(foreign_table[foreign_key])
+            conditions = self.conditions[i]
 
             if reflection.klass.finder_needs_type_condition?
-              conditions << reflection.klass.send(:type_condition, table)
+              conditions += [reflection.klass.send(:type_condition, table)]
             end
 
-            relation = relation.from(join(table, *conditions))
+            relation = relation.from(join(
+              table,
+              table[key].eq(foreign_table[foreign_key]),
+              *conditions
+            ))
 
             # The current table in this iteration becomes the foreign table in the next
             foreign_table = table
@@ -121,53 +127,6 @@ module ActiveRecord
 
         private
 
-        def table_alias_for(reflection, join = false)
-          name = alias_tracker.pluralize(reflection.name)
-          name << "_#{parent_table_name}"
-          name << "_join" if join
-          name
-        end
-
-        # Generate aliases and Arel::Table instances for each of the tables which we will
-        # later generate joins for. We must do this in advance in order to correctly allocate
-        # the proper alias.
-        def setup_tables
-          @tables = []
-          chain.each do |reflection|
-            @tables << alias_tracker.aliased_table_for(
-              reflection.table_name,
-              table_alias_for(reflection, reflection != self.reflection)
-            )
-
-            if reflection.source_macro == :has_and_belongs_to_many
-              @tables << alias_tracker.aliased_table_for(
-                (reflection.source_reflection || reflection).options[:join_table],
-                table_alias_for(reflection, true)
-              )
-            end
-          end
-
-          # We construct the tables in the forward order so that the aliases are generated
-          # correctly, but then reverse the array because that is the order in which we will
-          # iterate the chain.
-          @tables.reverse!
-        end
-
-        def join(table, *conditions)
-          conditions = sanitize_conditions(table, conditions)
-          table.create_join(table, table.create_on(conditions), join_type)
-        end
-
-        def sanitize_conditions(table, conditions)
-          conditions = conditions.map do |condition|
-            condition = active_record.send(:sanitize_sql, interpolate(condition), table.table_alias || table.name)
-            condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
-            condition
-          end
-
-          conditions.length == 1 ? conditions.first : Arel::Nodes::And.new(conditions)
-        end
-
         def interpolate(conditions)
           if conditions.respond_to?(:to_proc)
             instance_eval(&conditions)
diff --git a/activerecord/lib/active_record/associations/join_helper.rb b/activerecord/lib/active_record/associations/join_helper.rb
new file mode 100644
index 0000000000..6474f39503
--- /dev/null
+++ b/activerecord/lib/active_record/associations/join_helper.rb
@@ -0,0 +1,58 @@
+module ActiveRecord
+  module Associations
+    # Helper class module which gets mixed into JoinDependency::JoinAssociation and AssociationScope
+    module JoinHelper #:nodoc:
+
+      def join_type
+        Arel::InnerJoin
+      end
+
+      private
+
+      def construct_tables
+        tables = []
+        chain.each do |reflection|
+          tables << alias_tracker.aliased_table_for(
+            table_name_for(reflection),
+            table_alias_for(reflection, reflection != self.reflection)
+          )
+
+          if reflection.source_macro == :has_and_belongs_to_many
+            tables << alias_tracker.aliased_table_for(
+              (reflection.source_reflection || reflection).options[:join_table],
+              table_alias_for(reflection, true)
+            )
+          end
+        end
+        tables
+      end
+
+      def table_name_for(reflection)
+        reflection.table_name
+      end
+
+      def table_alias_for(reflection, join = false)
+        name = alias_tracker.pluralize(reflection.name)
+        name << "_#{alias_suffix}"
+        name << "_join" if join
+        name
+      end
+
+      def join(table, *conditions)
+        table.create_join(table, table.create_on(sanitize(conditions)), join_type)
+      end
+
+      def sanitize(conditions)
+        table = conditions.first.left.relation
+
+        conditions = conditions.map do |condition|
+          condition = active_record.send(:sanitize_sql, interpolate(condition), table.table_alias || table.name)
+          condition = Arel.sql(condition) unless condition.is_a?(Arel::Node)
+          condition
+        end
+
+        conditions.length == 1 ? conditions.first : Arel::Nodes::And.new(conditions)
+      end
+    end
+  end
+end