Merge pull request #13710 from ujjwalt/hotfix/sti

Active Record's subclass_from_attributes shouldn't assume :type is for STI unless there is a type column.

activerecord/CHANGELOG.md

+*   Don't try to get the subclass if the inheritance column doesn't exist
+
+    The `subclass_from_attrs` method is called even if the column specified by
+    the `inheritance_column` setting doesn't exist. This prevents setting associations
+    via the attributes hash if the association name clashes with the value of the setting,
+    typically `:type`. This worked previously in Rails 3.2.
+
+    *Ujjwal Thaakar*
+
 *   Enum mappings are now exposed via class methods instead of constants.
 
     Example:

activerecord/lib/active_record/inheritance.rb

@@ -18,13 +18,17 @@ def new(*args, &block)
         if abstract_class? || self == Base
           raise NotImplementedError, "#{self} is an abstract class and cannot be instantiated."
         end
-        if (attrs = args.first).is_a?(Hash)
-          if subclass = subclass_from_attrs(attrs)
-            return subclass.new(*args, &block)
-          end
+
+        attrs = args.first
+        if subclass_from_attributes?(attrs)
+          subclass = subclass_from_attributes(attrs)
+        end
+
+        if subclass
+          subclass.new(*args, &block)
+        else
+          super
         end
-        # Delegate to the original .new
-        super
       end
 
       # Returns +true+ if this does not need STI type condition. Returns
@@ -172,7 +176,11 @@ def type_condition(table = arel_table)
       # is not self or a valid subclass, raises ActiveRecord::SubclassNotFound
       # If this is a StrongParameters hash, and access to inheritance_column is not permitted,
       # this will ignore the inheritance column and return nil
-      def subclass_from_attrs(attrs)
+      def subclass_from_attributes?(attrs)
+        columns_hash.include?(inheritance_column) && attrs.is_a?(Hash)
+      end
+
+      def subclass_from_attributes(attrs)
         subclass_name = attrs.with_indifferent_access[inheritance_column]
 
         if subclass_name.present? && subclass_name != self.name

activerecord/test/cases/inheritance_test.rb

-require "cases/helper"
+require 'cases/helper'
 require 'models/company'
 require 'models/person'
 require 'models/post'
 require 'models/project'
 require 'models/subscriber'
 require 'models/vegetables'
+require 'models/shop'
 
 class InheritanceTest < ActiveRecord::TestCase
   fixtures :companies, :projects, :subscribers, :accounts, :vegetables
@@ -367,4 +368,10 @@ def test_instantiation_doesnt_try_to_require_corresponding_file
   ensure
     ActiveRecord::Base.store_full_sti_class = true
   end
+
+  def test_sti_type_from_attributes_disabled_in_non_sti_class
+    phone = Shop::Product::Type.new(name: 'Phone')
+    product = Shop::Product.new(:type => phone)
+    assert product.save
+  end
 end

activerecord/test/models/shop.rb

@@ -5,6 +5,11 @@ class Collection < ActiveRecord::Base
 
   class Product < ActiveRecord::Base
     has_many :variants, :dependent => :delete_all
+    belongs_to :type
+
+    class Type < ActiveRecord::Base
+      has_many :products
+    end
   end
 
   class Variant < ActiveRecord::Base

activerecord/test/schema/schema.rb

@@ -557,9 +557,14 @@ def create_table(*args, &block)
 
   create_table :products, force: true do |t|
     t.references :collection
+    t.references :type
     t.string     :name
   end
 
+  create_table :product_types, force: true do |t|
+    t.string :name
+  end
+
   create_table :projects, force: true do |t|
     t.string :name
     t.string :type