Declared broken for hash params with overlapping names
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 10k
- Forks
- 1.2k
- Avg merge
- 14h 38m
- Merged PRs (30d)
- 92
Description
Upgrading from 1.3.3, noticed several bugs around declared. I think some of them already have issues/PRs (eg. https://github.com/ruby-grape/grape/pull/2112 https://github.com/ruby-grape/grape/pull/2001) but don't see this one:
when passing null to hash params with unique names, the output is consistent (but still incorrect I think, but that's a different issue, see a note at the bottom):
gem 'grape', '1.6.0'
require 'grape'
class API < Grape::API
format :json
params do
optional(:aa, type: Hash)
optional(:ab, type: Hash)
end
put do
{
params: params,
declared: declared(params, include_missing: false)
}
end
end
run API
> curl -X PUT http://127.0.0.1:9292 -H 'Content-Type: application/json' -d '{"aa": null, "ab": null}' | jq
{
"params": {
"aa": null,
"ab": null
},
"declared": {
"aa": {},
"ab": {}
}
}
but when the param names overlap:
params do
optional(:aa, type: Hash)
optional(:aab, type: Hash)
end
> curl -X PUT http://127.0.0.1:9292 -H 'Content-Type: application/json' -d '{"aa": null, "aab": null}' | jq
{
"params": {
"aa": null,
"aab": null
},
"declared": {
"aa": null, # expected {}
"aab": {}
}
}
Present since 1.5.0, so it seems to be introduced by https://github.com/ruby-grape/grape/pull/2103, my guess is in this line https://github.com/ruby-grape/grape/blob/43936ac719ee524fd0f8ccd830d1eb50abd721c7/lib/grape/dsl/inside_route.rb#L98
Other than that:
Upgrading to >= 1.5.0 says behaviour changes only when params are missing and include_missing=true
Upgrading to >= 1.3.3 says that For now on, nil values stay nil values for all types, including arrays, sets and hashes.
so I assume changing nil to {} in the first example when null is passed explicitly (and include_missing isn't true as well) is an unintended behaviour as well?
This also makes it impossible to set nil value for params with type: Hash when using declared. While it can be bypassed by using eg. types: [Hash, anything] (single item triggers another bug where this time nil is changed to []), it's only because the mentioned PR doesn't take types under consideration at all, which is maybe yet another problem?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the overlapping-name and unique-name cases in the issue, then inspect lib/grape/dsl/inside_route.rb around the line linked by the reporter and compare it with PR 2103. Check the declared behavior against the documented include_missing and nil-value behavior; done means explicit null Hash parameters retain the intended distinction without regressing missing-parameter handling.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100