diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 5df2e964be..cf859bbdee 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -4,7 +4,7 @@ module ActiveRecord attr_reader :association, :alias_tracker delegate :klass, :owner, :reflection, :interpolate, :to => :association - delegate :through_reflection_chain, :through_conditions, :options, :source_options, :to => :reflection + delegate :chain, :conditions, :options, :source_options, :to => :reflection def initialize(association) @association = association @@ -50,7 +50,7 @@ module ActiveRecord def add_constraints(scope) tables = construct_tables - through_reflection_chain.each_with_index do |reflection, i| + chain.each_with_index do |reflection, i| table, foreign_table = tables.shift, tables.first if reflection.source_macro == :has_and_belongs_to_many @@ -73,10 +73,10 @@ module ActiveRecord foreign_key = reflection.active_record_primary_key end - if reflection == through_reflection_chain.last + if reflection == chain.last scope = scope.where(table[key].eq(owner[foreign_key])) - through_conditions[i].each do |condition| + conditions[i].each do |condition| if options[:through] && condition.is_a?(Hash) condition = { table.name => condition } end @@ -86,7 +86,7 @@ module ActiveRecord else constraint = table[key].eq foreign_table[foreign_key] - join = inner_join(foreign_table, reflection, constraint, *through_conditions[i]) + join = inner_join(foreign_table, reflection, constraint, *conditions[i]) scope = scope.joins(join) end end @@ -96,7 +96,7 @@ module ActiveRecord def construct_tables tables = [] - through_reflection_chain.each do |reflection| + chain.each do |reflection| tables << alias_tracker.aliased_table_for( table_name_for(reflection), table_alias_for(reflection, reflection != self.reflection) diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index fc8e75b10d..f26881a6c6 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -22,7 +22,7 @@ module ActiveRecord attr_reader :tables - delegate :options, :through_reflection, :source_reflection, :through_reflection_chain, :to => :reflection + delegate :options, :through_reflection, :source_reflection, :chain, :to => :reflection delegate :table, :table_name, :to => :parent, :prefix => :parent delegate :alias_tracker, :to => :join_dependency @@ -57,14 +57,12 @@ module ActiveRecord end def join_to(relation) - # The chain starts with the target table, but we want to end with it here (makes - # more sense in this context) - chain = through_reflection_chain.reverse - foreign_table = parent_table index = 0 - chain.each do |reflection| + # The chain starts with the target table, but we want to end with it here (makes + # more sense in this context), so we reverse + chain.reverse.each do |reflection| table = tables[index] conditions = [] @@ -178,7 +176,7 @@ module ActiveRecord # later generate joins for. We must do this in advance in order to correctly allocate # the proper alias. def setup_tables - @tables = through_reflection_chain.map do |reflection| + @tables = chain.map do |reflection| table = alias_tracker.aliased_table_for( reflection.table_name, table_alias_for(reflection, reflection != self.reflection) @@ -200,7 +198,7 @@ module ActiveRecord end end - # The joins are generated from the through_reflection_chain in reverse order, so + # The joins are generated from the chain in reverse order, so # reverse the tables too (but it's important to generate the aliases in the 'forward' # order, which is why we only do the reversal now. @tables.reverse! @@ -219,7 +217,7 @@ module ActiveRecord end def reflection_conditions(index, table) - reflection.through_conditions.reverse[index].map do |condition| + reflection.conditions.reverse[index].map do |condition| process_conditions(condition, table.table_alias || table.name) end end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 408237c689..c0c92e8d72 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -3,8 +3,7 @@ module ActiveRecord module Associations module ThroughAssociation #:nodoc: - delegate :source_options, :through_options, :source_reflection, :through_reflection, - :through_reflection_chain, :through_conditions, :to => :reflection + delegate :source_reflection, :through_reflection, :chain, :to => :reflection protected @@ -18,7 +17,7 @@ module ActiveRecord # itself. def target_scope scope = super - through_reflection_chain[1..-1].each do |reflection| + chain[1..-1].each do |reflection| scope = scope.merge(reflection.klass.scoped) end scope diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 8c73e49e74..1f3ace93a9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -262,23 +262,28 @@ module ActiveRecord end def through_reflection - false - end - - def through_reflection_chain - [self] - end - - def through_conditions - conditions = [options[:conditions]].compact - conditions << { type => active_record.base_class.name } if options[:as] - [conditions] + nil end def source_reflection nil end + # A chain of reflections from this one back to the owner. For more see the explanation in + # ThroughReflection. + def chain + [self] + end + + # An array of arrays of conditions. Each item in the outside array corresponds to a reflection + # in the #chain. The inside arrays are simply conditions (and each condition may itself be + # a hash, array, arel predicate, etc...) + def conditions + conditions = [options[:conditions]].compact + conditions << { type => active_record.base_class.name } if options[:as] + [conditions] + end + alias :source_macro :macro def has_inverse? @@ -401,33 +406,34 @@ module ActiveRecord @through_reflection ||= active_record.reflect_on_association(options[:through]) end - # Returns an array of AssociationReflection objects which are involved in this through - # association. Each item in the array corresponds to a table which will be part of the - # query for this association. + # Returns an array of reflections which are involved in this association. Each item in the + # array corresponds to a table which will be part of the query for this association. # # If the source reflection is itself a ThroughReflection, then we don't include self in # the chain, but just defer to the source reflection. # - # The chain is built by recursively calling through_reflection_chain on the source - # reflection and the through reflection. The base case for the recursion is a normal - # association, which just returns [self] for its through_reflection_chain. - def through_reflection_chain - @through_reflection_chain ||= begin + # The chain is built by recursively calling #chain on the source reflection and the through + # reflection. The base case for the recursion is a normal association, which just returns + # [self] as its #chain. + def chain + @chain ||= begin if source_reflection.source_reflection # If the source reflection has its own source reflection, then the chain must start # by getting us to that source reflection. - chain = source_reflection.through_reflection_chain + chain = source_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 # # It is important to use self, rather than the source_reflection, because self # may has a :source_type option which needs to be used. + # + # FIXME: Not sure this is correct justification now that we have #conditions chain = [self] end # Recursively build the rest of the chain - chain += through_reflection.through_reflection_chain + chain += through_reflection.chain # Finally return the completed chain chain @@ -451,18 +457,18 @@ module ActiveRecord # end # # There may be conditions on Person.comment_tags, Article.comment_tags and/or Comment.tags, - # but only Comment.tags will be represented in the through_reflection_chain. So this method - # creates an array of conditions corresponding to the through_reflection_chain. Each item in - # the through_conditions array corresponds to an item in the through_reflection_chain, and is - # itself an array of conditions from an arbitrary number of relevant reflections. - def through_conditions - @through_conditions ||= begin - conditions = source_reflection.through_conditions + # but only Comment.tags will be represented in the #chain. So this method creates an array + # of conditions corresponding to the chain. Each item in the #conditions array corresponds + # to an item in the #chain, and is itself an array of conditions from an arbitrary number + # of relevant reflections, plus any :source_type or polymorphic :as constraints. + def conditions + @conditions ||= begin + conditions = source_reflection.conditions # Add to it the conditions from this reflection if necessary. conditions.first << options[:conditions] if options[:conditions] - through_conditions = through_reflection.through_conditions + through_conditions = through_reflection.conditions if options[:source_type] through_conditions.first << { foreign_type => options[:source_type] } @@ -476,14 +482,14 @@ module ActiveRecord end end + # The macro used by the source association 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 || - through_reflection.macro == :has_and_belongs_to_many + chain.length > 2 || through_reflection.macro == :has_and_belongs_to_many end # We want to use the klass from this reflection, rather than just delegate straight to diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index baaa08a359..4b881969cc 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -202,24 +202,24 @@ class ReflectionTest < ActiveRecord::TestCase assert_kind_of ThroughReflection, Subscriber.reflect_on_association(:books) end - def test_through_reflection_chain + def test_chain expected = [ Author.reflect_on_association(:essay_categories), Author.reflect_on_association(:essays), Organization.reflect_on_association(:authors) ] - actual = Organization.reflect_on_association(:author_essay_categories).through_reflection_chain + actual = Organization.reflect_on_association(:author_essay_categories).chain assert_equal expected, actual end - def test_through_conditions + def test_conditions expected = [ ["tags.name = 'Blue'"], ["taggings.comment = 'first'", {"taggable_type"=>"Post"}], ["posts.title LIKE 'misc post%'"] ] - actual = Author.reflect_on_association(:misc_post_first_blue_tags).through_conditions + actual = Author.reflect_on_association(:misc_post_first_blue_tags).conditions assert_equal expected, actual expected = [ @@ -227,7 +227,7 @@ class ReflectionTest < ActiveRecord::TestCase [{"taggable_type"=>"Post"}], [] ] - actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).through_conditions + actual = Author.reflect_on_association(:misc_post_first_blue_tags_2).conditions assert_equal expected, actual end