Refactor join_keys to remove complex conditionals

Pushing conditionals down to through reflection

Only the through association needs the part of this conditional
that deals with belongs to and polymorphic? so that can be pushed
down into the ThroughReflection reducing the conditionals.

Remove conditional because we can delegate join keys to source reflection

Remove need for source_macro checking

By adding join_id_for to the other reflections we remove the need
to cehck against the source_macro and can reduce the conditioanl
from the original join_id_for(owner)

Using polymorphism instead of testing the source_macro

This case statement in join_association is almost exactly the same
as the original join_keys code. Testing taht theory by creating a
new join_dependency_keys(assoc_klass) method.

Refactor join_keys further to be more concise

Fixed format of "#:nodoc:" to "# :nodoc:" where I added them to this
file.

activerecord/lib/active_record/associations/join_dependency/join_association.rb

@@ -37,14 +37,9 @@ def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scop
             table = tables.shift
             klass = reflection.klass
 
-            case reflection.source_macro
-            when :belongs_to
-              key         = reflection.association_primary_key
-              foreign_key = reflection.foreign_key
-            else
-              key         = reflection.foreign_key
-              foreign_key = reflection.active_record_primary_key
-            end
+            join_keys   = reflection.join_keys(klass)
+            key         = join_keys.key
+            foreign_key = join_keys.foreign_key
 
             constraint = build_constraint(klass, table, key, foreign_table, foreign_key)
 

activerecord/lib/active_record/reflection.rb

@@ -149,19 +149,10 @@ def class_name
       JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
 
       def join_keys(assoc_klass)
-        if source_macro == :belongs_to
-          if polymorphic?
-            reflection_key = association_primary_key(assoc_klass)
-          else
-            reflection_key = association_primary_key
-          end
-          reflection_foreign_key = foreign_key
-        else
-          reflection_foreign_key = active_record_primary_key
-          reflection_key = foreign_key
-        end
-        JoinKeys.new(reflection_key, reflection_foreign_key)
+        JoinKeys.new(foreign_key, active_record_primary_key)
       end
+
+      def source_macro; macro; end
     end
     # Base class for AggregateReflection and AssociationReflection. Objects of
     # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods.
@@ -354,9 +345,8 @@ def check_preloadable!
       end
       alias :check_eager_loadable! :check_preloadable!
 
-      def join_id_for(owner) #:nodoc:
-        key = (source_macro == :belongs_to) ? foreign_key : active_record_primary_key
-        owner[key]
+      def join_id_for(owner) # :nodoc:
+        owner[active_record_primary_key]
       end
 
       def through_reflection
@@ -383,8 +373,6 @@ def scope_chain
         scope ? [[scope]] : [[]]
       end
 
-      def source_macro; macro; end
-
       def has_inverse?
         inverse_name
       end
@@ -574,7 +562,7 @@ def primary_key(klass)
         end
     end
 
-    class HasManyReflection < AssociationReflection #:nodoc:
+    class HasManyReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -584,7 +572,7 @@ def macro; :has_many; end
       def collection?; true; end
     end
 
-    class HasOneReflection < AssociationReflection #:nodoc:
+    class HasOneReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -594,7 +582,7 @@ def macro; :has_one; end
       def has_one?; true; end
     end
 
-    class BelongsToReflection < AssociationReflection #:nodoc:
+    class BelongsToReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super(name, scope, options, active_record)
       end
@@ -602,9 +590,18 @@ def initialize(name, scope, options, active_record)
       def macro; :belongs_to; end
 
       def belongs_to?; true; end
+
+      def join_keys(assoc_klass)
+        key = polymorphic? ? association_primary_key(assoc_klass) : association_primary_key
+        JoinKeys.new(key, foreign_key)
+      end
+
+      def join_id_for(owner) # :nodoc:
+        owner[foreign_key]
+      end
     end
 
-    class HasAndBelongsToManyReflection < AssociationReflection #:nodoc:
+    class HasAndBelongsToManyReflection < AssociationReflection # :nodoc:
       def initialize(name, scope, options, active_record)
         super
       end
@@ -735,6 +732,10 @@ def scope_chain
         end
       end
 
+      def join_keys(assoc_klass)
+        source_reflection.join_keys(assoc_klass)
+      end
+
       # The macro used by the source association
       def source_macro
         source_reflection.source_macro
@@ -802,6 +803,10 @@ def through_options
         through_reflection.options
       end
 
+      def join_id_for(owner) # :nodoc:
+        source_reflection.join_id_for(owner)
+      end
+
       def check_validity!
         if through_reflection.nil?
           raise HasManyThroughAssociationNotFoundError.new(active_record.name, self)