From 926c4b95e49cc65a1d7420392c95d2193b099965 Mon Sep 17 00:00:00 2001 From: wangjohn Date: Thu, 21 Mar 2013 17:10:17 -0400 Subject: Raising an error when nil or non-hash is passed to update_attributes. --- activerecord/CHANGELOG.md | 10 ++++++++++ activerecord/lib/active_record/attribute_assignment.rb | 4 +++- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/persistence_test.rb | 15 +++++++++++---- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index e38d8aecf5..980d132051 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,13 @@ +* Calling `update_attributes` will now throw an `ArgumentError` whenever it + gets a `nil` argument. More specifically, it will throw an error if the + argument that it gets passed does not respond to to `stringify_keys`. + + `Example:` + + @my_comment.update_attributes() # => raises ArgumentError + + *John Wang* + * Remove Oracle / Sqlserver / Firebird database tasks that were deprecated in 4.0. *kennyj* diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 75377bba57..53ce5f4952 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -13,7 +13,9 @@ module ActiveRecord # of this method is +false+ an ActiveModel::ForbiddenAttributesError # exception is raised. def assign_attributes(new_attributes) - return if new_attributes.blank? + if !new_attributes.respond_to?(:stringify_keys) + raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." + end attributes = new_attributes.stringify_keys multi_parameter_attributes = [] diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index ee0150558d..a3a7d338d1 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -92,7 +92,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_set_attributes_without_hash topic = Topic.new - assert_nothing_raised { topic.attributes = '' } + assert_raise(ArgumentError) { topic.attributes = '' } end def test_integers_as_nil diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 30dc2a34c6..6cd3e2154e 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -419,10 +419,6 @@ class PersistenceTest < ActiveRecord::TestCase assert !Topic.find(1).approved? end - def test_update_attribute_does_not_choke_on_nil - assert Topic.find(1).update(nil) - end - def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } @@ -701,6 +697,17 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal topic.title, Topic.find(1234).title end + def test_update_attributes_parameters + topic = Topic.find(1) + assert_nothing_raised do + topic.update_attributes({}) + end + + assert_raises(ArgumentError) do + topic.update_attributes(nil) + end + end + def test_update! Reply.validates_presence_of(:title) reply = Reply.find(2) -- cgit v1.2.3