Declared broken for hash params with overlapping names
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?
Ouch. These look like real problems. Turn your example(s) into specs and see if you can fix'em? At least PR the specs.
I think the other problem is real and needs to be similarly investigated.
I've just sent a PR for solving this https://github.com/ruby-grape/grape/pull/2372
Solved in #2372 :)