grape
grape copied to clipboard
Issue with same param name in different given blocks
.....
given name: ->(val) do
val == 'name'
end do
requires :values, type: Array[String]
end
given name: ->(val) do
val == 'another_name'
end do
requires :values, type: Array[Json]
end
....
Seems like the check on the values param in the second block override the first. When both the blocks are executed in the same call the second check on values
fails as invalid.
Sounds like a bug?
Looks like a non-feature of supporting two given
blocks with the same key?
Turn this into a spec, maybe implement/fix?
@dblock I will add a spec.
@dblock can you point me to the best place for adding the test case?
I would say spec/grape/validations/params_scope_spec.rb has most of the given
specs.
^^ My workaround should NOT be used. It appears to work by breaking parameter validation.
I spent time looking into this today, and I think I understand the problem, but I believe the fix would require a non-trivial change to how this library works.
Parameter declarations are set when the class is loaded, not when a request is made. This manifests as a problem related to given
but it’s not exactly that.
Suppose you were to do this:
requires :type, type: String
given type: ->(val) { val == 'typeA' } do
requires :attributes, type: Hash do
requires :foo, type: String, values: ['foo-a', 'foo-b']
requires :bar, type: String
end
end
given type: ->(val) { val == 'typeB' } do
requires :attributes, type: Hash do
requires :foo, type: Integer, values: [1, 2]
end
end
In terms of validations, this actually works as expected:
# Requests with these params will pass validation, as we want
{type: 'typeA', attributes: { foo: 'foo-a', bar: 'z' }}
{type: 'typeB', attributes: { foo: 1 }}
# Requests with these params will fail to validate, as we want
{type: 'typeA', attributes: { foo: 'foo-a' }} # requires bar param
{type: 'typeA', attributes: { foo: 1, bar: 'y' }} # foo needs to be in ['foo-a', 'foo-b']
{type: 'typeB', attributes: { foo: 'foo-a' }} # foo needs to be in [1, 2]
This is good, but the good news stops there. If we try to use declared()
to access our declared params, things fall apart:
# If we make a request with these params,
{type: 'typeA', attributes: { foo: 'foo-a', bar: 'z' }}
# Then declared(params) gives us:
{type: 'typeA', attributes: { foo: 'foo-a' }} # Note how the `bar` param is missing even though we declared it!
The reason is that declared params are determined when the class is loaded, so the second requires :attributes
block overwrites the first one. The given
block is entirely irrelevant when the parameter stack is being determined at load-time, and from the perspective of the declared()
helper, the requires :bar
declaration doesn't exist.
In order to solve this problem, I think we would need for two things to happen:
- The block contents of one parameter should be appended to the block contents of another parameter, rather than overwriting them.
- We would need for the
declared()
helper to appropriately take into account the results of the proc supplied togiven
.