diff options
author | Jon Leighton <j@jonathanleighton.com> | 2010-10-02 18:50:17 +0100 |
---|---|---|
committer | Jon Leighton <j@jonathanleighton.com> | 2010-10-02 18:50:17 +0100 |
commit | 34ee586e993ad9e466b81f376fa92feb5d312b4c (patch) | |
tree | 5ab70bda381aeb9f068d970a517801ebd06f8e49 | |
parent | 4f69a61107d9d59f96bf249ef077483e90babe72 (diff) | |
download | rails-34ee586e993ad9e466b81f376fa92feb5d312b4c.tar.gz rails-34ee586e993ad9e466b81f376fa92feb5d312b4c.tar.bz2 rails-34ee586e993ad9e466b81f376fa92feb5d312b4c.zip |
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.
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 |