Raise error when using write_attribute with a non-existent attribute.

Previously we would just silently write the attribute. This can lead to
subtle bugs (for example, see the change in AutosaveAssociation where a
through association would wrongly gain an attribute.

Also, ensuring that we never gain any new attributes after
initialization will allow me to reduce our dependence on method_missing.

8 files changed, 47 insertions, 25 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb
index 5621b44c8c..e9cdb130db 100644
--- a/activerecord/lib/active_record/attribute_methods/write.rb
+++ b/activerecord/lib/active_record/attribute_methods/write.rb
@@ -28,12 +28,16 @@ module ActiveRecord
       # for fixnum and float columns are turned into +nil+.
       def write_attribute(attr_name, value)
         attr_name = attr_name.to_s
-        attr_name = self.class.primary_key if attr_name == 'id'
+        attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key
         @attributes_cache.delete(attr_name)
-        if (column = column_for_attribute(attr_name)) && column.number?
+        column = column_for_attribute(attr_name)
+
+        if column && column.number?
           @attributes[attr_name] = convert_number_column_value(value)
-        else
+        elsif column || @attributes.has_key?(attr_name)
           @attributes[attr_name] = value
+        else
+          raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
         end
       end
       alias_method :raw_write_attribute, :write_attribute
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 085fdba639..056170d82a 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -370,7 +370,10 @@ module ActiveRecord
         else
           key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
           if autosave != false && (new_record? || record.new_record? || record[reflection.foreign_key] != key || autosave)
-            record[reflection.foreign_key] = key
+            unless reflection.through_reflection
+              record[reflection.foreign_key] = key
+            end
+
             saved = record.save(:validate => !autosave)
             raise ActiveRecord::Rollback if !saved && autosave
             saved
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 2dac9ea0fb..5e65e46a7d 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -314,7 +314,7 @@ module ActiveRecord
 
       new_id = self.class.unscoped.insert attributes_values
 
-      self.id ||= new_id
+      self.id ||= new_id if self.class.primary_key
 
       IdentityMap.add(self) if IdentityMap.enabled?
       @new_record = false
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index cccac6ffd7..4d5e469a7f 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -48,7 +48,9 @@ module ActiveRecord
         current_time = current_time_from_proper_timezone
 
         all_timestamp_attributes.each do |column|
-          write_attribute(column.to_s, current_time) if respond_to?(column) && self.send(column).nil?
+          if respond_to?(column) && respond_to?("#{column}=") && self.send(column).nil?
+            write_attribute(column.to_s, current_time)
+          end
         end
       end
 
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 9cd07fa8a5..adfd8e83a1 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -202,9 +202,12 @@ class PersistencesTest < ActiveRecord::TestCase
   end
 
   def test_create_columns_not_equal_attributes
-    topic = Topic.new
-    topic.title = 'Another New Topic'
-    topic.send :write_attribute, 'does_not_exist', 'test'
+    topic = Topic.allocate.init_with(
+      'attributes' => {
+        'title'          => 'Another New Topic',
+        'does_not_exist' => 'test'
+      }
+    )
     assert_nothing_raised { topic.save }
   end
 
@@ -249,9 +252,11 @@ class PersistencesTest < ActiveRecord::TestCase
     topic.title = "Still another topic"
     topic.save
 
-    topicReloaded = Topic.find(topic.id)
-    topicReloaded.title = "A New Topic"
-    topicReloaded.send :write_attribute, 'does_not_exist', 'test'
+    topicReloaded = Topic.allocate
+    topicReloaded.init_with(
+      'attributes' => topic.attributes.merge('does_not_exist' => 'test')
+    )
+    topicReloaded.title = 'A New Topic'
     assert_nothing_raised { topicReloaded.save }
   end
 
diff --git a/activerecord/test/cases/serialization_test.rb b/activerecord/test/cases/serialization_test.rb
index 382d659289..61b04b3e37 100644
--- a/activerecord/test/cases/serialization_test.rb
+++ b/activerecord/test/cases/serialization_test.rb
@@ -7,12 +7,13 @@ class SerializationTest < ActiveRecord::TestCase
 
   def setup
     @contact_attributes = {
-      :name        => 'aaron stack',
-      :age         => 25,
-      :avatar      => 'binarydata',
-      :created_at  => Time.utc(2006, 8, 1),
-      :awesome     => false,
-      :preferences => { :gem => '<strong>ruby</strong>' }
+      :name           => 'aaron stack',
+      :age            => 25,
+      :avatar         => 'binarydata',
+      :created_at     => Time.utc(2006, 8, 1),
+      :awesome        => false,
+      :preferences    => { :gem => '<strong>ruby</strong>' },
+      :alternative_id => nil
     }
   end
 
diff --git a/activerecord/test/models/contact.rb b/activerecord/test/models/contact.rb
index e081eee661..3d15c7fbed 100644
--- a/activerecord/test/models/contact.rb
+++ b/activerecord/test/models/contact.rb
@@ -11,12 +11,13 @@ class Contact < ActiveRecord::Base
     connection.merge_column('contacts', name, sql_type, options)
   end
 
-  column :name,        :string
-  column :age,         :integer
-  column :avatar,      :binary
-  column :created_at,  :datetime
-  column :awesome,     :boolean
-  column :preferences, :string
+  column :name,           :string
+  column :age,            :integer
+  column :avatar,         :binary
+  column :created_at,     :datetime
+  column :awesome,        :boolean
+  column :preferences,    :string
+  column :alternative_id, :integer
 
   serialize :preferences
 
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 64e0452100..9d5ad16a3c 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -47,6 +47,7 @@ ActiveRecord::Schema.define do
   create_table :audit_logs, :force => true do |t|
     t.column :message, :string, :null=>false
     t.column :developer_id, :integer, :null=>false
+    t.integer :unvalidated_developer_id
   end
 
   create_table :authors, :force => true do |t|
@@ -156,6 +157,7 @@ ActiveRecord::Schema.define do
     t.string  :type
     t.integer :taggings_count, :default => 0
     t.integer :children_count, :default => 0
+    t.integer :parent_id
   end
 
   create_table :companies, :force => true do |t|
@@ -461,6 +463,7 @@ ActiveRecord::Schema.define do
   create_table :pirates, :force => true do |t|
     t.column :catchphrase, :string
     t.column :parrot_id, :integer
+    t.integer :non_validated_parrot_id
     t.column :created_on, :datetime
     t.column :updated_on, :datetime
   end
@@ -529,6 +532,7 @@ ActiveRecord::Schema.define do
   create_table :ships, :force => true do |t|
     t.string :name
     t.integer :pirate_id
+    t.integer :update_only_pirate_id
     t.datetime :created_at
     t.datetime :created_on
     t.datetime :updated_at
@@ -663,7 +667,9 @@ ActiveRecord::Schema.define do
     t.string  :description
     t.integer :man_id
     t.integer :polymorphic_man_id
-    t.string :polymorphic_man_type
+    t.string  :polymorphic_man_type
+    t.integer :horrible_polymorphic_man_id
+    t.string  :horrible_polymorphic_man_type
   end
 
   create_table :interests, :force => true do |t|