diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index e20ebde8b1..9cebb680fc 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -6,7 +6,7 @@ include ActiveModel::AttributeAssignment attr_accessor :name, :status end - + cat = Cat.new cat.assign_attributes(name: "Gorby", status: "yawning") cat.name # => 'Gorby' diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb index 36ddbad826..356421476c 100644 --- a/activemodel/lib/active_model/attribute_assignment.rb +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -34,6 +34,7 @@ module ActiveModel end private + def _assign_attributes(attributes) attributes.each do |k, v| _assign_attribute(k, v) @@ -41,10 +42,8 @@ module ActiveModel end def _assign_attribute(k, v) - public_send("#{k}=", v) - rescue NoMethodError if respond_to?("#{k}=") - raise + public_send("#{k}=", v) else raise UnknownAttributeError.new(self, k) end diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb index 5c1be6a456..402caf21f7 100644 --- a/activemodel/test/cases/attribute_assignment_test.rb +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -1,8 +1,7 @@ -require 'cases/helper' -require 'active_support/hash_with_indifferent_access' +require "cases/helper" +require "active_support/hash_with_indifferent_access" class AttributeAssignmentTest < ActiveModel::TestCase - class Model include ActiveModel::AttributeAssignment @@ -13,13 +12,15 @@ class AttributeAssignmentTest < ActiveModel::TestCase end def broken_attribute=(value) - non_existing_method(value) + raise ErrorFromAttributeWriter end - private - def metadata=(data) - @metadata = data - end + protected + + attr_writer :metadata + end + + class ErrorFromAttributeWriter < StandardError end class ProtectedParams < ActiveSupport::HashWithIndifferentAccess @@ -41,14 +42,14 @@ class AttributeAssignmentTest < ActiveModel::TestCase test "simple assignment" do model = Model.new - model.assign_attributes(name: 'hello', description: 'world') - assert_equal 'hello', model.name - assert_equal 'world', model.description + model.assign_attributes(name: "hello", description: "world") + assert_equal "hello", model.name + assert_equal "world", model.description end test "assign non-existing attribute" do model = Model.new - error = assert_raises ActiveModel::AttributeAssignment::UnknownAttributeError do + error = assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do model.assign_attributes(hz: 1) end @@ -58,49 +59,49 @@ class AttributeAssignmentTest < ActiveModel::TestCase test "assign private attribute" do model = Model.new - assert_raises ActiveModel::AttributeAssignment::UnknownAttributeError do + assert_raises(ActiveModel::AttributeAssignment::UnknownAttributeError) do model.assign_attributes(metadata: { a: 1 }) end end - test "raises NoMethodError if raised in attribute writer" do - assert_raises NoMethodError do + test "does not swallow errors raised in an attribute writer" do + assert_raises(ErrorFromAttributeWriter) do Model.new(broken_attribute: 1) end end - test "raises ArgumentError if non-hash object passed" do - assert_raises ArgumentError do + test "an ArgumentError is raised if a non-hash-like obejct is passed" do + assert_raises(ArgumentError) do Model.new(1) end end - test 'forbidden attributes cannot be used for mass assignment' do - params = ProtectedParams.new(name: 'Guille', description: 'm') + test "forbidden attributes cannot be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "m") + assert_raises(ActiveModel::ForbiddenAttributesError) do Model.new(params) end end - test 'permitted attributes can be used for mass assignment' do - params = ProtectedParams.new(name: 'Guille', description: 'desc') + test "permitted attributes can be used for mass assignment" do + params = ProtectedParams.new(name: "Guille", description: "desc") params.permit! model = Model.new(params) - assert_equal 'Guille', model.name - assert_equal 'desc', model.description + assert_equal "Guille", model.name + assert_equal "desc", model.description end - test 'regular hash should still be used for mass assignment' do - model = Model.new(name: 'Guille', description: 'm') + test "regular hash should still be used for mass assignment" do + model = Model.new(name: "Guille", description: "m") - assert_equal 'Guille', model.name - assert_equal 'm', model.description + assert_equal "Guille", model.name + assert_equal "m", model.description end - test 'blank attributes should not raise' do + test "assigning no attributes should not raise, even if the hash is un-permitted" do model = Model.new assert_nil model.assign_attributes(ProtectedParams.new({})) end - end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index e283db0386..e368fdfff9 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -22,22 +22,10 @@ module ActiveRecord assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty? end - # Allows you to set all the attributes by passing in a hash of attributes with - # keys matching the attribute names (which again matches the column names). - # - # If the passed hash responds to permitted? method and the return value - # of this method is +false+ an ActiveModel::ForbiddenAttributesError - # exception is raised. - # - # cat = Cat.new(name: "Gorby", status: "yawning") - # cat.attributes # => { "name" => "Gorby", "status" => "yawning", "created_at" => nil, "updated_at" => nil} - # cat.assign_attributes(status: "sleeping") - # cat.attributes # => { "name" => "Gorby", "status" => "sleeping", "created_at" => nil, "updated_at" => nil } - # - # New attributes will be persisted in the database when the object is saved. - # - # Aliased to assign_attributes. - alias attributes= assign_attributes + # Alias for `assign_attributes`. See +ActiveModel::AttributeAssignment+ + def attributes=(attributes) + assign_attributes(attributes) + end private