From dfe0c658619679b3c255c9b4266893f90c4e010d Mon Sep 17 00:00:00 2001 From: Minh Tran Date: Mon, 9 Mar 2009 15:58:22 -0700 Subject: [PATCH] Fixed a belongs_to association preload error that occurs when the foreign key is the same as the association name. E.g. belongs_to :created_by, :foreign_key => 'created_by', ... The subtle error results from trying to get the key id by doing 'record.send(key)' instead of 'record[key]'. In the example above, the former approach will give the association object, not the desired id. --- .../lib/active_record/association_preload.rb | 4 ++-- activerecord/test/cases/associations/eager_test.rb | 13 +++++++++++++ activerecord/test/fixtures/funny_jokes.yml | 2 ++ activerecord/test/models/joke.rb | 2 ++ activerecord/test/schema/schema.rb | 3 ++- 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index e4ab69a..70df775 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -294,7 +294,7 @@ module ActiveRecord # Construct a mapping from klass to a list of ids to load and a mapping of those ids back to their parent_records records.each do |record| if klass = record.send(polymorph_type) - klass_id = record.send(primary_key_name) + klass_id = record[primary_key_name] if klass_id id_map = klasses_and_ids[klass] ||= {} id_list_for_klass_id = (id_map[klass_id.to_s] ||= []) @@ -306,7 +306,7 @@ module ActiveRecord else id_map = {} records.each do |record| - key = record.send(primary_key_name) + key = record[primary_key_name] if key mapped_records = (id_map[key.to_s] ||= []) mapped_records << record diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 4072381..599f26b 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -780,6 +780,19 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_preload_belongs_to_with_foreign_key_name_same_as_association_name + # query 1: grabs Joke ; query 2: preload :created_by (Person) + assert_queries(2) do + jokes = Joke.find(:all, :conditions => 'created_by IS NOT NULL', + :include => :created_by) + jokes.each do |joke| + assert !joke.created_by.nil? + # if preload worked, joke.created_by won't generate another query + assert_equal joke[:created_by], joke.created_by.id + end + end + end + def test_preload_has_many_uses_exclusive_scope people = Person.males.find :all, :include => :agents people.each do |person| diff --git a/activerecord/test/fixtures/funny_jokes.yml b/activerecord/test/fixtures/funny_jokes.yml index d47c4a6..a63491f 100644 --- a/activerecord/test/fixtures/funny_jokes.yml +++ b/activerecord/test/fixtures/funny_jokes.yml @@ -1,9 +1,11 @@ a_joke: id: 1 + created_by: 2 name: Knock knock another_joke: id: 2 + created_by: 1 name: | The \n Aristocrats Ate the candy diff --git a/activerecord/test/models/joke.rb b/activerecord/test/models/joke.rb index 3978abc..1ff923b 100644 --- a/activerecord/test/models/joke.rb +++ b/activerecord/test/models/joke.rb @@ -1,3 +1,5 @@ class Joke < ActiveRecord::Base set_table_name 'funny_jokes' + # Note: association name is the same as foreign key (:created_by) + belongs_to :created_by, :class_name => 'Person', :foreign_key => 'created_by' end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index ea848a2..05cd4f0 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -160,7 +160,8 @@ ActiveRecord::Schema.define do end create_table :funny_jokes, :force => true do |t| - t.string :name + t.string :name + t.integer :created_by end create_table :goofy_string_id, :force => true, :id => false do |t| -- 1.5.6.2