Skip to content

Comments

Add a flag to disable deprecated AC::Parameters comparison#44826

Merged
tenderlove merged 1 commit intorails:mainfrom
stefkin:acp-eql-flag
Apr 4, 2022
Merged

Add a flag to disable deprecated AC::Parameters comparison#44826
tenderlove merged 1 commit intorails:mainfrom
stefkin:acp-eql-flag

Conversation

@stefkin
Copy link
Contributor

@stefkin stefkin commented Apr 1, 2022

Summary

The PR #44812 added a deprecation for the comparison of AC::Params with Hashes.

The existing implementation can sometimes lead to the following bug:

Given a process where hashes of AC::Parameters and equivalent Hash collide

hash = { "bar" => [{"foo" => 1}]}
acp = ActionController::Parameters.new(hash)
h = {}
h[acp] = true
h[hash] # => true

the following happens:

hash = { "bar" => [{"foo" => 1}]}
acp = ActionController::Parameters.new(hash)
acp.fetch("bar")
acp["bar"] # => [{"foo"=>1}]

Reads of the key with fetch (also delete, values_at) lead to all subsequent read operations of the key to return an array of raw Hashes.

The addition of the flag is going to allow affected users to turn off the deprecated implementation and avoid the bug without waiting for the next release cycle.

@rails-bot rails-bot bot added the actionpack label Apr 1, 2022
Copy link
Member

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should talk about the config in this message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added the following to the warning

To disable the deprecated behaviour set
`Rails.application.config.action_controller.allow_deprecated_parameters_hash_equality = false`.

@tenderlove tenderlove merged commit cfa7284 into rails:main Apr 4, 2022
matsales28 added a commit to matsales28/shoulda-matchers that referenced this pull request Oct 27, 2023
The behavior of the comparison between hash and a
`ActionController::Parameters` was changed and now will be deprecated.

Check this Rails PR for more context
rails/rails#44826.
matsales28 added a commit to thoughtbot/shoulda-matchers that referenced this pull request Nov 17, 2023
* Add Rails 7.1 to appraisals

* fix: Adjust validate options when managing columns and tables in migration

This commit adjusts the way we manage columns and tables in migrations
to account for the changes in Rails 7.1.0, it was introduced in this
Rails version a validation around the options passed to tables and
columns in migrations, so we need to adjust our code to skip this
validation.

This PR introduced this new validation on the migrations:
rails/rails#46178

* Refine `ActiveRecord::SerializeMatcher` for Rails 7.1 compatibility

This commit refines the `ActiveRecord::SerializeMatcher` to
accommodate changes in the public API introduced in Rails 7.1.
The `serialize` method's API has been updated, necessitating
adjustments to our matcher specs to align with the new API.

While this PR addresses the immediate need for compatibility,
there is potential for further enhancements and refinements in
the matcher's implementation to bring it in line with the revised
public API. However, such improvements will be explored in a future PR.

For reference, the changes in the public API can be found in the following PR: rails/rails#47463

This commit adjusts the `ActiveRecord::SerializeMatcher` the public
API for the `serialize` method has changed on Rails 7.1, so we
had to

* feat: Upadte workflow matrix

* fix: Adjust comparison between hash and `ActionController::Parameters`

The behavior of the comparison between hash and a
`ActionController::Parameters` was changed and now will be deprecated.

Check this Rails PR for more context
rails/rails#44826.

* fix: Adjust usage of `has_secure_token` without a token attr

The behaviour of this method was changed in Rails 7.1, and now
it'll search for a `token` attr in the initialization of the record
instead of the creation. To stay with the same behaviour as before
it's necessary to pass a new argument `on: : create` to the method.

* fix: Adjust specs for primary_key check

* fix: Test updating error_highlight gem

* fix: Add default Rails tables to acceptance spec

This commit fixes the problem of not finding the tables
when running the specs in eager load mode, this was introduced
by Rails 7.1.

Load the model schema when running test in eager load
context rails/rails#49470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants