From 73c0b390b3a1ea9487c3f667352463a90af6dd71 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 4 Mar 2011 22:29:40 +0000 Subject: [PATCH] When preloading has_and_belongs_to_many associations, we should only instantiate one AR object per actual record in the database. (Even when IM is off.) --- .../preloader/has_and_belongs_to_many.rb | 6 ++++-- .../test/cases/associations/eager_test.rb | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb index e794f05340dcd..24be279449baf 100644 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb @@ -31,10 +31,12 @@ def association_key private # Once we have used the join table column (in super), we manually instantiate the - # actual records + # actual records, ensuring that we don't create more than one instances of the same + # record def associated_records_by_owner + records = {} super.each do |owner_key, rows| - rows.map! { |row| klass.instantiate(row) } + rows.map! { |row| records[row[klass.primary_key]] ||= klass.instantiate(row) } end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index ed6337b596ee8..40c82f2fb82bf 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -525,6 +525,22 @@ def test_eager_with_has_and_belongs_to_many_and_limit assert posts[1].categories.include?(categories(:general)) end + # This is only really relevant when the identity map is off. Since the preloader for habtm + # gets raw row hashes from the database and then instantiates them, this test ensures that + # it only instantiates one actual object per record from the database. + def test_has_and_belongs_to_many_should_not_instantiate_same_records_multiple_times + welcome = posts(:welcome) + categories = Category.includes(:posts) + + general = categories.find { |c| c == categories(:general) } + technology = categories.find { |c| c == categories(:technology) } + + post1 = general.posts.to_a.find { |p| p == posts(:welcome) } + post2 = technology.posts.to_a.find { |p| p == posts(:welcome) } + + assert_equal post1.object_id, post2.object_id + end + def test_eager_with_has_many_and_limit_and_conditions_on_the_eagers posts = authors(:david).posts.find(:all, :include => :comments,