Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a performance issue with dependent params #2127

Merged
merged 1 commit into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#2127](https://github.com/ruby-grape/grape/pull/2127): Fix a performance issue with dependent params - [@dnesteryuk](https://github.com/dnesteryuk).
* [#2126](https://github.com/ruby-grape/grape/pull/2126): Fix warnings about redefined attribute accessors in `AttributeTranslator` - [@samsonjs](https://github.com/samsonjs).
* [#2121](https://github.com/ruby-grape/grape/pull/2121): Fix 2.7 deprecation warning in validator_factory - [@Legogris](https://github.com/Legogris).
* [#2115](https://github.com/ruby-grape/grape/pull/2115): Fix declared_params regression with multiple allowed types - [@stanhu](https://github.com/stanhu).
Expand Down
22 changes: 22 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
Upgrading Grape
===============

### Upgrading to >= 1.5.1

#### Dependent params

If you use [dependent params](https://github.com/ruby-grape/grape#dependent-parameters) with
`Grape::Extensions::Hash::ParamBuilder`, make sure a parameter to be dependent on is set as a Symbol.
If a String is given, a parameter that other parameters depend on won't be found even if it is present.
Copy link
Member

Choose a reason for hiding this comment

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

So is this a bug? Should we also fix this (in another PR)?


_Correct_:
```ruby
given :matrix do
# dependent params
end
```

_Wrong_:
```ruby
given 'matrix' do
# dependent params
end
```

### Upgrading to >= 1.5.0

Prior to 1.3.3, the `declared` helper would always return the complete params structure if `include_missing=true` was set. In 1.3.3 a regression was introduced such that a missing Hash with or without nested parameters would always resolve to `{}`.
Expand Down
5 changes: 3 additions & 2 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ def meets_dependency?(params, request_params)
end

return params.any? { |param| meets_dependency?(param, request_params) } if params.is_a?(Array)
return false unless params.respond_to?(:with_indifferent_access)
params = params.with_indifferent_access

# params might be anything what looks like a hash, so it must implement a `key?` method
return false unless params.respond_to?(:key?)

@dependent_on.each do |dependency|
if dependency.is_a?(Hash)
Expand Down