From f8783abf0cd409d53e7e104b576d45966252378b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 3 Apr 2005 17:50:11 +0000 Subject: Made eager loading work even more git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1083 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/lib/active_record/associations.rb | 20 ++++++++++-- .../associations/has_many_association.rb | 22 ++++++------- .../associations/has_one_association.rb | 2 +- activerecord/lib/active_record/fixtures.rb | 7 ++++- activerecord/test/associations_go_eager_test.rb | 36 ++++++++++++++++++++++ activerecord/test/associations_test.rb | 27 +--------------- activerecord/test/fixtures/author.rb | 2 +- activerecord/test/fixtures/authors.yml | 2 -- .../test/fixtures/db_definitions/sqlite.sql | 2 +- activerecord/test/fixtures/post.rb | 4 +-- activerecord/test/fixtures/posts.yml | 2 ++ 11 files changed, 76 insertions(+), 50 deletions(-) create mode 100644 activerecord/test/associations_go_eager_test.rb diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 956589fa84..071660e7cf 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -551,6 +551,14 @@ module ActiveRecord end association end + + define_method("set_#{association_name}_target") do |target| + association = association_proxy_class.new(self, + association_name, association_class_name, + association_class_primary_key_name, options) + association.target = target + instance_variable_set("@#{association_name}", association) + end end def collection_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, association_proxy_class) @@ -630,7 +638,7 @@ module ActiveRecord when :has_many record.send(reflection.name).target = extract_association_for_record(record, rows, reflection) when :has_one, :belongs_to - record.send("#{reflection.name}=", extract_association_for_record(record, rows, reflection).first) + record.send("set_#{reflection.name}_target", extract_association_for_record(record, rows, reflection).first) end end end @@ -644,8 +652,14 @@ module ActiveRecord sql << " FROM #{table_name} " reflections.each do |reflection| - sql << " LEFT JOIN #{reflection.klass.table_name} ON " + - "#{reflection.klass.table_name}.#{table_name.classify.foreign_key} = #{table_name}.#{primary_key} " + case reflection.macro + when :has_many, :has_one + sql << " LEFT JOIN #{reflection.klass.table_name} ON " + + "#{reflection.klass.table_name}.#{table_name.classify.foreign_key} = #{table_name}.#{primary_key} " + when :belongs_to + sql << " LEFT JOIN #{reflection.klass.table_name} ON " + + "#{reflection.klass.table_name}.#{reflection.klass.primary_key} = #{table_name}.#{reflection.klass.table_name.classify.foreign_key} " + end end sql << "#{options[:joins]} " if options[:joins] diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 01374bd55a..f6bceee93a 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -50,17 +50,13 @@ module ActiveRecord end def find(*args) - # Return an Array if multiple ids are given. - expects_array = args.first.kind_of?(Array) - - ids = args.flatten.compact.uniq - - # If no ids given, raise RecordNotFound. - if ids.empty? - raise RecordNotFound, "Couldn't find #{@association_class.name} without an ID" + options = Base.send(:extract_options_from_args!, args) # If using a custom finder_sql, scan the entire collection. - elsif @options[:finder_sql] + if @options[:finder_sql] + expects_array = args.first.kind_of?(Array) + ids = args.flatten.compact.uniq + if ids.size == 1 id = ids.first record = load_target.detect { |record| id == record.id } @@ -68,11 +64,11 @@ module ActiveRecord else load_target.select { |record| ids.include?(record.id) } end - - # Otherwise, delegate to association class with conditions. else - args << { :conditions => "#{@association_class_primary_key_name} = #{@owner.quoted_id} #{@conditions ? " AND " + @conditions : ""}" } - @association_class.find(*args) + original_conditions = options[:conditions] ? " AND #{options[:conditions]}" : "" + options[:conditions] = + "#{@association_class_primary_key_name} = #{@owner.quoted_id} #{@conditions ? " AND " + @conditions : ""}#{original_conditions}" + @association_class.find(args.size == 1 ? args.first : args, options) end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index e566089013..f3c3515055 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -38,7 +38,7 @@ module ActiveRecord private def find_target - @association_class.find_first(@finder_sql, @options[:order]) + @association_class.find(:first, :conditions => @finder_sql, :order => @options[:order]) end def target_obsolete? diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 21bf0249c8..77f701e4ab 100755 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -185,6 +185,9 @@ class Fixtures < Hash DEFAULT_FILTER_RE = /\.ya?ml$/ def self.instantiate_fixtures(object, table_name, fixtures, load_instances=true) + old_logger_level = ActiveRecord::Base.logger.level + ActiveRecord::Base.logger.level = Logger::ERROR + object.instance_variable_set "@#{table_name}", fixtures if load_instances fixtures.each do |name, fixture| @@ -193,12 +196,14 @@ class Fixtures < Hash end end end + + ActiveRecord::Base.logger.level = old_logger_level end def self.instantiate_all_loaded_fixtures(object, load_instances=true) all_loaded_fixtures.each do |table_name, fixtures| Fixtures.instantiate_fixtures(object, table_name, fixtures, load_instances) - end + end end cattr_accessor :all_loaded_fixtures diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb new file mode 100644 index 0000000000..59ca4cac85 --- /dev/null +++ b/activerecord/test/associations_go_eager_test.rb @@ -0,0 +1,36 @@ +require 'abstract_unit' +require 'fixtures/post' +require 'fixtures/comment' +require 'fixtures/author' + +class EagerAssociationTest < Test::Unit::TestCase + fixtures :posts, :comments, :authors + + def test_loading_with_one_association + posts = Post.find(:all, :include => :comments) + assert_equal 2, posts.first.comments.size + assert_equal @greetings.body, posts.first.comments.first.body + + post = Post.find(:first, :include => :comments, :conditions => "posts.title = 'Welcome to the weblog'") + assert_equal 2, post.comments.size + assert_equal @greetings.body, post.comments.first.body + end + + def test_loading_with_multiple_associations + posts = Post.find(:all, :include => [ :comments, :author ]) + assert_equal 2, posts.first.comments.size + assert_equal @greetings.body, posts.first.comments.first.body + end + + def test_loading_from_an_association + posts = @david.posts.find(:all, :include => :comments) + assert_equal 2, posts.first.comments.size + end + + def test_eager_association_loading_with_belongs_to + comments = Comment.find(:all, :include => :post) + assert_equal @welcome.title, comments.first.post.title + assert_equal @thinking.title, comments.last.post.title + end +end + diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index e1a05e481b..cd9fa7cb18 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -5,9 +5,6 @@ require 'fixtures/company' require 'fixtures/topic' require 'fixtures/reply' require 'fixtures/computer' -require 'fixtures/post' -require 'fixtures/comment' -require 'fixtures/author' # Can't declare new classes in test case methods, so tests before that bad_collection_keys = false @@ -206,7 +203,7 @@ end class HasManyAssociationsTest < Test::Unit::TestCase - fixtures :accounts, :companies, :developers, :projects, :developers_projects, :topics, :posts, :comments + fixtures :accounts, :companies, :developers, :projects, :developers_projects, :topics def setup @signals37 = Firm.find(1) @@ -534,28 +531,6 @@ class HasManyAssociationsTest < Test::Unit::TestCase def test_adding_array_and_collection assert_nothing_raised { Firm.find_first.clients + Firm.find_all.last.clients } end - - def test_eager_association_loading_with_one_association - posts = Post.find(:all, :include => :comments) - assert_equal 2, posts.first.comments.size - assert_equal @greetings.body, posts.first.comments.first.body - - post = Post.find(:first, :include => :comments, :conditions => "posts.title = 'Welcome to the weblog'") - assert_equal 2, post.comments.size - assert_equal @greetings.body, post.comments.first.body - end - - def test_eager_association_loading_with_multiple_associations - posts = Post.find(:all, :include => [ :comments, :author ]) - assert_equal 2, posts.first.comments.size - assert_equal @greetings.body, posts.first.comments.first.body - end - - def xtest_eager_association_loading_with_belongs_to - comments = Comment.find(:all, :include => :post) - assert_equal @welcome.title, comments.first.post.title - assert_equal @thinking.title, comments.last.post.title - end end class BelongsToAssociationsTest < Test::Unit::TestCase diff --git a/activerecord/test/fixtures/author.rb b/activerecord/test/fixtures/author.rb index 8d12190086..e159d38597 100644 --- a/activerecord/test/fixtures/author.rb +++ b/activerecord/test/fixtures/author.rb @@ -1,3 +1,3 @@ class Author < ActiveRecord::Base - belongs_to :post + has_many :posts end \ No newline at end of file diff --git a/activerecord/test/fixtures/authors.yml b/activerecord/test/fixtures/authors.yml index 718c317c37..f59b84fa45 100644 --- a/activerecord/test/fixtures/authors.yml +++ b/activerecord/test/fixtures/authors.yml @@ -1,9 +1,7 @@ david: id: 1 - post_id: 1 name: David mary: id: 2 - post_id: 2 name: Mary diff --git a/activerecord/test/fixtures/db_definitions/sqlite.sql b/activerecord/test/fixtures/db_definitions/sqlite.sql index a6f2efcbfe..db31835807 100644 --- a/activerecord/test/fixtures/db_definitions/sqlite.sql +++ b/activerecord/test/fixtures/db_definitions/sqlite.sql @@ -118,6 +118,7 @@ CREATE TABLE 'computers' ( CREATE TABLE 'posts' ( 'id' INTEGER NOT NULL PRIMARY KEY, + 'author_id' INTEGER NOT NULL, 'title' VARCHAR(255) NOT NULL, 'body' TEXT NOT NULL ); @@ -130,7 +131,6 @@ CREATE TABLE 'comments' ( CREATE TABLE 'authors' ( 'id' INTEGER NOT NULL PRIMARY KEY, - 'post_id' INTEGER NOT NULL, 'name' VARCHAR(255) NOT NULL ); diff --git a/activerecord/test/fixtures/post.rb b/activerecord/test/fixtures/post.rb index 1fd78b8343..a09e192c04 100644 --- a/activerecord/test/fixtures/post.rb +++ b/activerecord/test/fixtures/post.rb @@ -1,4 +1,4 @@ class Post < ActiveRecord::Base - has_many :comments - has_one :author + belongs_to :author + has_many :comments end \ No newline at end of file diff --git a/activerecord/test/fixtures/posts.yml b/activerecord/test/fixtures/posts.yml index 21a110ef91..7c706b6818 100644 --- a/activerecord/test/fixtures/posts.yml +++ b/activerecord/test/fixtures/posts.yml @@ -1,9 +1,11 @@ welcome: id: 1 + author_id: 1 title: Welcome to the weblog body: Such a lovely day thinking: id: 2 + author_id: 1 title: So I was thinking body: Like I hopefully always am -- cgit v1.2.3