Make behaviour of has_value?/value? more consistent

This commit is contained in:
Seva Stefkin 2022-04-09 16:48:39 +02:00
parent 8f39fbe18a
commit 743ab43686
No known key found for this signature in database
GPG Key ID: A0B58514E547A59B
3 changed files with 44 additions and 22 deletions

View File

@ -1,3 +1,10 @@
* Reimplement `ActionController::Parameters#has_value?` and `#value?` to avoid parameters and hashes comparison.
Deprecated equality between parameters and hashes is going to be removed in Rails 7.2.
The new implementation takes care of conversions.
*Seva Stefkin*
* Allow only String and Symbol keys in `ActionController::Parameters`.
Raise `ActionController::InvalidParameterKey` when initializing Parameters
with keys that aren't strings or symbols.

View File

@ -177,14 +177,6 @@ module ActionController
#
# Returns true if the given key is present in the parameters.
##
# :method: has_value?
#
# :call-seq:
# has_value?(value)
#
# Returns true if the given value is present for some key in the parameters.
##
# :method: include?
#
@ -225,14 +217,7 @@ module ActionController
#
# Returns the content of the parameters as a string.
##
# :method: value?
#
# :call-seq:
# value?(value)
#
# Returns true if the given value is present for some key in the parameters.
delegate :keys, :key?, :has_key?, :member?, :has_value?, :value?, :empty?, :include?,
delegate :keys, :key?, :has_key?, :member?, :empty?, :include?,
:as_json, :to_s, :each_key, to: :@parameters
# By default, never raise an UnpermittedParameters exception if these
@ -856,6 +841,13 @@ module ActionController
reject! { |_k, v| v.blank? }
end
# Returns true if the given value is present for some key in the parameters.
def has_value?(value)
each_value.include?(convert_value_to_parameters(value))
end
alias value? has_value?
# Returns values that were assigned to the given +keys+. Note that all the
# +Hash+ objects will be converted to <tt>ActionController::Parameters</tt>.
def values_at(*keys)

View File

@ -6,7 +6,6 @@ require "action_controller/metal/strong_parameters"
class ParametersAccessorsTest < ActiveSupport::TestCase
setup do
ActionController::Parameters.permit_all_parameters = false
ActionController::Parameters.allow_deprecated_parameters_hash_equality = true
@params = ActionController::Parameters.new(
person: {
@ -106,11 +105,11 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
end
test "deprecated comparison disabled" do
ActionController::Parameters.allow_deprecated_parameters_hash_equality = false
assert_kind_of Enumerator, @params.each_pair
assert_not_deprecated do
assert_not_equal @params, @params.each_pair.to_h
without_deprecated_params_hash_equality do
assert_kind_of Enumerator, @params.each_pair
assert_not_deprecated do
assert_not_equal @params, @params.each_pair.to_h
end
end
end
@ -419,4 +418,28 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
@params.dig(:person, :addresses)[0] = { city: "Boston", state: "Massachusetts" }
assert_equal "Boston", @params.dig(:person, :addresses, 0, :city)
end
test "has_value? converts hashes to parameters" do
assert_not_deprecated do
params = ActionController::Parameters.new(foo: { bar: "baz" })
assert params.has_value?("bar" => "baz")
params[:foo] # converts value to AC::Params
assert params.has_value?("bar" => "baz")
end
end
test "has_value? works with parameters" do
without_deprecated_params_hash_equality do
params = ActionController::Parameters.new(foo: { bar: "baz" })
assert params.has_value?(ActionController::Parameters.new("bar" => "baz"))
end
end
private
def without_deprecated_params_hash_equality
ActionController::Parameters.allow_deprecated_parameters_hash_equality = false
yield
ensure
ActionController::Parameters.allow_deprecated_parameters_hash_equality = true
end
end