From 8b59960cd42c6243f2519972a6a5101045b1c209 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 31 Jan 2020 16:51:17 -0500 Subject: [PATCH 1/4] Do not stringify attributes in assign_attributes --- activemodel/lib/active_model/attribute_assignment.rb | 7 +++---- activerecord/lib/active_record/attribute_assignment.rb | 8 +++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb index 9bdec0dfda..466986d6e6 100644 --- a/activemodel/lib/active_model/attribute_assignment.rb +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -26,13 +26,12 @@ module ActiveModel # cat.name # => 'Gorby' # cat.status # => 'sleeping' def assign_attributes(new_attributes) - if !new_attributes.respond_to?(:stringify_keys) + unless new_attributes.respond_to?(:stringify_keys) raise ArgumentError, "When assigning attributes, you must pass a hash as an argument, #{new_attributes.class} passed." end return if new_attributes.empty? - attributes = new_attributes.stringify_keys - _assign_attributes(sanitize_for_mass_assignment(attributes)) + _assign_attributes(sanitize_for_mass_assignment(new_attributes.dup)) end alias attributes= assign_attributes @@ -49,7 +48,7 @@ module ActiveModel if respond_to?(setter) public_send(setter, v) else - raise UnknownAttributeError.new(self, k) + raise UnknownAttributeError.new(self, k.to_s) end end end diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index acb8ba7e5a..54c315a576 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -12,10 +12,12 @@ module ActiveRecord nested_parameter_attributes = {} attributes.each do |k, v| - if k.include?("(") - multi_parameter_attributes[k] = attributes.delete(k) + key = k.to_s + + if key.include?("(") + multi_parameter_attributes[key] = attributes.delete(k) elsif v.is_a?(Hash) - nested_parameter_attributes[k] = attributes.delete(k) + nested_parameter_attributes[key] = attributes.delete(k) end end super(attributes) From 112c3ec0f5a34d1b6219defb40b1a9590f53ef39 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 6 Feb 2020 17:02:56 -0500 Subject: [PATCH 2/4] Only dup attributes in activerecord attribute_assignment --- activemodel/lib/active_model/attribute_assignment.rb | 2 +- activerecord/lib/active_record/attribute_assignment.rb | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb index 466986d6e6..61aba61ba9 100644 --- a/activemodel/lib/active_model/attribute_assignment.rb +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -31,7 +31,7 @@ module ActiveModel end return if new_attributes.empty? - _assign_attributes(sanitize_for_mass_assignment(new_attributes.dup)) + _assign_attributes(sanitize_for_mass_assignment(new_attributes)) end alias attributes= assign_attributes diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 54c315a576..cd14e66c5b 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -10,17 +10,18 @@ module ActiveRecord def _assign_attributes(attributes) multi_parameter_attributes = {} nested_parameter_attributes = {} + new_attributes = attributes.dup - attributes.each do |k, v| + new_attributes.each do |k, v| key = k.to_s if key.include?("(") - multi_parameter_attributes[key] = attributes.delete(k) + multi_parameter_attributes[key] = new_attributes.delete(k) elsif v.is_a?(Hash) - nested_parameter_attributes[key] = attributes.delete(k) + nested_parameter_attributes[key] = new_attributes.delete(k) end end - super(attributes) + super(new_attributes) assign_nested_parameter_attributes(nested_parameter_attributes) unless nested_parameter_attributes.empty? assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty? From 2e9e940e22c2231272e979bdd72a71764dec941f Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 7 Feb 2020 11:36:35 -0500 Subject: [PATCH 3/4] Change safe guard to check for each_pair instead of stringify_keys --- activemodel/lib/active_model/attribute_assignment.rb | 2 +- activemodel/test/cases/attribute_assignment_test.rb | 4 ++-- activerecord/test/support/stubs/strong_parameters.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/activemodel/lib/active_model/attribute_assignment.rb b/activemodel/lib/active_model/attribute_assignment.rb index 61aba61ba9..ea4dae101c 100644 --- a/activemodel/lib/active_model/attribute_assignment.rb +++ b/activemodel/lib/active_model/attribute_assignment.rb @@ -26,7 +26,7 @@ module ActiveModel # cat.name # => 'Gorby' # cat.status # => 'sleeping' def assign_attributes(new_attributes) - unless new_attributes.respond_to?(:stringify_keys) + unless new_attributes.respond_to?(:each_pair) raise ArgumentError, "When assigning attributes, you must pass a hash as an argument, #{new_attributes.class} passed." end return if new_attributes.empty? diff --git a/activemodel/test/cases/attribute_assignment_test.rb b/activemodel/test/cases/attribute_assignment_test.rb index 30e8419685..4352714f12 100644 --- a/activemodel/test/cases/attribute_assignment_test.rb +++ b/activemodel/test/cases/attribute_assignment_test.rb @@ -49,8 +49,8 @@ class AttributeAssignmentTest < ActiveModel::TestCase @parameters end - def stringify_keys - dup + def each_pair(&block) + @parameters.each_pair(&block) end def dup diff --git a/activerecord/test/support/stubs/strong_parameters.rb b/activerecord/test/support/stubs/strong_parameters.rb index da8f9892f9..8e57c7e776 100644 --- a/activerecord/test/support/stubs/strong_parameters.rb +++ b/activerecord/test/support/stubs/strong_parameters.rb @@ -28,8 +28,8 @@ class ProtectedParams end alias to_unsafe_h to_h - def stringify_keys - dup + def each_pair(&block) + @parameters.each_pair(&block) end def dup From d472111a817fee21dd5092c7a76e63b59a2d68b1 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 7 Feb 2020 16:40:48 -0500 Subject: [PATCH 4/4] Add ar assign_attributes to avoid unnecessary attributes dup --- .../lib/active_record/attribute_assignment.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index cd14e66c5b..2882b1b583 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -6,22 +6,25 @@ module ActiveRecord module AttributeAssignment include ActiveModel::AttributeAssignment + def assign_attributes(attributes) + super(attributes.dup) + end + private def _assign_attributes(attributes) multi_parameter_attributes = {} nested_parameter_attributes = {} - new_attributes = attributes.dup - new_attributes.each do |k, v| + attributes.each do |k, v| key = k.to_s if key.include?("(") - multi_parameter_attributes[key] = new_attributes.delete(k) + multi_parameter_attributes[key] = attributes.delete(k) elsif v.is_a?(Hash) - nested_parameter_attributes[key] = new_attributes.delete(k) + nested_parameter_attributes[key] = attributes.delete(k) end end - super(new_attributes) + super(attributes) assign_nested_parameter_attributes(nested_parameter_attributes) unless nested_parameter_attributes.empty? assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty?