From 1d999e681ea2bec8a5411c9302c7d6d17e4616ea Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 15 Jun 2022 18:48:16 +0100 Subject: [PATCH] Support `ActionController::Parameters#deep_merge` When [rails/rails#20868][] changed the `ActionController::Parameters` ancestory from `HashWithIndifferentAccess` to `Object`, support for `#deep_merge` and `#deep_merge!` were omitted. This commit restores support by integrating with [ActiveSupport::DeepMergeable](./activesupport/lib/active_support/deep_mergeable.rb). [rails/rails#20868]: https://github.com/rails/rails/pull/20868 Co-authored-by: Jonathan Hefner --- actionpack/CHANGELOG.md | 5 ++ .../metal/strong_parameters.rb | 35 +++++++++++- .../parameters/parameters_permit_test.rb | 56 +++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 7a99df54ccf..8042d107c1d 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,8 @@ +* Add support for `#deep_merge` and `#deep_merge!` to + `ActionController::Parameters`. + + *Sean Doyle* + ## Rails 7.1.0.beta1 (September 13, 2023) ## * `AbstractController::Translation.raise_on_missing_translations` removed diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 72a9a73d33b..f0137d06eef 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -4,6 +4,7 @@ require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/array/wrap" require "active_support/core_ext/string/filters" require "active_support/core_ext/object/to_query" +require "active_support/deep_mergeable" require "action_dispatch/http/upload" require "rack/test" require "stringio" @@ -137,6 +138,8 @@ module ActionController # params[:key] # => "value" # params["key"] # => "value" class Parameters + include ActiveSupport::DeepMergeable + cattr_accessor :permit_all_parameters, instance_accessor: false, default: false cattr_accessor :action_on_unpermitted_parameters, instance_accessor: false @@ -853,13 +856,41 @@ module ActionController ) end + ## + # :call-seq: merge!(other_hash) + # # Returns the current +ActionController::Parameters+ instance with # +other_hash+ merged into current hash. - def merge!(other_hash) - @parameters.merge!(other_hash.to_h) + def merge!(other_hash, &block) + @parameters.merge!(other_hash.to_h, &block) self end + ## + # :method: deep_merge + # :call-seq: deep_merge(other_hash, &block) + # + # Returns a new +ActionController::Parameters+ instance with +self+ and +other_hash+ merged recursively. + # + # Like with +Hash#merge+ in the standard library, a block can be provided + # to merge values. + # + #-- + # Implemented by ActiveSupport::DeepMergeable#deep_merge. + + ## + # :method: deep_merge! + # :call-seq: deep_merge!(other_hash, &block) + # + # Same as +#deep_merge+, but modifies +self+. + # + #-- + # Implemented by ActiveSupport::DeepMergeable#deep_merge!. + + def deep_merge?(other_hash) # :nodoc + other_hash.is_a?(ActiveSupport::DeepMergeable) + end + # Returns a new +ActionController::Parameters+ instance with all keys # from current hash merged into +other_hash+. def reverse_merge(other_hash) diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb index e3ecfa0266b..97acfd46529 100644 --- a/actionpack/test/controller/parameters/parameters_permit_test.rb +++ b/actionpack/test/controller/parameters/parameters_permit_test.rb @@ -319,6 +319,62 @@ class ParametersPermitTest < ActiveSupport::TestCase assert_equal "32", @params[:person][:age] end + test "not permitted is sticky beyond deep merges" do + assert_not_predicate @params.deep_merge(a: "b"), :permitted? + end + + test "permitted is sticky beyond deep merges" do + @params.permit! + assert_predicate @params.deep_merge(a: "b"), :permitted? + end + + test "not permitted is sticky beyond deep_merge!" do + assert_not_predicate @params.deep_merge!(a: "b"), :permitted? + end + + test "permitted is sticky beyond deep_merge!" do + @params.permit! + assert_predicate @params.deep_merge!(a: "b"), :permitted? + end + + test "deep_merge with other Hash" do + first, last = @params.dig(:person, :name).values_at(:first, :last) + merged_params = @params.deep_merge(person: { name: { last: "A." } }) + + assert_equal first, merged_params.dig(:person, :name, :first) + assert_not_equal last, merged_params.dig(:person, :name, :last) + assert_equal "A.", merged_params.dig(:person, :name, :last) + end + + test "deep_merge! with other Hash" do + first, last = @params.dig(:person, :name).values_at(:first, :last) + @params.deep_merge!(person: { name: { last: "A." } }) + + assert_equal first, @params.dig(:person, :name, :first) + assert_not_equal last, @params.dig(:person, :name, :last) + assert_equal "A.", @params.dig(:person, :name, :last) + end + + test "deep_merge with other Parameters" do + first, last = @params.dig(:person, :name).values_at(:first, :last) + other_params = ActionController::Parameters.new(person: { name: { last: "A." } }).permit! + merged_params = @params.deep_merge(other_params) + + assert_equal first, merged_params.dig(:person, :name, :first) + assert_not_equal last, merged_params.dig(:person, :name, :last) + assert_equal "A.", merged_params.dig(:person, :name, :last) + end + + test "deep_merge! with other Parameters" do + first, last = @params.dig(:person, :name).values_at(:first, :last) + other_params = ActionController::Parameters.new(person: { name: { last: "A." } }).permit! + @params.deep_merge!(other_params) + + assert_equal first, @params.dig(:person, :name, :first) + assert_not_equal last, @params.dig(:person, :name, :last) + assert_equal "A.", @params.dig(:person, :name, :last) + end + test "#reverse_merge with parameters" do default_params = ActionController::Parameters.new(id: "1234", person: {}).permit! merged_params = @params.reverse_merge(default_params)