Skip to content

Commit

Permalink
Merge pull request #45221 from jhawthorn/ac_params_eql_fix
Browse files Browse the repository at this point in the history
Fix eql? of AC::Parameters to match hash
  • Loading branch information
jhawthorn committed Sep 6, 2022
1 parent 8525e57 commit 4d25c64
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 2 deletions.
9 changes: 7 additions & 2 deletions actionpack/lib/action_controller/metal/strong_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,15 @@ def ==(other)
@parameters == other
end
end
alias eql? ==

def eql?(other)
self.class == other.class &&
permitted? == other.permitted? &&
parameters.eql?(other.parameters)
end

def hash
[@parameters.hash, @permitted].hash
[self.class, @parameters, @permitted].hash
end

# Returns a safe <tt>ActiveSupport::HashWithIndifferentAccess</tt>
Expand Down
66 changes: 66 additions & 0 deletions actionpack/test/controller/parameters/equality_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true

require "abstract_unit"
require "action_controller/metal/strong_parameters"

class ParametersAccessorsTest < ActiveSupport::TestCase
setup do
ActionController::Parameters.permit_all_parameters = false

@params = ActionController::Parameters.new(
person: {
age: "32",
name: {
first: "David",
last: "Heinemeier Hansson"
},
addresses: [{ city: "Chicago", state: "Illinois" }]
}
)
end

test "comparison works" do
@hash = @params.each_pair.to_h
assert_equal @params, @hash
end

test "not eql? to equivalent hash" do
@hash = {}
@params = ActionController::Parameters.new(@hash)
assert_not_deprecated do
assert_not @params.eql?(@hash)
end
end

test "not eql? to equivalent nested hash" do
@params1 = ActionController::Parameters.new({ foo: {} })
@params2 = ActionController::Parameters.new({ foo: ActionController::Parameters.new({}) })
assert_not_deprecated do
assert_not @params1.eql?(@params2)
end
end

test "not eql? when permitted is different" do
permitted = @params.permit(:person)
assert_not @params.eql?(permitted)
end

test "eql? when equivalent" do
permitted = @params.permit(:person)
assert @params.permit(:person).eql?(permitted)
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
params = ActionController::Parameters.new(foo: { bar: "baz" })
assert params.has_value?(ActionController::Parameters.new("bar" => "baz"))
end
end
24 changes: 24 additions & 0 deletions actionpack/test/controller/parameters_integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ class IntegrationController < ActionController::Base
def yaml_params
render plain: params.to_yaml
end

def permit_params
params.permit(
key1: {}
)

render plain: "Home"
end
end

class ActionControllerParametersIntegrationTest < ActionController::TestCase
Expand All @@ -24,4 +32,20 @@ class ActionControllerParametersIntegrationTest < ActionController::TestCase
YAML
assert_equal expected, response.body
end

# Ensure no deprecation warning from comparing AC::Parameters against Hash
# See https://github.com/rails/rails/issues/44940
test "identical arrays can be permitted" do
params = {
key1: {
a: [{ same_key: { c: 1 } }],
b: [{ same_key: { c: 1 } }]
}
}

assert_not_deprecated do
post :permit_params, params: params
end
assert_response :ok
end
end

0 comments on commit 4d25c64

Please sign in to comment.