From fb32fa7ed35a41c7778ba5ceb9dad662d9bc33ff Mon Sep 17 00:00:00 2001 From: David Genord II Date: Thu, 27 May 2010 14:16:48 -0400 Subject: [PATCH 1/2] Tests updated due to fixture changes --- activerecord/test/cases/associations/eager_test.rb | 16 ++++++++-------- activerecord/test/cases/batches_test.rb | 2 +- activerecord/test/cases/finder_test.rb | 6 +++--- activerecord/test/cases/method_scoping_test.rb | 4 ++-- activerecord/test/cases/named_scope_test.rb | 2 +- activerecord/test/cases/relations_test.rb | 18 +++++++++--------- activerecord/test/fixtures/comments.yml | 6 ++++++ activerecord/test/fixtures/posts.yml | 7 +++++++ 8 files changed, 37 insertions(+), 24 deletions(-) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 445e688..2ec5904 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/batches_test.rb b/activerecord/test/cases/batches_test.rb index dcc49e1..70883ad 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -24,7 +24,7 @@ class EachTest < ActiveRecord::TestCase end def test_each_should_execute_if_id_is_in_select - assert_queries(4) do + assert_queries(5) do Post.find_each(:select => "id, title, type", :batch_size => 2) do |post| assert_kind_of Post, post end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 860d330..0135614 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 e93f22b..e71a586 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/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 2007f54..c07dfa2 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -143,7 +143,7 @@ class NamedScopeTest < ActiveRecord::TestCase end def test_named_scope_with_STI - assert_equal 3,Post.containing_the_letter_a.count + assert_equal 4,Post.containing_the_letter_a.count assert_equal 1,SpecialPost.containing_the_letter_a.count end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b6815af..7de7121 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 859c2dbc1f05009bd0b769e069709b50f3ce4e52 Mon Sep 17 00:00:00 2001 From: David Genord II Date: Thu, 27 May 2010 14:19:21 -0400 Subject: [PATCH 2/2] Preserve join condition when conditions specified on has many through on eager load --- .../lib/active_record/relation/query_methods.rb | 2 +- activerecord/test/cases/associations/eager_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6782554..a89e3e4 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -128,7 +128,7 @@ module ActiveRecord join_dependency.join_associations.each do |association| if (association_relation = association.relation).is_a?(Array) to_join << [association_relation.first, association.join_class, association.association_join.first] - to_join << [association_relation.last, association.join_class, association.association_join.last] + to_join << [association_relation.last, association.join_class, association.association_join[1..-1]] else to_join << [association_relation, association.join_class, association.association_join] end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 2ec5904..0e8dbfb 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -350,6 +350,14 @@ 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_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