Permalink
Browse files

Add support for nested through associations in JoinAssociation. Hence…

… Foo.joins(:bar) will work for through associations. There is some duplicated code now, which will be refactored.
  • Loading branch information...
1 parent c954d54 commit ab5a9335020eff0da35b62b86a62ed8587a4d598 @jonleighton jonleighton committed Oct 9, 2010
@@ -2129,7 +2129,7 @@ class JoinAssociation < JoinPart # :nodoc:
# These implement abstract methods from the superclass
attr_reader :aliased_prefix, :aliased_table_name
- delegate :options, :through_reflection, :source_reflection, :to => :reflection
+ delegate :options, :through_reflection, :source_reflection, :through_reflection_chain, :to => :reflection
delegate :table, :table_name, :to => :parent, :prefix => true
def initialize(reflection, join_dependency, parent = nil)
@@ -2185,13 +2185,13 @@ def table
protected
- def aliased_table_name_for(name, suffix = nil)
+ def aliased_table_name_for(name, aliased_name, suffix = nil)
if @join_dependency.table_aliases[name].zero?
@join_dependency.table_aliases[name] = @join_dependency.count_aliases_from_table_joins(name)
end
if !@join_dependency.table_aliases[name].zero? # We need an alias
- name = active_record.connection.table_alias_for "#{pluralize(reflection.name)}_#{parent_table_name}#{suffix}"
+ name = active_record.connection.table_alias_for "#{aliased_name}_#{parent_table_name}#{suffix}"
@join_dependency.table_aliases[name] += 1
if @join_dependency.table_aliases[name] == 1 # First time we've seen this name
# Also need to count the aliases from the table_aliases to avoid incorrect count
@@ -2226,12 +2226,26 @@ def interpolate_sql(sql)
def allocate_aliases
@aliased_prefix = "t#{ join_dependency.join_parts.size }"
- @aliased_table_name = aliased_table_name_for(table_name)
+ @aliased_table_name = aliased_table_name_for(table_name, pluralize(reflection.name))
- if reflection.macro == :has_and_belongs_to_many
- @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join")
- elsif [:has_many, :has_one].include?(reflection.macro) && reflection.options[:through]
- @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join")
+ case reflection.macro
+ when :has_and_belongs_to_many
+ @aliased_join_table_name = aliased_table_name_for(
+ reflection.options[:join_table],
+ pluralize(reflection.name), "_join"
+ )
+ when :has_many, :has_one
+ # Add the target table name which was already generated. We don't want to generate
+ # it again as that would lead to an unnecessary alias.
+ @aliased_through_table_names = [@aliased_table_name]
+
+ # Generate the rest in the original order
+ @aliased_through_table_names += through_reflection_chain[1..-1].map do |reflection|
+ aliased_table_name_for(reflection.table_name, pluralize(reflection.name), "_join")
+ end
+
+ # Now reverse the list, as we will use it in that order
+ @aliased_through_table_names.reverse!
end
end
@@ -2284,99 +2298,81 @@ def join_has_and_belongs_to_many_to(relation)
eq(join_table[klass_fk])
)
end
-
- def join_has_many_to(relation)
- if reflection.options[:through]
- join_has_many_through_to(relation)
- elsif reflection.options[:as]
- join_has_many_polymorphic_to(relation)
- else
- foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key
- primary_key = options[:primary_key] || parent.primary_key
-
- join_target_table(
- relation,
- target_table[foreign_key].
- eq(parent_table[primary_key])
- )
- end
- end
- alias :join_has_one_to :join_has_many_to
- def join_has_many_through_to(relation)
- join_table = Arel::Table.new(
- through_reflection.klass.table_name, :engine => arel_engine,
- :as => @aliased_join_table_name
- )
+ def join_has_many_to(relation)
+ # Chain usually starts with target, but we want to end with it here (just makes it
+ # easier to understand the joins that are generated)
+ chain = through_reflection_chain.reverse
- jt_conditions = []
- jt_foreign_key = first_key = second_key = nil
-
- if through_reflection.options[:as] # has_many :through against a polymorphic join
- as_key = through_reflection.options[:as].to_s
- jt_foreign_key = as_key + '_id'
+ foreign_table = parent_table
+
+ chain.zip(@aliased_through_table_names).each do |reflection, aliased_table_name|
+ table = Arel::Table.new(
+ reflection.table_name, :engine => arel_engine,
+ :as => aliased_table_name, :columns => reflection.klass.columns
+ )
- jt_conditions <<
- join_table[as_key + '_type'].
- eq(parent.active_record.base_class.name)
- else
- jt_foreign_key = through_reflection.primary_key_name
- end
-
- case source_reflection.macro
- when :has_many
- second_key = options[:foreign_key] || primary_key
-
- if source_reflection.options[:as]
- first_key = "#{source_reflection.options[:as]}_id"
+ conditions = []
+
+ if reflection.source_reflection.nil?
+ case reflection.macro
+ when :belongs_to
+ key = reflection.options[:primary_key] ||
+ reflection.klass.primary_key
+ foreign_key = reflection.primary_key_name
+ when :has_many, :has_one
+ key = reflection.primary_key_name
+ foreign_key = reflection.options[:primary_key] ||
+ reflection.active_record.primary_key
+
+ if reflection.options[:as]
+ conditions <<
+ table["#{reflection.options[:as]}_type"].
+ eq(reflection.active_record.base_class.name)
+ end
+ end
+ elsif reflection.source_reflection.macro == :belongs_to
+ key = reflection.klass.primary_key
+ foreign_key = reflection.source_reflection.primary_key_name
+
+ if reflection.options[:source_type]
+ conditions <<
+ foreign_table[reflection.source_reflection.options[:foreign_type]].
+ eq(reflection.options[:source_type])
+ end
else
- first_key = through_reflection.klass.base_class.to_s.foreign_key
+ key = reflection.source_reflection.primary_key_name
+ foreign_key = reflection.source_reflection.klass.primary_key
end
-
- unless through_reflection.klass.descends_from_active_record?
- jt_conditions <<
- join_table[through_reflection.active_record.inheritance_column].
- eq(through_reflection.klass.sti_name)
+
+ conditions << table[key].eq(foreign_table[foreign_key])
+
+ if reflection.options[:conditions]
+ conditions << process_conditions(reflection.options[:conditions], aliased_table_name)
end
- when :belongs_to
- first_key = primary_key
- if reflection.options[:source_type]
- second_key = source_reflection.association_foreign_key
+ # If the target table is an STI model then we must be sure to only include records of
+ # its type and its sub-types.
+ unless reflection.klass.descends_from_active_record?
+ sti_column = table[reflection.klass.inheritance_column]
- jt_conditions <<
- join_table[reflection.source_reflection.options[:foreign_type]].
- eq(reflection.options[:source_type])
- else
- second_key = source_reflection.primary_key_name
+ sti_condition = sti_column.eq(reflection.klass.sti_name)
+ reflection.klass.descendants.each do |subclass|
+ sti_condition = sti_condition.or(sti_column.eq(subclass.sti_name))
+ end
+
+ conditions << sti_condition
end
+
+ relation = relation.join(table, join_type).on(*conditions)
+
+ # The current table in this iteration becomes the foreign table in the next
+ foreign_table = table
end
- jt_conditions <<
- parent_table[parent.primary_key].
- eq(join_table[jt_foreign_key])
-
- if through_reflection.options[:conditions]
- jt_conditions << process_conditions(through_reflection.options[:conditions], aliased_table_name)
- end
-
- relation = relation.join(join_table, join_type).on(*jt_conditions)
-
- join_target_table(
- relation,
- target_table[first_key].eq(join_table[second_key])
- )
- end
-
- def join_has_many_polymorphic_to(relation)
- join_target_table(
- relation,
- target_table["#{reflection.options[:as]}_id"].
- eq(parent_table[parent.primary_key]),
- target_table["#{reflection.options[:as]}_type"].
- eq(parent.active_record.base_class.name)
- )
+ relation
end
+ alias :join_has_one_to :join_has_many_to
def join_belongs_to_to(relation)
foreign_key = options[:foreign_key] || reflection.primary_key_name
@@ -73,6 +73,8 @@ def construct_through_joins
if left.options[:as]
polymorphic_join = "AND %s.%s = %s" % [
table_aliases[left], "#{left.options[:as]}_type",
+ # TODO: Why right.klass.name? Rather than left.active_record.name?
+ # TODO: Also should maybe use the base_class (see related code in JoinAssociation)
@owner.class.quote_value(right.klass.name)
]
end
@@ -117,6 +119,8 @@ def construct_through_joins
joins.join(" ")
end
+ # TODO: Use the same aliasing strategy (and code?) as JoinAssociation (as this is the
+ # documented behaviour)
def table_aliases
@table_aliases ||= begin
tally = {}
@@ -17,18 +17,37 @@
require 'models/subscriber'
require 'models/book'
require 'models/subscription'
+require 'models/rating'
+
+# NOTE: Some of these tests might not really test "nested" HMT associations, as opposed to ones which
+# are just one level deep. But it's all the same thing really, as the "nested" code is being
+# written in a generic way which applies to "non-nested" HMT associations too. So let's just shove
+# all useful tests in here for now and then work out where they ought to live properly later.
class NestedHasManyThroughAssociationsTest < ActiveRecord::TestCase
- fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings
+ fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings,
+ :people, :readers, :references, :jobs, :ratings, :comments
def test_has_many_through_a_has_many_through_association_on_source_reflection
author = authors(:david)
assert_equal [tags(:general), tags(:general)], author.tags
+
+ # Only David has a Post tagged with General
+ authors = Author.joins(:tags).where('tags.id' => tags(:general).id)
+ assert_equal [authors(:david)], authors.uniq
+
+ # This ensures that the polymorphism of taggings is being observed correctly
+ authors = Author.joins(:tags).where('taggings.taggable_type' => 'FakeModel')
+ assert authors.empty?
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
+
+ # All authors with subscribers where one of the subscribers' nick is 'alterself'
+ authors = Author.joins(:subscribers).where('subscribers.nick' => 'alterself')
+ assert_equal [authors(:david)], authors
end
def test_distinct_has_many_through_a_has_many_through_association_on_source_reflection
@@ -44,11 +63,41 @@ def test_distinct_has_many_through_a_has_many_through_association_on_through_ref
def test_nested_has_many_through_with_a_table_referenced_multiple_times
author = authors(:bob)
assert_equal [posts(:misc_by_bob), posts(:misc_by_mary)], author.similar_posts.sort_by(&:id)
+
+ # Mary and Bob both have posts in misc, but they are the only ones.
+ authors = Author.joins(:similar_posts).where('posts.id' => posts(:misc_by_bob).id)
+ assert_equal [authors(:mary), authors(:bob)], authors.uniq.sort_by(&:id)
+
+ # Check the polymorphism of taggings is being observed correctly (in both joins)
+ authors = Author.joins(:similar_posts).where('taggings.taggable_type' => 'FakeModel')
+ assert authors.empty?
+ authors = Author.joins(:similar_posts).where('taggings_authors_join.taggable_type' => 'FakeModel')
+ assert authors.empty?
end
- def test_nested_has_many_through_as_a_join
- # All authors with subscribers where one of the subscribers' nick is 'alterself'
- authors = Author.joins(:subscribers).where('subscribers.nick' => 'alterself')
- assert_equal [authors(:david)], authors
+ def test_has_many_through_with_foreign_key_option_on_through_reflection
+ assert_equal [posts(:welcome), posts(:authorless)], people(:david).agents_posts
+ assert_equal [authors(:david)], references(:david_unicyclist).agents_posts_authors
+
+ references = Reference.joins(:agents_posts_authors).where('authors.id' => authors(:david).id)
+ assert_equal [references(:david_unicyclist)], references
+ end
+
+ def test_has_many_through_with_foreign_key_option_on_source_reflection
+ assert_equal [people(:michael), people(:susan)], jobs(:unicyclist).agents
+
+ jobs = Job.joins(:agents)
+ assert_equal [jobs(:unicyclist), jobs(:unicyclist)], jobs
+ end
+
+ def test_has_many_through_with_sti_on_through_reflection
+ ratings = posts(:sti_comments).special_comments_ratings.sort_by(&:id)
+ assert_equal [ratings(:special_comment_rating), ratings(:sub_special_comment_rating)], ratings
+
+ # Ensure STI is respected in the join
+ scope = Post.joins(:special_comments_ratings).where(:id => posts(:sti_comments).id)
+ assert scope.where("comments.type" => "Comment").empty?
+ assert !scope.where("comments.type" => "SpecialComment").empty?
+ assert !scope.where("comments.type" => "SubSpecialComment").empty?
end
end
@@ -0,0 +1,14 @@
+normal_comment_rating:
+ id: 1
+ comment_id: 8
+ value: 1
+
+special_comment_rating:
+ id: 2
+ comment_id: 6
+ value: 1
+
+sub_special_comment_rating:
+ id: 3
+ comment_id: 12
+ value: 1
@@ -7,6 +7,7 @@ class Comment < ActiveRecord::Base
:conditions => { "posts.author_id" => 1 }
belongs_to :post, :counter_cache => true
+ has_many :ratings
def self.what_are_you
'a comment...'
@@ -2,4 +2,6 @@ class Job < ActiveRecord::Base
has_many :references
has_many :people, :through => :references
belongs_to :ideal_reference, :class_name => 'Reference'
+
+ has_many :agents, :through => :people
end
@@ -13,6 +13,9 @@ class Person < ActiveRecord::Base
belongs_to :primary_contact, :class_name => 'Person'
has_many :agents, :class_name => 'Person', :foreign_key => 'primary_contact_id'
belongs_to :number1_fan, :class_name => 'Person'
+
+ has_many :agents_posts, :through => :agents, :source => :posts
+ has_many :agents_posts_authors, :through => :agents_posts, :source => :author
scope :males, :conditions => { :gender => 'M' }
scope :females, :conditions => { :gender => 'F' }
@@ -46,6 +46,8 @@ def find_most_recent
has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
has_many :special_comments
has_many :nonexistant_comments, :class_name => 'Comment', :conditions => 'comments.id < 0'
+
+ has_many :special_comments_ratings, :through => :special_comments, :source => :ratings
has_and_belongs_to_many :categories
has_and_belongs_to_many :special_categories, :join_table => "categories_posts", :association_foreign_key => 'category_id'
@@ -0,0 +1,3 @@
+class Rating < ActiveRecord::Base
+ belongs_to :comment
+end
@@ -1,6 +1,8 @@
class Reference < ActiveRecord::Base
belongs_to :person
belongs_to :job
+
+ has_many :agents_posts_authors, :through => :person
end
class BadReference < ActiveRecord::Base
@@ -449,6 +449,11 @@ def create_table(*args, &block)
t.string :type
end
+ create_table :ratings, :force => true do |t|
+ t.integer :comment_id
+ t.integer :value
+ end
+
create_table :readers, :force => true do |t|
t.integer :post_id, :null => false
t.integer :person_id, :null => false

0 comments on commit ab5a933

Please sign in to comment.