From b029ea61fc5f398e3b7694bf1ea3b6391442102e Mon Sep 17 00:00:00 2001 From: David Genord II Date: Wed, 17 Feb 2010 11:11:02 -0500 Subject: [PATCH 1/2] Test updates due to fixtures updates --- activerecord/test/cases/associations/eager_test.rb | 16 ++++++++-------- activerecord/test/cases/finder_test.rb | 6 +++--- activerecord/test/cases/method_scoping_test.rb | 4 ++-- activerecord/test/cases/relations_test.rb | 18 +++++++++--------- activerecord/test/fixtures/comments.yml | 6 ++++++ activerecord/test/fixtures/posts.yml | 7 +++++++ 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index ffa6d45..c81d842 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -53,7 +53,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_with_ordering list = Post.find(:all, :include => :comments, :order => "posts.id DESC") - [:eager_other, :sti_habtm, :sti_post_and_comments, :sti_comments, + [:alt_post, :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] @@ -174,7 +174,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_association_loading_with_belongs_to comments = Comment.find(:all, :include => :post) - assert_equal 10, comments.length + assert_equal 11, comments.length titles = comments.map { |c| c.post.title } assert titles.include?(posts(:welcome).title) assert titles.include?(posts(:sti_post_and_comments).title) @@ -720,21 +720,21 @@ class EagerAssociationTest < ActiveRecord::TestCase posts = assert_queries(2) do Post.find(:all, :joins => :comments, :include => :author, :order => 'comments.id DESC') end - assert_equal posts(:eager_other), posts[0] - assert_equal authors(:mary), assert_no_queries { posts[0].author} + assert_equal posts(:eager_other), posts[1] + assert_equal authors(:mary), assert_no_queries { posts[1].author} end def test_eager_loading_with_conditions_on_joined_table_preloads posts = assert_queries(2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => [:comments], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end - assert_equal [posts(:welcome)], posts + assert_equal posts(:welcome, :alt_post), posts assert_equal authors(:david), assert_no_queries { posts[0].author} posts = assert_queries(2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => [:comments], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end - assert_equal [posts(:welcome)], posts + assert_equal posts(:welcome, :alt_post), posts assert_equal authors(:david), assert_no_queries { posts[0].author} posts = assert_queries(2) do @@ -753,13 +753,13 @@ class EagerAssociationTest < ActiveRecord::TestCase posts = assert_queries(2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => "INNER JOIN comments on comments.post_id = posts.id", :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end - assert_equal [posts(:welcome)], posts + assert_equal posts(:welcome, :alt_post), posts assert_equal authors(:david), assert_no_queries { posts[0].author} posts = assert_queries(2) do Post.find(:all, :select => 'distinct posts.*', :include => :author, :joins => ["INNER JOIN comments on comments.post_id = posts.id"], :conditions => "comments.body like 'Thank you%'", :order => 'posts.id') end - assert_equal [posts(:welcome)], posts + assert_equal posts(:welcome, :alt_post), posts assert_equal authors(:david), assert_no_queries { posts[0].author} end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index d2451f2..3704812 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -177,9 +177,9 @@ class FinderTest < ActiveRecord::TestCase second_three_posts = Post.find :all, :order => ' author_id,id ', :limit => 3, :offset => 3 last_posts = Post.find :all, :order => ' author_id, id ', :limit => 3, :offset => 6 - assert_equal [[0,3],[1,1],[1,2]], first_three_posts.map { |p| [p.author_id, p.id] } - assert_equal [[1,4],[1,5],[1,6]], second_three_posts.map { |p| [p.author_id, p.id] } - assert_equal [[2,7]], last_posts.map { |p| [p.author_id, p.id] } + assert_equal [[0,3],[0,8],[1,1]], first_three_posts.map { |p| [p.author_id, p.id] } + assert_equal [[1,2],[1,4],[1,5]], second_three_posts.map { |p| [p.author_id, p.id] } + assert_equal [[1,6],[2,7]], last_posts.map { |p| [p.author_id, p.id] } end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 1081aa4..c0296e9 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -546,12 +546,12 @@ class HasManyScopingTest< ActiveRecord::TestCase end def test_forwarding_to_scoped - assert_equal 4, Comment.search_by_type('Comment').size + assert_equal 5, Comment.search_by_type('Comment').size assert_equal 2, @welcome.comments.search_by_type('Comment').size end def test_forwarding_to_dynamic_finders - assert_equal 4, Comment.find_all_by_type('Comment').size + assert_equal 5, Comment.find_all_by_type('Comment').size assert_equal 2, @welcome.comments.find_all_by_type('Comment').size end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 1e34539..df1b93e 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -433,22 +433,22 @@ class RelationTest < ActiveRecord::TestCase def test_count posts = Post.scoped - assert_equal 7, posts.count - assert_equal 7, posts.count(:all) - assert_equal 7, posts.count(:id) + assert_equal 8, posts.count + assert_equal 8, posts.count(:all) + assert_equal 8, posts.count(:id) assert_equal 1, posts.where('comments_count > 1').count - assert_equal 5, posts.where(:comments_count => 0).count + assert_equal 6, posts.where(:comments_count => 0).count end def test_count_with_distinct posts = Post.scoped assert_equal 3, posts.count(:comments_count, :distinct => true) - assert_equal 7, posts.count(:comments_count, :distinct => false) + assert_equal 8, posts.count(:comments_count, :distinct => false) assert_equal 3, posts.select(:comments_count).count(:distinct => true) - assert_equal 7, posts.select(:comments_count).count(:distinct => false) + assert_equal 8, posts.select(:comments_count).count(:distinct => false) end def test_count_explicit_columns @@ -458,7 +458,7 @@ class RelationTest < ActiveRecord::TestCase assert_equal [0], posts.select('comments_count').where('id is not null').group('id').order('id').count.values.uniq assert_equal 0, posts.where('id is not null').select('comments_count').count - assert_equal 7, posts.select('comments_count').count('id') + assert_equal 8, posts.select('comments_count').count('id') assert_equal 0, posts.select('comments_count').count assert_equal 0, posts.count(:comments_count) assert_equal 0, posts.count('comments_count') @@ -467,12 +467,12 @@ class RelationTest < ActiveRecord::TestCase def test_size posts = Post.scoped - assert_queries(1) { assert_equal 7, posts.size } + assert_queries(1) { assert_equal 8, posts.size } assert ! posts.loaded? best_posts = posts.where(:comments_count => 0) best_posts.to_a # force load - assert_no_queries { assert_equal 5, best_posts.size } + assert_no_queries { assert_equal 6, best_posts.size } end def test_count_complex_chained_relations diff --git a/activerecord/test/fixtures/comments.yml b/activerecord/test/fixtures/comments.yml index 236bdb2..bc4a9b7 100644 --- a/activerecord/test/fixtures/comments.yml +++ b/activerecord/test/fixtures/comments.yml @@ -57,3 +57,9 @@ eager_other_comment1: post_id: 7 body: go crazy type: SpecialComment + +alt_thank_you: + id: 12 + post_id: 8 + body: Thank you for the post + type: Comment diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index f817493..d061471 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -50,3 +50,10 @@ eager_other: title: eager loading with OR'd conditions body: hello type: Post + +alt_post: + id: 8 + author_id: 0 + title: No author + body: I just appeared one day + type: Post -- 1.6.2.3 From ae1e70c2d73dec97dec074bb17c47e5507fbaa36 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Wed, 17 Feb 2010 11:14:40 -0500 Subject: [PATCH 2/2] Corrected join conditions for habtm and has many through --- activerecord/lib/active_record/associations.rb | 2 +- .../lib/active_record/relation/query_methods.rb | 2 +- activerecord/test/cases/associations/eager_test.rb | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 57785b4..5ff252f 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2046,7 +2046,7 @@ module ActiveRecord def join_relation(joining_relation, join = nil) if (relations = relation).is_a?(Array) joining_relation.joins(Relation::JoinOperation.new(relations.first, Arel::OuterJoin, association_join.first)). - joins(Relation::JoinOperation.new(relations.last, Arel::OuterJoin, association_join.last)) + joins(Relation::JoinOperation.new(relations.last, Arel::OuterJoin, association_join[1..-1])) else joining_relation.joins(Relation::JoinOperation.new(relations, Arel::OuterJoin, association_join)) end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 0266700..37bc4ef 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -85,7 +85,7 @@ module ActiveRecord join_dependency.join_associations.each do |association| if (association_relation = association.relation).is_a?(Array) to_join << [association_relation.first, association.association_join.first] - to_join << [association_relation.last, association.association_join.last] + to_join << [association_relation.last, association.association_join[1..-1]] else to_join << [association_relation, association.association_join] end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index c81d842..c280ff2 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -350,6 +350,20 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal comments(:more_greetings), Author.find(authors(:david).id, :include => :comments_with_order_and_conditions).comments_with_order_and_conditions.first end + def test_eager_join_with_has_many_through_join_model_with_conditions_on_top_level + id = authors(:david).id + author = assert_queries(2) { Author.eager_load(:comments_with_order_and_conditions).find(id) } + comments = assert_no_queries { author.comments_with_order_and_conditions.to_ary } + assert_equal 2, assert_no_queries { comments.size } + assert_equal comments(:more_greetings), assert_no_queries { comments.sort_by(&:body)[0] } + end + + def test_eager_join_with_has_many_through_join_model_with_conditions_on_top_level + id = authors(:david).id + author = Author.find(:first, :joins => :comments_with_order_and_conditions, :conditions => {'id' => id, 'comments.post_id' => 8}) + assert_equal nil, author + end + def test_eager_with_has_many_through_join_model_with_include author_comments = Author.find(authors(:david).id, :include => :comments_with_include).comments_with_include.to_a assert_no_queries do -- 1.6.2.3