aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSean Griffin <sean@thoughtbot.com>2014-06-26 09:36:40 -0600
committerSean Griffin <sean@thoughtbot.com>2014-06-26 09:43:35 -0600
commit3bc314e65830dabd7b1c47ad1fa27be5ace0699f (patch)
treea34a12dd12aca74f837f48bdfd125830e1766744
parent031588ebe6b449bcba6e3becfaff6978ba10464a (diff)
downloadrails-3bc314e65830dabd7b1c47ad1fa27be5ace0699f.tar.gz
rails-3bc314e65830dabd7b1c47ad1fa27be5ace0699f.tar.bz2
rails-3bc314e65830dabd7b1c47ad1fa27be5ace0699f.zip
Move writing unknown column exception to null attribute
Making this change revealed several subtle bugs related to models with no primary key, and anonymous classes. These have been fixed as well, with regression tests added.
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/attribute.rb5
-rw-r--r--activerecord/lib/active_record/attribute_methods/primary_key.rb7
-rw-r--r--activerecord/lib/active_record/attribute_methods/write.rb4
-rw-r--r--activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb2
-rw-r--r--activerecord/lib/active_record/core.rb2
-rw-r--r--activerecord/test/cases/attribute_methods_test.rb9
-rw-r--r--activerecord/test/cases/primary_keys_test.rb12
-rw-r--r--activerecord/test/models/contact.rb1
9 files changed, 34 insertions, 13 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c3ded8d4a9..4b39791b73 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+* Fix subtle bugs regarding attribute assignment on models with no primary
+ key. `'id'` will no longer be part of the attributes hash.
+
+ *Sean Griffin*
+
* Deprecate automatic counter caches on `has_many :through`. The behavior was
broken and inconsistent.
diff --git a/activerecord/lib/active_record/attribute.rb b/activerecord/lib/active_record/attribute.rb
index 33c20bb5cc..6d38224830 100644
--- a/activerecord/lib/active_record/attribute.rb
+++ b/activerecord/lib/active_record/attribute.rb
@@ -90,6 +90,11 @@ module ActiveRecord
def value
nil
end
+
+ def with_value_from_database(value)
+ raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{name}`"
+ end
+ alias_method :with_value_from_user, :with_value_from_database
end
class Uninitialized < Attribute # :nodoc:
diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb
index 1c81a5b71b..cadad60ddd 100644
--- a/activerecord/lib/active_record/attribute_methods/primary_key.rb
+++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb
@@ -83,12 +83,9 @@ module ActiveRecord
end
def get_primary_key(base_name) #:nodoc:
- return 'id' if base_name.blank?
-
- case primary_key_prefix_type
- when :table_name
+ if base_name && primary_key_prefix_type == :table_name
base_name.foreign_key(false)
- when :table_name_with_underscore
+ elsif base_name && primary_key_prefix_type == :table_name_with_underscore
base_name.foreign_key
else
if ActiveRecord::Base != self && table_exists?
diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb
index 94fce2db60..b3c8209a74 100644
--- a/activerecord/lib/active_record/attribute_methods/write.rb
+++ b/activerecord/lib/active_record/attribute_methods/write.rb
@@ -70,10 +70,6 @@ module ActiveRecord
attr_name = attr_name.to_s
attr_name = self.class.primary_key if attr_name == 'id' && self.class.primary_key
- unless has_attribute?(attr_name) || self.class.columns_hash.key?(attr_name)
- raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
- end
-
if should_type_cast
@attributes.write_from_user(attr_name, value)
else
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 3ef8878ad1..def04dbed2 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -381,7 +381,7 @@ module ActiveRecord
end
def table_exists?(name)
- return false unless name
+ return false unless name.present?
return true if tables(nil, nil, name).any?
name = name.to_s
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index c434b0d40a..3321e268d5 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -521,7 +521,7 @@ module ActiveRecord
def init_internals
pk = self.class.primary_key
- unless @attributes.include?(pk)
+ if pk && !@attributes.include?(pk)
@attributes.write_from_database(pk, nil)
end
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 7566af920f..2048e0d5ad 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -253,6 +253,15 @@ class AttributeMethodsTest < ActiveRecord::TestCase
assert_equal @loaded_fixtures['computers']['workstation'].to_hash, Computer.first.attributes
end
+ def test_attributes_without_primary_key
+ klass = Class.new(ActiveRecord::Base) do
+ self.table_name = 'developers_projects'
+ end
+
+ assert_equal klass.column_names, klass.new.attributes.keys
+ assert_not klass.new.attributes.key?('id')
+ end
+
def test_hashes_not_mangled
new_topic = { :title => "New Topic" }
new_topic_values = { :title => "AnotherTopic" }
diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index f43483d291..b04df7ce43 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -134,14 +134,22 @@ class PrimaryKeysTest < ActiveRecord::TestCase
end
def test_primary_key_returns_value_if_it_exists
+ klass = Class.new(ActiveRecord::Base) do
+ self.table_name = 'developers'
+ end
+
if ActiveRecord::Base.connection.supports_primary_key?
- assert_equal 'id', ActiveRecord::Base.connection.primary_key('developers')
+ assert_equal 'id', klass.primary_key
end
end
def test_primary_key_returns_nil_if_it_does_not_exist
+ klass = Class.new(ActiveRecord::Base) do
+ self.table_name = 'developers_projects'
+ end
+
if ActiveRecord::Base.connection.supports_primary_key?
- assert_nil ActiveRecord::Base.connection.primary_key('developers_projects')
+ assert_nil klass.primary_key
end
end
diff --git a/activerecord/test/models/contact.rb b/activerecord/test/models/contact.rb
index a1cb8d62b6..3ea17c3abf 100644
--- a/activerecord/test/models/contact.rb
+++ b/activerecord/test/models/contact.rb
@@ -8,6 +8,7 @@ module ContactFakeColumns
table_name => 'id'
}
+ column :id, :integer
column :name, :string
column :age, :integer
column :avatar, :binary