Merge pull request #423 from richardiux/master

Identity Map caching bug

3 files changed, 44 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 6149865f80..1fe867495d 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 649715fbb5..a0e16400d2 100644
--- a/activerecord/test/cases/identity_map_test.rb
+++ b/activerecord/test/cases/identity_map_test.rb
@@ -129,6 +129,41 @@ class IdentityMapTest < ActiveRecord::TestCase
   end
 
   ##############################################################################
+  # Tests checking if IM is functioning properly on classes with multiple      #
+  # 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
+      c = comments(:sub_special_comment)
+      c1 = SubSpecialComment.find(c.id)
+      c2 = Comment.find(c.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)
+    assert_not_same(p1, p2)
+  end
+
+  def test_inherited_with_type_attribute
+    c = comments(:sub_special_comment)
+    c1 = SubSpecialComment.find(c.id)
+    c2 = Comment.find(c.id)
+    assert_same(c1, c2)
+  end
+
+  ##############################################################################
   # Tests checking dirty attribute behaviour with IM                           #
   ##############################################################################