From fb2a1c4b47800d6ed65662bed26fcdae66de5869 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 30 May 2014 15:57:42 -0700 Subject: [PATCH] Return a null column when no column exists for an attribute --- activerecord/CHANGELOG.md | 4 ++++ .../lib/active_record/attribute_methods.rb | 16 +++++++++------- .../active_record/attribute_methods/dirty.rb | 3 +-- .../active_record/attribute_methods/write.rb | 12 +++++++----- activerecord/test/cases/reflection_test.rb | 19 +++++++++++++++++++ 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c471879885e..226f1dd120f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Return a null column from `column_for_attribute` when no column exists. + + *Sean Griffin* + * Preserve type when dumping PostgreSQL bit and bit varying columns. *Yves Senn* diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index b6520b9b3db..e56a4cc805d 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -350,10 +350,12 @@ module ActiveRecord # # => # # # person.column_for_attribute(:nothing) - # # => nil + # # => #, ...> 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 attr_name after it has been typecast (for example, @@ -438,16 +440,16 @@ module ActiveRecord # 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 diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 6cd4e43ddd5..4e32b78e342 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -94,8 +94,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index c3e601a208a..5203b304625 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -72,18 +72,20 @@ module ActiveRecord @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 diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index e6603f28be8..b3c02d29cbe 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -80,6 +80,25 @@ class ReflectionTest < ActiveRecord::TestCase 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