DRY up STI subclass logic

the newer method used for discriminating new records did not
use the older and more robust method used for instantiating
existing records, but did have a better post-check to ensure
the sublass was in the hierarchy. so move the descendants check
to find_sti_class, and then simply call find_sti_class from
subclass_from_attributes

now with fixed specs

activerecord/lib/active_record/inheritance.rb

@@ -55,7 +55,7 @@ module ActiveRecord
           subclass = subclass_from_attributes(attrs)
         end
 
-        if subclass
+        if subclass && subclass != self
           subclass.new(*args, &block)
         else
           super
@@ -167,17 +167,23 @@ module ActiveRecord
       end
 
       def find_sti_class(type_name)
-        if store_full_sti_class
-          ActiveSupport::Dependencies.constantize(type_name)
-        else
-          compute_type(type_name)
+        subclass = begin
+          if store_full_sti_class
+            ActiveSupport::Dependencies.constantize(type_name)
+          else
+            compute_type(type_name)
+          end
+        rescue NameError
+          raise SubclassNotFound,
+            "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " +
+            "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " +
+            "Please rename this column if you didn't intend it to be used for storing the inheritance class " +
+            "or overwrite #{name}.inheritance_column to use another column for that information."
         end
-      rescue NameError
-        raise SubclassNotFound,
-          "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " +
-          "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " +
-          "Please rename this column if you didn't intend it to be used for storing the inheritance class " +
-          "or overwrite #{name}.inheritance_column to use another column for that information."
+        unless subclass == self || descendants.include?(subclass)
+          raise SubclassNotFound, "Invalid single-table inheritance type: #{type_name} is not a subclass of #{name}"
+        end
+        subclass
       end
 
       def type_condition(table = arel_table)
@@ -198,14 +204,8 @@ module ActiveRecord
       def subclass_from_attributes(attrs)
         subclass_name = attrs.with_indifferent_access[inheritance_column]
 
-        if subclass_name.present? && subclass_name != self.name
-          subclass = subclass_name.safe_constantize
-
-          unless descendants.include?(subclass)
-            raise ActiveRecord::SubclassNotFound.new("Invalid single-table inheritance type: #{subclass_name} is not a subclass of #{name}")
-          end
-
-          subclass
+        if subclass_name.present?
+          find_sti_class(subclass_name)
         end
       end
     end

activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -1117,10 +1117,10 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
 
   def test_has_many_through_with_default_scope_on_the_target
     person = people(:michael)
-    assert_equal [posts(:thinking)], person.first_posts
+    assert_equal [posts(:thinking).id], person.first_posts.map(&:id)
 
     readers(:michael_authorless).update(first_post_id: 1)
-    assert_equal [posts(:thinking)], person.reload.first_posts
+    assert_equal [posts(:thinking).id], person.reload.first_posts.map(&:id)
   end
 
   def test_has_many_through_with_includes_in_through_association_scope

activerecord/test/cases/batches_test.rb

@@ -158,7 +158,7 @@ class EachTest < ActiveRecord::TestCase
     end
     # posts.first will be ordered using id only. Title order scope should not apply here
     assert_not_equal first_post, posts.first
-    assert_equal posts(:welcome), posts.first
+    assert_equal posts(:welcome).id, posts.first.id
   end
 
   def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then_order

activerecord/test/models/post.rb

@@ -185,6 +185,7 @@ class SubStiPost < StiPost
 end
 
 class FirstPost < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   default_scope { where(:id => 1) }
 
@@ -193,6 +194,7 @@ class FirstPost < ActiveRecord::Base
 end
 
 class PostWithDefaultInclude < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   default_scope { includes(:comments) }
   has_many :comments, :foreign_key => :post_id
@@ -204,16 +206,19 @@ class PostWithSpecialCategorization < Post
 end
 
 class PostWithDefaultScope < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   default_scope { order(:title) }
 end
 
 class SpecialPostWithDefaultScope < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   default_scope { where(:id => [1, 5,6]) }
 end
 
 class PostThatLoadsCommentsInAnAfterSaveHook < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   has_many :comments, class_name: "CommentThatAutomaticallyAltersPostBody", foreign_key: :post_id
 
@@ -223,6 +228,7 @@ class PostThatLoadsCommentsInAnAfterSaveHook < ActiveRecord::Base
 end
 
 class PostWithAfterCreateCallback < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   has_many :comments, foreign_key: :post_id
 
@@ -232,6 +238,7 @@ class PostWithAfterCreateCallback < ActiveRecord::Base
 end
 
 class PostWithCommentWithDefaultScopeReferencesAssociation < ActiveRecord::Base
+  self.inheritance_column = :disabled
   self.table_name = 'posts'
   has_many :comment_with_default_scope_references_associations, foreign_key: :post_id
   has_one :first_comment, class_name: "CommentWithDefaultScopeReferencesAssociation", foreign_key: :post_id