From 564922b32ceec259c442e965ac8a61ea5545bd48 Mon Sep 17 00:00:00 2001 From: Richard Millan Date: Thu, 5 May 2011 04:52:42 -0700 Subject: Testing identity map on inherited active record classes. Distinct models that use the same database table shouldn't be retrieved as the same object when there is not a type attribute. --- activerecord/test/cases/identity_map_test.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 649715fbb5..6cd29ed5b4 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -128,6 +128,23 @@ class IdentityMapTest < ActiveRecord::TestCase assert_same(t1, t2) end + ############################################################################## + # Tests checking if IM is functioning properly on classes with multiple # + # types of inheritance # + ############################################################################## + + def test_inherited_without_type_attribute + p1 = DestructivePirate.create!(:catchphrase => "I'm not a regular Pirate") + p2 = Pirate.find(p1.id) + assert_not_same(p1, p2) + end + + def test_inherited_with_type_attribute + c1 = comments(:sub_special_comment) + c2 = Comment.find(c1.id) + assert_same(c1, c2) + end + ############################################################################## # Tests checking dirty attribute behaviour with IM # ############################################################################## -- cgit v1.2.3 From d53b406a76db291f9d1ef15685f87a0977f16c89 Mon Sep 17 00:00:00 2001 From: Richard Millan Date: Thu, 5 May 2011 07:28:51 -0700 Subject: Test: identity on inherited classes should behave the same when turned on or off --- activerecord/test/cases/identity_map_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index 6cd29ed5b4..c183690187 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -133,6 +133,22 @@ class IdentityMapTest < ActiveRecord::TestCase # types of inheritance # ############################################################################## + def test_inherited_without_type_attribute_without_identity_map + ActiveRecord::IdentityMap.without do + p1 = DestructivePirate.create!(:catchphrase => "I'm not a regular Pirate") + p2 = Pirate.find(p1.id) + assert_not_same(p1, p2) + end + end + + def test_inherited_with_type_attribute_without_identity_map + ActiveRecord::IdentityMap.without do + c1 = comments(:sub_special_comment) + c2 = Comment.find(c1.id) + assert_same(c1.class, c2.class) + end + end + def test_inherited_without_type_attribute p1 = DestructivePirate.create!(:catchphrase => "I'm not a regular Pirate") p2 = Pirate.find(p1.id) -- cgit v1.2.3 From fc2823a85c8c5e4c0415fbb6b9e7d01f020b3036 Mon Sep 17 00:00:00 2001 From: Richard Millan Date: Fri, 6 May 2011 10:05:43 -0700 Subject: Adding base method symbolized_sti_name to activerecord base to be used on identity map. Identity map now considers the inheritance when creating the caching keys --- activerecord/lib/active_record/base.rb | 4 ++++ activerecord/lib/active_record/identity_map.rb | 10 +++++----- activerecord/test/cases/identity_map_test.rb | 10 ++++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 58a056bce9..32edae4ded 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -830,6 +830,10 @@ module ActiveRecord #:nodoc: @symbolized_base_class ||= base_class.to_s.to_sym end + def symbolized_sti_name + @symbolized_sti_name ||= sti_name ? sti_name.to_sym : symbolized_base_class + end + # Returns the base AR subclass that this class descends from. If A # extends AR::Base, A.base_class will return A. If B descends from A # through some arbitrarily deep hierarchy, B.base_class will return A. diff --git a/activerecord/lib/active_record/identity_map.rb b/activerecord/lib/active_record/identity_map.rb index 9eb47ad99f..f88ead9ca0 100644 --- a/activerecord/lib/active_record/identity_map.rb +++ b/activerecord/lib/active_record/identity_map.rb @@ -49,7 +49,7 @@ module ActiveRecord end def get(klass, primary_key) - record = repository[klass.symbolized_base_class][primary_key] + record = repository[klass.symbolized_sti_name][primary_key] if record.is_a?(klass) ActiveSupport::Notifications.instrument("identity.active_record", @@ -64,15 +64,15 @@ module ActiveRecord end def add(record) - repository[record.class.symbolized_base_class][record.id] = record + repository[record.class.symbolized_sti_name][record.id] = record end def remove(record) - repository[record.class.symbolized_base_class].delete(record.id) + repository[record.class.symbolized_sti_name].delete(record.id) end - def remove_by_id(symbolized_base_class, id) - repository[symbolized_base_class].delete(id) + def remove_by_id(symbolized_sti_name, id) + repository[symbolized_sti_name].delete(id) end def clear diff --git a/activerecord/test/cases/identity_map_test.rb b/activerecord/test/cases/identity_map_test.rb index c183690187..a0e16400d2 100644 --- a/activerecord/test/cases/identity_map_test.rb +++ b/activerecord/test/cases/identity_map_test.rb @@ -143,8 +143,9 @@ class IdentityMapTest < ActiveRecord::TestCase def test_inherited_with_type_attribute_without_identity_map ActiveRecord::IdentityMap.without do - c1 = comments(:sub_special_comment) - c2 = Comment.find(c1.id) + c = comments(:sub_special_comment) + c1 = SubSpecialComment.find(c.id) + c2 = Comment.find(c.id) assert_same(c1.class, c2.class) end end @@ -156,8 +157,9 @@ class IdentityMapTest < ActiveRecord::TestCase end def test_inherited_with_type_attribute - c1 = comments(:sub_special_comment) - c2 = Comment.find(c1.id) + c = comments(:sub_special_comment) + c1 = SubSpecialComment.find(c.id) + c2 = Comment.find(c.id) assert_same(c1, c2) end -- cgit v1.2.3