diff options
5 files changed, 107 insertions, 59 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 diff --git a/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb b/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb index 539e6e000a..938643b1b3 100644 --- a/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_has_many_through_associations_test.rb @@ -21,23 +21,23 @@ require 'models/subscription' class NestedHasManyThroughAssociationsTest < ActiveRecord::TestCase fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings -# def test_has_many_through_a_has_many_through_association_on_source_reflection -# author = authors(:david) -# assert_equal [tags(:general), tags(:general)], author.tags -# end + def test_has_many_through_a_has_many_through_association_on_source_reflection + author = authors(:david) + assert_equal [tags(:general), tags(:general)], author.tags + end def test_has_many_through_a_has_many_through_association_on_through_reflection author = authors(:david) assert_equal [subscribers(:first), subscribers(:second), subscribers(:second)], author.subscribers end -# def test_distinct_has_many_through_a_has_many_through_association_on_source_reflection -# author = authors(:david) -# assert_equal [tags(:general)], author.distinct_tags -# end + def test_distinct_has_many_through_a_has_many_through_association_on_source_reflection + author = authors(:david) + assert_equal [tags(:general)], author.distinct_tags + end -# def test_distinct_has_many_through_a_has_many_through_association_on_through_reflection -# author = authors(:david) -# assert_equal [subscribers(:first), subscribers(:second)], author.distinct_subscribers -# end + def test_distinct_has_many_through_a_has_many_through_association_on_through_reflection + author = authors(:david) + assert_equal [subscribers(:first), subscribers(:second)], author.distinct_subscribers + end end |