Permalink
Browse files

Properly support conditions on any of the reflections involved in a n…

…ested through association
  • Loading branch information...
1 parent 78b8c51 commit 9ec07348749675110843c44f680da79223218db2 @jonleighton jonleighton committed Oct 18, 2010
@@ -2164,8 +2164,10 @@ def join_to(relation)
chain = through_reflection_chain.reverse
foreign_table = parent_table
+ index = 0
- chain.zip(@tables).each do |reflection, table|
+ chain.each do |reflection|
+ table = @tables[index]
conditions = []
if reflection.source_reflection.nil?
@@ -2234,13 +2236,14 @@ def join_to(relation)
conditions << table[key].eq(foreign_table[foreign_key])
- conditions << reflection_conditions(reflection, table)
+ conditions << reflection_conditions(index, table)
conditions << sti_conditions(reflection, table)
- relation = relation.join(table, join_type).on(*conditions.compact)
+ relation = relation.join(table, join_type).on(*conditions.flatten.compact)
# The current table in this iteration becomes the foreign table in the next
foreign_table = table
+ index += 1
end
relation
@@ -2325,10 +2328,10 @@ def setup_tables
@tables
end
- def reflection_conditions(reflection, table)
- if reflection.options[:conditions]
+ def reflection_conditions(index, table)
+ @reflection.through_conditions.reverse[index].map do |condition|
Arel.sql(interpolate_sql(sanitize_sql(
- reflection.options[:conditions],
+ condition,
table.table_alias || table.name
)))
end
@@ -24,7 +24,6 @@ def construct_create_scope
end
# Build SQL conditions from attributes, qualified by table name.
- # TODO: Conditions on joins
def construct_conditions
reflection = @reflection.through_reflection_chain.last
@@ -34,11 +33,12 @@ def construct_conditions
table_alias = table_aliases[reflection]
end
- conditions = construct_quoted_owner_attributes(reflection).map do |attr, value|
+ parts = construct_quoted_owner_attributes(reflection).map do |attr, value|
"#{table_alias}.#{attr} = #{value}"
end
- conditions << sql_conditions if sql_conditions
- "(" + conditions.join(') AND (') + ")"
+ parts += reflection_conditions(0)
+
+ "(" + parts.join(') AND (') + ")"
end
# Associate attributes pointing to owner, quoted.
@@ -55,23 +55,21 @@ def construct_quoted_owner_attributes(reflection)
end
end
- def construct_from
- @reflection.table_name
- end
-
def construct_select(custom_select = nil)
distinct = "DISTINCT " if @reflection.options[:uniq]
selected = custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*"
end
def construct_joins(custom_joins = nil)
- # p @reflection.through_reflection_chain
+ # TODO: Remove this at the end
+ #p @reflection.through_reflection_chain
+ #p @reflection.through_conditions
"#{construct_through_joins} #{@reflection.options[:joins]} #{custom_joins}"
end
def construct_through_joins
- joins = []
+ joins, right_index = [], 1
# Iterate over each pair in the through reflection chain, joining them together
@reflection.through_reflection_chain.each_cons(2) do |left, right|
@@ -86,20 +84,23 @@ def construct_through_joins
joins << inner_join_sql(
right_table_and_alias,
table_aliases[left], left.klass.primary_key,
- table_aliases[right], left.primary_key_name
+ table_aliases[right], left.primary_key_name,
+ reflection_conditions(right_index)
)
when :has_many, :has_one
joins << inner_join_sql(
right_table_and_alias,
table_aliases[left], left.primary_key_name,
table_aliases[right], right.klass.primary_key,
- polymorphic_conditions(left, left)
+ polymorphic_conditions(left, left),
+ reflection_conditions(right_index)
)
when :has_and_belongs_to_many
joins << inner_join_sql(
right_table_and_alias,
table_aliases[left].first, left.primary_key_name,
- table_aliases[right], right.klass.primary_key
+ table_aliases[right], right.klass.primary_key,
+ reflection_conditions(right_index)
)
end
else
@@ -109,7 +110,8 @@ def construct_through_joins
right_table_and_alias,
table_aliases[left], left.klass.primary_key,
table_aliases[right], left.source_reflection.primary_key_name,
- source_type_conditions(left)
+ source_type_conditions(left),
+ reflection_conditions(right_index)
)
when :has_many, :has_one
if right.macro == :has_and_belongs_to_many
@@ -123,7 +125,8 @@ def construct_through_joins
right_table_and_alias,
table_aliases[left], left.source_reflection.primary_key_name,
right_table, right.klass.primary_key,
- polymorphic_conditions(left, left.source_reflection)
+ polymorphic_conditions(left, left.source_reflection),
+ reflection_conditions(right_index)
)
if right.macro == :has_and_belongs_to_many
@@ -151,10 +154,13 @@ def construct_through_joins
joins << inner_join_sql(
right_table_and_alias,
join_table, left.source_reflection.primary_key_name,
- table_aliases[right], right.klass.primary_key
+ table_aliases[right], right.klass.primary_key,
+ reflection_conditions(right_index)
)
end
end
+
+ right_index += 1
end
joins.join(" ")
@@ -206,18 +212,45 @@ def table_name_and_alias(table_name, table_alias)
"#{table_name} #{table_alias if table_alias != table_name}".strip
end
- def inner_join_sql(table, on_left_table, on_left_key, on_right_table, on_right_key, conds = nil)
- "INNER JOIN %s ON %s.%s = %s.%s %s" % [
- table,
- on_left_table, on_left_key,
- on_right_table, on_right_key,
- conds
- ]
+ def inner_join_sql(table, on_left_table, on_left_key, on_right_table, on_right_key, *conditions)
+ conditions << "#{on_left_table}.#{on_left_key} = #{on_right_table}.#{on_right_key}"
+ conditions = conditions.flatten.compact
+ conditions = conditions.map { |sql| "(#{sql})" } * ' AND '
+
+ "INNER JOIN #{table} ON #{conditions}"
+ end
+
+ def reflection_conditions(index)
+ reflection = @reflection.through_reflection_chain[index]
+ reflection_conditions = @reflection.through_conditions[index]
+
+ conditions = []
+
+ if reflection.options[:as].nil? && # reflection.klass is a Module if :as is used
+ reflection.klass.finder_needs_type_condition?
+ conditions << reflection.klass.send(:type_condition).to_sql
+ end
+
+ reflection_conditions.each do |condition|
+ sanitized_condition = reflection.klass.send(:sanitize_sql, condition)
+ interpolated_condition = interpolate_sql(sanitized_condition)
+
+ if condition.is_a?(Hash)
+ interpolated_condition.gsub!(
+ @reflection.quoted_table_name,
+ reflection.quoted_table_name
+ )
+ end
+
+ conditions << interpolated_condition
+ end
+
+ conditions
end
def polymorphic_conditions(reflection, polymorphic_reflection)
if polymorphic_reflection.options[:as]
- "AND %s.%s = %s" % [
+ "%s.%s = %s" % [
table_aliases[reflection], "#{polymorphic_reflection.options[:as]}_type",
@owner.class.quote_value(polymorphic_reflection.active_record.base_class.name)
]
@@ -226,7 +259,7 @@ def polymorphic_conditions(reflection, polymorphic_reflection)
def source_type_conditions(reflection)
if reflection.options[:source_type]
- "AND %s.%s = %s" % [
+ "%s.%s = %s" % [
table_aliases[reflection.through_reflection],
reflection.source_reflection.options[:foreign_type].to_s,
@owner.class.quote_value(reflection.options[:source_type])
@@ -245,6 +278,8 @@ def construct_owner_attributes(reflection)
end
# Construct attributes for :through pointing to owner and associate.
+ # This method is used when adding records to the association. Since this only makes sense for
+ # non-nested through associations, that's the only case we have to worry about here.
def construct_join_attributes(associate)
# TODO: revisit this to allow it for deletion, supposing dependent option is supported
raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro)
@@ -261,48 +296,6 @@ def construct_join_attributes(associate)
join_attributes
end
-
- def conditions
- @conditions = build_conditions unless defined?(@conditions)
- @conditions
- end
-
- def build_conditions
- association_conditions = @reflection.options[:conditions]
- through_conditions = build_through_conditions
- source_conditions = @reflection.source_reflection.options[:conditions]
- uses_sti = !@reflection.through_reflection.klass.descends_from_active_record?
-
- if association_conditions || through_conditions || source_conditions || uses_sti
- all = []
-
- [association_conditions, source_conditions].each do |conditions|
- all << interpolate_sql(sanitize_sql(conditions)) if conditions
- end
-
- all << through_conditions if through_conditions
- all << build_sti_condition if uses_sti
-
- all.map { |sql| "(#{sql})" } * ' AND '
- end
- end
-
- def build_through_conditions
- conditions = @reflection.through_reflection.options[:conditions]
- if conditions.is_a?(Hash)
- interpolate_sql(@reflection.through_reflection.klass.send(:sanitize_sql, conditions)).gsub(
- @reflection.quoted_table_name,
- @reflection.through_reflection.quoted_table_name)
- elsif conditions
- interpolate_sql(sanitize_sql(conditions))
- end
- end
-
- def build_sti_condition
- @reflection.through_reflection.klass.send(:type_condition).to_sql
- end
-
- alias_method :sql_conditions, :conditions
def ensure_not_nested
if @reflection.nested?
@@ -253,6 +253,10 @@ def through_reflection
def through_reflection_chain
[self]
end
+
+ def through_conditions
+ [Array.wrap(options[:conditions])]
+ end
def through_reflection_primary_key_name
end
@@ -378,9 +382,9 @@ def 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.
+ 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
else
# If the source reflection does not go through another reflection, then we can get
@@ -396,6 +400,49 @@ def through_reflection_chain
end
end
+ # Consider the following example:
+ #
+ # class Person
+ # has_many :articles
+ # has_many :comment_tags, :through => :articles
+ # end
+ #
+ # class Article
+ # has_many :comments
+ # has_many :comment_tags, :through => :comments, :source => :tags
+ # end
+ #
+ # class Comment
+ # has_many :tags
+ # 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
+ # Initialize the first item - which corresponds to this reflection - either by recursing
+ # into the souce reflection (if it is itself a through reflection), or by grabbing the
+ # source reflection conditions.
+ if source_reflection.source_reflection
+ conditions = source_reflection.through_conditions
+ else
+ conditions = [Array.wrap(source_reflection.options[:conditions])]
+ end
+
+ # Add to it the conditions from this reflection if necessary.
+ conditions.first << options[:conditions] if options[:conditions]
+
+ # Recursively fill out the rest of the array from the through reflection
+ conditions += through_reflection.through_conditions
+
+ # And return
+ conditions
+ end
+ end
+
def nested?
through_reflection_chain.length > 2
end
@@ -15,15 +15,15 @@ def test_eager_association_loading_with_cascaded_two_levels
authors = Author.find(:all, :include=>{:posts=>:comments}, :order=>"authors.id")
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
- assert_equal 2, authors[1].posts.size
+ assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i}
end
def test_eager_association_loading_with_cascaded_two_levels_and_one_level
authors = Author.find(:all, :include=>[{:posts=>:comments}, :categorizations], :order=>"authors.id")
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
- assert_equal 2, authors[1].posts.size
+ assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i}
assert_equal 1, authors[0].categorizations.size
assert_equal 2, authors[1].categorizations.size
@@ -56,7 +56,7 @@ def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_as
authors = Author.find(:all, :include=>{:posts=>[:comments, :categorizations]}, :order=>"authors.id")
assert_equal 3, authors.size
assert_equal 5, authors[0].posts.size
- assert_equal 2, authors[1].posts.size
+ assert_equal 3, authors[1].posts.size
assert_equal 10, authors[0].posts.collect{|post| post.comments.size }.inject(0){|sum,i| sum+i}
end
@@ -53,8 +53,8 @@ def test_loading_conditions_with_or
def test_with_ordering
list = Post.find(:all, :include => :comments, :order => "posts.id DESC")
- [:misc_by_mary, :misc_by_bob, :eager_other, :sti_habtm, :sti_post_and_comments,
- :sti_comments, :authorless, :thinking, :welcome
+ [:other_by_mary, :other_by_bob, :misc_by_mary, :misc_by_bob, :eager_other,
+ :sti_habtm, :sti_post_and_comments, :sti_comments, :authorless, :thinking, :welcome
].each_with_index do |post, index|
assert_equal posts(post), list[index]
end
Oops, something went wrong.

0 comments on commit 9ec0734

Please sign in to comment.