Integrate nested support into ThroughAssociationScope, using my concept of generating a 'chain' of reflections to be joined. It seems to work at the moment, all existing tests are passing. There may be further complications as we add more test cases for nested associations, though.

4 files changed, 95 insertions, 47 deletions

diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index ee892d373c..313d9da621 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -7,7 +7,6 @@ module ActiveRecord
   module Associations
     class HasManyThroughAssociation < HasManyAssociation #:nodoc:
       include ThroughAssociationScope
-      # include NestedHasManyThrough
 
       alias_method :new, :build
 
diff --git a/activerecord/lib/active_record/associations/nested_has_many_through.rb b/activerecord/lib/active_record/associations/nested_has_many_through.rb
index 2d03b81128..d699a60edb 100644
--- a/activerecord/lib/active_record/associations/nested_has_many_through.rb
+++ b/activerecord/lib/active_record/associations/nested_has_many_through.rb
@@ -1,3 +1,5 @@
+# TODO: Remove in the end, when its functionality is fully integrated in ThroughAssociationScope.
+
 module ActiveRecord
   module Associations
     module NestedHasManyThrough
diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb
index c433c9e66e..c5453fa79f 100644
--- a/activerecord/lib/active_record/associations/through_association_scope.rb
+++ b/activerecord/lib/active_record/associations/through_association_scope.rb
@@ -1,3 +1,5 @@
+require 'enumerator'
+
 module ActiveRecord
   # = Active Record Through Association Scope
   module Associations
@@ -19,8 +21,9 @@ module ActiveRecord
 
       # Build SQL conditions from attributes, qualified by table name.
       def construct_conditions
-        table_name = @reflection.final_through_reflection.quoted_table_name
-        conditions = construct_quoted_owner_attributes(@reflection.final_through_reflection).map do |attr, value|
+        reflection = @reflection.through_reflection_chain.last
+        table_name = reflection.quoted_table_name
+        conditions = construct_quoted_owner_attributes(reflection).map do |attr, value|
           "#{table_name}.#{attr} = #{value}"
         end
         conditions << sql_conditions if sql_conditions
@@ -51,43 +54,57 @@ module ActiveRecord
       end
       
       def construct_joins(custom_joins = nil)
-        "#{construct_through_joins(@reflection)} #{@reflection.options[:joins]} #{custom_joins}"
+        # puts @reflection.through_reflection_chain.map(&:inspect)
+        
+        "#{construct_through_joins} #{@reflection.options[:joins]} #{custom_joins}"
       end
 
-      def construct_through_joins(reflection)
-        polymorphic_join = nil
-        if reflection.source_reflection.macro == :belongs_to
-          reflection_primary_key = reflection.klass.primary_key
-          source_primary_key     = reflection.source_reflection.primary_key_name
-          if reflection.options[:source_type]
-            polymorphic_join = "AND %s.%s = %s" % [
-              reflection.through_reflection.quoted_table_name, "#{@reflection.source_reflection.options[:foreign_type]}",
-              @owner.class.quote_value(reflection.options[:source_type])
-            ]
-          end
-        else
-          reflection_primary_key = reflection.source_reflection.primary_key_name
-          source_primary_key     = reflection.through_reflection.klass.primary_key
-          if reflection.source_reflection.options[:as]
-            polymorphic_join = "AND %s.%s = %s" % [
-              reflection.quoted_table_name, "#{@reflection.source_reflection.options[:as]}_type",
-              @owner.class.quote_value(reflection.through_reflection.klass.name)
-            ]
-          end
-        end
-
-        joins = "INNER JOIN %s ON %s.%s = %s.%s %s" % [
-          reflection.through_reflection.quoted_table_name,
-          reflection.quoted_table_name, reflection_primary_key,
-          reflection.through_reflection.quoted_table_name, source_primary_key,
-          polymorphic_join
-        ]
+      def construct_through_joins
+        joins = []
         
