Refactor ThroughAssociation#join_to to be much smaller, and independe…

…nt of construct_owner_conditions.
rails · Mar 6, 2011 · 5dc1fb3 · 5dc1fb3
1 parent d02c326
commit 5dc1fb3
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 84 deletions.
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
@@ -28,7 +28,6 @@ def target_scope
 
         def association_scope
           scope = join_to(super)
-          scope = scope.where(reflection_conditions(0))
 
           unless options[:include]
             scope = scope.includes(source_options[:include])
@@ -57,95 +56,42 @@ def aliased_through_table
         end
 
         def construct_owner_conditions
-          super(tables.last, through_reflection_chain.last)
         end
 
         def join_to(scope)
           joins  = []
           tables = tables().dup # FIXME: Ugly
 
-          foreign_reflection = through_reflection_chain.first
+          through_reflection_chain.each_with_index do |reflection, i|
-          foreign_table      = tables.shift
+            table, foreign_table = tables.shift, tables.first
-
+
-          through_reflection_chain[1..-1].each_with_index do |reflection, i|
+            if reflection.source_macro == :has_and_belongs_to_many
-            i += 1
+              join_table = tables.shift
-            table = tables.shift
+
-
+              joins << inner_join(
-            if foreign_reflection.source_reflection.nil?
+                join_table,
-              case foreign_reflection.macro
+                table[reflection.active_record_primary_key].
-                when :belongs_to
+                  eq(join_table[reflection.association_foreign_key])
-                  joins << inner_join(
+              )
-                    table,
+
-                    foreign_table[foreign_reflection.association_primary_key],
+              table, foreign_table = join_table, tables.first
-                    table[foreign_reflection.foreign_key],
+            end
-                    reflection_conditions(i)
+
-                  )
+            if reflection.source_macro == :belongs_to
-                when :has_many, :has_one
+              key         = reflection.association_primary_key
-                  joins << inner_join(
+              foreign_key = reflection.foreign_key
-                    table,
-                    foreign_table[foreign_reflection.foreign_key],
-                    table[reflection.association_primary_key],
-                    reflection_conditions(i)
-                  )
-                when :has_and_belongs_to_many
-                  join_table = foreign_table
-
-                  joins << inner_join(
-                    table,
-                    join_table[foreign_reflection.foreign_key],
-                    table[reflection.klass.primary_key],
-                    reflection_conditions(i)
-                  )
-              end
             else
-              case foreign_reflection.source_reflection.macro
+              key         = reflection.foreign_key
-                when :belongs_to
+              foreign_key = reflection.association_primary_key
-                  joins << inner_join(
-                    table,
-                    foreign_table[foreign_reflection.association_primary_key],
-                    table[foreign_reflection.foreign_key],
-                    reflection_conditions(i)
-                  )
-                when :has_many, :has_one
-                  joins << inner_join(
-                    table,
-                    foreign_table[foreign_reflection.foreign_key],
-                    table[foreign_reflection.source_reflection.active_record_primary_key],
-                    reflection_conditions(i)
-                  )
-
-                  if reflection.macro == :has_and_belongs_to_many
-                    join_table = tables.shift
-
-                    joins << inner_join(
-                      join_table,
-                      table[reflection.klass.primary_key],
-                      join_table[reflection.association_foreign_key]
-                    )
-
-                    # hack to make it become the foreign_table
-                    table = join_table
-                  end
-                when :has_and_belongs_to_many
-                  join_table, table = table, tables.shift
-
-                  joins << inner_join(
-                    join_table,
-                    foreign_table[foreign_reflection.klass.primary_key],
-                    join_table[foreign_reflection.association_foreign_key]
-                  )
-
-                  joins << inner_join(
-                    table,
-                    join_table[foreign_reflection.foreign_key],
-                    table[reflection.klass.primary_key],
-                    reflection_conditions(i)
-                  )
-              end
             end
 
-            foreign_reflection = reflection
+            if reflection == through_reflection_chain.last
-            foreign_table      = table
+              constraint = table[key].eq owner[foreign_key]
+              scope = scope.where(constraint).where(reflection_conditions(i))
+            else
+              constraint = table[key].eq foreign_table[foreign_key]
+              joins << inner_join(foreign_table, constraint, reflection_conditions(i))
+            end
           end
 
           scope.joins(joins)
@@ -221,9 +167,7 @@ def table_alias_for(reflection, join = false)
           name
         end
 
-        def inner_join(table, left_column, right_column, *conditions)
+        def inner_join(table, *conditions)
-          conditions << left_column.eq(right_column)
-
           table.create_join(
             table,
             table.create_on(table.create_and(conditions.flatten.compact)))

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
@@ -279,6 +279,8 @@ def source_reflection
         nil
       end
 
+      alias :source_macro :macro
+
       def has_inverse?
         @options[:inverse_of]
       end
@@ -474,6 +476,10 @@ def through_conditions
         end
       end
 
+      def source_macro
+        source_reflection.source_macro
+      end
+
       # A through association is nested iff there would be more than one join table
       def nested?
         through_reflection_chain.length > 2 ||