Skip to content

Commit

Permalink
Support ActionController::Parameters#deep_merge
Browse files Browse the repository at this point in the history
When [#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).

[#20868]: #20868

Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
  • Loading branch information
seanpdoyle and jonathanhefner committed Sep 26, 2023
1 parent 5fcc610 commit 1d999e6
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 2 deletions.
5 changes: 5 additions & 0 deletions 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
Expand Down
35 changes: 33 additions & 2 deletions actionpack/lib/action_controller/metal/strong_parameters.rb
Expand Up @@ -4,6 +4,7 @@
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"
Expand Down Expand Up @@ -137,6 +138,8 @@ class InvalidParameterKey < ArgumentError
# 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
Expand Down Expand Up @@ -853,13 +856,41 @@ def merge(other_hash)
)
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)
Expand Down
56 changes: 56 additions & 0 deletions actionpack/test/controller/parameters/parameters_permit_test.rb
Expand Up @@ -319,6 +319,62 @@ def walk_permitted(params)
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)
Expand Down

0 comments on commit 1d999e6

Please sign in to comment.