-        # If the reflection we are going :through goes itself :through another reflection, then
-        # we must recursively get the joins to make that happen too.
-        if reflection.through_reflection.through_reflection
-          joins << " "
-          joins << construct_through_joins(reflection.through_reflection)
+        # Iterate over each pair in the through reflection chain, joining them together
+        @reflection.through_reflection_chain.each_cons(2) do |left, right|
+          polymorphic_join  = nil
+          
+          case
+            when left.options[:as]
+              left_primary_key  = left.primary_key_name
+              right_primary_key = right.klass.primary_key
+              
+              polymorphic_join = "AND %s.%s = %s" % [
+                left.quoted_table_name, "#{left.options[:as]}_type",
+                @owner.class.quote_value(right.klass.name)
+              ]
+            when left.source_reflection.macro == :belongs_to
+              left_primary_key  = left.klass.primary_key
+              right_primary_key = left.source_reflection.primary_key_name
+              
+              if left.options[:source_type]
+                polymorphic_join = "AND %s.%s = %s" % [
+                  right.quoted_table_name,
+                  left.source_reflection.options[:foreign_type].to_s,
+                  @owner.class.quote_value(left.options[:source_type])
+                ]
+              end
+            else
+              left_primary_key  = left.source_reflection.primary_key_name
+              right_primary_key = right.klass.primary_key
+              
+              if left.source_reflection.options[:as]
+                polymorphic_join = "AND %s.%s = %s" % [
+                  left.quoted_table_name,
+                  "#{left.source_reflection.options[:as]}_type",
+                  @owner.class.quote_value(right.klass.name)
+                ]
+              end
+          end
+          
+          joins << "INNER JOIN %s ON %s.%s = %s.%s %s" % [
+            right.quoted_table_name,
+            left.quoted_table_name,  left_primary_key,
+            right.quoted_table_name, right_primary_key,
+            polymorphic_join
+          ]
         end
         
         joins
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 888ddcdd5b..b7cd466e13 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -131,6 +131,14 @@ module ActiveRecord
         @sanitized_conditions ||= klass.send(:sanitize_sql, options[:conditions]) if options[:conditions]
       end
 
+      # TODO: Remove these in the final patch. I am just using them for debugging etc.
+      def inspect
+        "#<#{code_name}>"
+      end
+      def code_name
+        "#{active_record.name}.#{macro} :#{name}"
+      end
+
       private
         def derive_class_name
           name.to_s.camelize
@@ -241,6 +249,10 @@ module ActiveRecord
       def through_reflection
         false
       end
+      
+      def through_reflection_chain
+        [self]
+      end
 
       def through_reflection_primary_key_name
       end
@@ -304,6 +316,16 @@ module ActiveRecord
       def belongs_to?
         macro == :belongs_to
       end
+      
+      # TODO: Remove for final patch. Just here for debugging.
+      def inspect
+        str = "#<#{code_name}, @source_reflection="
+        str << (source_reflection.respond_to?(:code_name) ? source_reflection.code_name : source_reflection.inspect)
+        str << ", @through_reflection="
+        str << (through_reflection.respond_to?(:code_name) ? through_reflection.code_name : through_reflection.inspect)
+        str << ">"
+        str
+      end
 
       private
         def derive_class_name
@@ -353,18 +375,24 @@ module ActiveRecord
         @through_reflection ||= active_record.reflect_on_association(options[:through])
       end
       
-      # A :through reflection may have a :through reflection itself. This method returns the through
-      # reflection which is furthest away, i.e. the last in the chain, so the first which does not
-      # have its own :through reflection.
-      def final_through_reflection
-        @final_through_reflection ||= begin
-          reflection = through_reflection
-          
-          while reflection.through_reflection
-            reflection = reflection.through_reflection
+      # TODO: Documentation
+      def through_reflection_chain
+        @through_reflection_chain ||= begin
+          if source_reflection.through_reflection
+            # If the source reflection goes through another reflection, then the chain must start
+            # by getting us to the source reflection.
+            chain = source_reflection.through_reflection_chain
+          else
+            # If the source reflection does not go through another reflection, then we can get
+            # to this reflection directly, and so start the chain here
+            chain = [self]
           end
           
-          reflection
+          # Recursively build the rest of the chain
+          chain += through_reflection.through_reflection_chain
+          
+          # Finally return the completed chain
+          chain
         end
       end
 
@@ -393,6 +421,8 @@ module ActiveRecord
           raise HasManyThroughAssociationPolymorphicError.new(active_record.name, self, source_reflection)
         end
 
+        # TODO: Presumably remove the HasManyThroughSourceAssociationMacroError class and delete these lines.
+        # Think about whether there are any cases which should still be disallowed.
         # unless [:belongs_to, :has_many, :has_one].include?(source_reflection.macro) && source_reflection.options[:through].nil?
         #   raise HasManyThroughSourceAssociationMacroError.new(self)
         # end