Allowed passing arrays-of-strings to :join everywhere. Merge duplicate join strings to avoid table aliasing problems.
Signed-off-by: Michael Koziarski <michael@koziarski.com> [#1077 state:committed]
This commit is contained in:
parent
70b8ea4fa6
commit
487758b3b8
@ -1576,19 +1576,19 @@ def merge_includes(first, second)
|
||||
(safe_to_array(first) + safe_to_array(second)).uniq
|
||||
end
|
||||
|
||||
def merge_joins(first, second)
|
||||
if first.is_a?(String) && second.is_a?(String)
|
||||
"#{first} #{second}"
|
||||
elsif first.is_a?(String) || second.is_a?(String)
|
||||
if first.is_a?(String)
|
||||
join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, second, nil)
|
||||
"#{first} #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join}"
|
||||
else
|
||||
join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, first, nil)
|
||||
"#{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} #{second}"
|
||||
def merge_joins(*joins)
|
||||
if joins.any?{|j| j.is_a?(String) || array_of_strings?(j) }
|
||||
joins = joins.collect do |join|
|
||||
join = [join] if join.is_a?(String)
|
||||
unless array_of_strings?(join)
|
||||
join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil)
|
||||
join = join_dependency.join_associations.collect { |assoc| assoc.association_join }
|
||||
end
|
||||
join
|
||||
end
|
||||
joins.flatten.uniq
|
||||
else
|
||||
(safe_to_array(first) + safe_to_array(second)).uniq
|
||||
joins.collect{|j| safe_to_array(j)}.flatten.uniq
|
||||
end
|
||||
end
|
||||
|
||||
@ -1604,6 +1604,10 @@ def safe_to_array(o)
|
||||
end
|
||||
end
|
||||
|
||||
def array_of_strings?(o)
|
||||
o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)}
|
||||
end
|
||||
|
||||
def add_order!(sql, order, scope = :auto)
|
||||
scope = scope(:find) if :auto == scope
|
||||
scoped_order = scope[:order] if scope
|
||||
@ -1652,8 +1656,12 @@ def add_joins!(sql, joins, scope = :auto)
|
||||
merged_joins = scope && scope[:joins] && joins ? merge_joins(scope[:joins], joins) : (joins || scope && scope[:joins])
|
||||
case merged_joins
|
||||
when Symbol, Hash, Array
|
||||
join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil)
|
||||
sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
|
||||
if array_of_strings?(merged_joins)
|
||||
sql << merged_joins.join(' ') + " "
|
||||
else
|
||||
join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, merged_joins, nil)
|
||||
sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
|
||||
end
|
||||
when String
|
||||
sql << " #{merged_joins} "
|
||||
end
|
||||
|
@ -935,6 +935,17 @@ def test_joins_dont_clobber_id
|
||||
assert_equal 1, first.id
|
||||
end
|
||||
|
||||
def test_joins_with_string_array
|
||||
person_with_reader_and_post = Post.find(
|
||||
:all,
|
||||
:joins => [
|
||||
"INNER JOIN categorizations ON categorizations.post_id = posts.id",
|
||||
"INNER JOIN categories ON categories.id = categorizations.category_id AND categories.type = 'SpecialCategory'"
|
||||
]
|
||||
)
|
||||
assert_equal 1, person_with_reader_and_post.size
|
||||
end
|
||||
|
||||
def test_find_by_id_with_conditions_with_or
|
||||
assert_nothing_raised do
|
||||
Post.find([1,2,3],
|
||||
|
@ -138,6 +138,36 @@ def test_scoped_find_merges_new_and_old_style_joins
|
||||
assert_equal authors(:david).attributes, scoped_authors.first.attributes
|
||||
end
|
||||
|
||||
def test_scoped_find_merges_string_array_style_and_string_style_joins
|
||||
scoped_authors = Author.with_scope(:find => { :joins => ["INNER JOIN posts ON posts.author_id = authors.id"]}) do
|
||||
Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'INNER JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1')
|
||||
end
|
||||
assert scoped_authors.include?(authors(:david))
|
||||
assert !scoped_authors.include?(authors(:mary))
|
||||
assert_equal 1, scoped_authors.size
|
||||
assert_equal authors(:david).attributes, scoped_authors.first.attributes
|
||||
end
|
||||
|
||||
def test_scoped_find_merges_string_array_style_and_hash_style_joins
|
||||
scoped_authors = Author.with_scope(:find => { :joins => :posts}) do
|
||||
Author.find(:all, :select => 'DISTINCT authors.*', :joins => ['INNER JOIN comments ON posts.id = comments.post_id'], :conditions => 'comments.id = 1')
|
||||
end
|
||||
assert scoped_authors.include?(authors(:david))
|
||||
assert !scoped_authors.include?(authors(:mary))
|
||||
assert_equal 1, scoped_authors.size
|
||||
assert_equal authors(:david).attributes, scoped_authors.first.attributes
|
||||
end
|
||||
|
||||
def test_scoped_find_merges_joins_and_eliminates_duplicate_string_joins
|
||||
scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON posts.author_id = authors.id'}) do
|
||||
Author.find(:all, :select => 'DISTINCT authors.*', :joins => ["INNER JOIN posts ON posts.author_id = authors.id", "INNER JOIN comments ON posts.id = comments.post_id"], :conditions => 'comments.id = 1')
|
||||
end
|
||||
assert scoped_authors.include?(authors(:david))
|
||||
assert !scoped_authors.include?(authors(:mary))
|
||||
assert_equal 1, scoped_authors.size
|
||||
assert_equal authors(:david).attributes, scoped_authors.first.attributes
|
||||
end
|
||||
|
||||
def test_scoped_count_include
|
||||
# with the include, will retrieve only developers for the given project
|
||||
Developer.with_scope(:find => { :include => :projects }) do
|
||||
|
@ -271,4 +271,10 @@ def test_size_should_use_length_when_results_are_loaded
|
||||
topics.size # use loaded (no query)
|
||||
end
|
||||
end
|
||||
|
||||
def test_chaining_with_duplicate_joins
|
||||
join = "INNER JOIN comments ON comments.post_id = posts.id"
|
||||
post = Post.find(1)
|
||||
assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size
|
||||
end
|
||||
end
|
||||
|
Loading…
Reference in New Issue
Block a user