Merge pull request #15438 from sgrif/sg-null-column

Return a null column when no column exists for an attribute

Conflicts:
	activerecord/CHANGELOG.md

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Return a null column from `column_for_attribute` when no column exists.
+
+    *Sean Griffin*
+
 *   Implemented ActiveRecord::Base#pretty_print to work with PP.
 
     *Ethan*

activerecord/lib/active_record/attribute_methods.rb

@@ -350,10 +350,12 @@ def attribute_present?(attribute)
     #   # => #<ActiveRecord::ConnectionAdapters::SQLite3Column:0x007ff4ab083980 @name="name", @sql_type="varchar(255)", @null=true, ...>
     #
     #   person.column_for_attribute(:nothing)
-    #   # => nil
+    #   # => #<ActiveRecord::ConnectionAdapters::Column:0xXXX @name=nil, @sql_type=nil, @cast_type=#<Type::Value>, ...>
     def column_for_attribute(name)
-      # FIXME: should this return a null object for columns that don't exist?
-      self.class.columns_hash[name.to_s]
+      name = name.to_s
+      self.class.columns_hash.fetch(name) do
+        ConnectionAdapters::Column.new(name, nil, Type::Value.new)
+      end
     end
 
     # Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example,
@@ -438,16 +440,16 @@ def arel_attributes_with_values(attribute_names)
 
     # Filters the primary keys and readonly attributes from the attribute names.
     def attributes_for_update(attribute_names)
-      attribute_names.select do |name|
-        column_for_attribute(name) && !readonly_attribute?(name)
+      attribute_names.reject do |name|
+        readonly_attribute?(name)
       end
     end
 
     # Filters out the primary keys, from the attribute names, when the primary
     # key is to be generated (e.g. the id attribute has no value).
     def attributes_for_create(attribute_names)
-      attribute_names.select do |name|
-        column_for_attribute(name) && !(pk_attribute?(name) && id.nil?)
+      attribute_names.reject do |name|
+        pk_attribute?(name) && id.nil?
       end
     end
 

activerecord/lib/active_record/attribute_methods/dirty.rb

@@ -94,8 +94,7 @@ def keys_for_partial_write
       end
 
       def _field_changed?(attr, old, value)
-        column = column_for_attribute(attr) || Type::Value.new
-        column.changed?(old, value)
+        column_for_attribute(attr).changed?(old, value)
       end
     end
   end

activerecord/lib/active_record/attribute_methods/write.rb

@@ -72,18 +72,20 @@ def write_attribute_with_type_cast(attr_name, value, should_type_cast)
         @attributes.delete(attr_name)
         column = column_for_attribute(attr_name)
 
+        unless has_attribute?(attr_name) || self.class.columns_hash.key?(attr_name)
+          raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
+        end
+
         # If we're dealing with a binary column, write the data to the cache
         # so we don't attempt to typecast multiple times.
-        if column && column.binary?
+        if column.binary?
           @attributes[attr_name] = value
         end
 
-        if column && should_type_cast
+        if should_type_cast
           @raw_attributes[attr_name] = column.type_cast_for_write(value)
-        elsif !should_type_cast || @raw_attributes.has_key?(attr_name)
-          @raw_attributes[attr_name] = value
         else
-          raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
+          @raw_attributes[attr_name] = value
         end
       end
     end

activerecord/test/cases/reflection_test.rb

@@ -80,6 +80,25 @@ def test_integer_columns
     assert_equal :integer, @first.column_for_attribute("id").type
   end
 
+  def test_non_existent_columns_return_null_object
+    column = @first.column_for_attribute("attribute_that_doesnt_exist")
+    assert_equal "attribute_that_doesnt_exist", column.name
+    assert_equal nil, column.sql_type
+    assert_equal nil, column.type
+    assert_not column.number?
+    assert_not column.text?
+    assert_not column.binary?
+  end
+
+  def test_non_existent_columns_are_identity_types
+    column = @first.column_for_attribute("attribute_that_doesnt_exist")
+    object = Object.new
+
+    assert_equal object, column.type_cast(object)
+    assert_equal object, column.type_cast_for_write(object)
+    assert_equal object, column.type_cast_for_database(object)
+  end
+
   def test_reflection_klass_for_nested_class_name
     reflection = MacroReflection.new(:company, nil, nil, { :class_name => 'MyApplication::Business::Company' }, ActiveRecord::Base)
     assert_nothing_raised do