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

regression: parameters bound by given always applied/checked when nested #1659

Closed
jnardone opened this issue Jul 13, 2017 · 2 comments
Closed
Labels

Comments

@jnardone
Copy link

jnardone commented Jul 13, 2017

This is a regression from 0.19.2 -> 1.0.0

Consider a declaration like:

     subject.params do
        requires :a, type: String, allow_blank: false, values: %w(x y)
        given a: ->(val) { val == 'x' } do
          requires :inner1, type: Hash, allow_blank: false do
            requires :foo, type: Integer, allow_blank: false
          end
        end
        given a: ->(val) { val == 'y' } do
          requires :inner2, type: Hash, allow_blank: false do
            requires :bar, type: Integer, allow_blank: false
          end
        end
      end

The parameters for "inner1" / "inner2" should only be applied when the given block matches, but in the nested case (like above) the parameter validations are always applied even if they do not match the given criteria.

For example, doing a request where a is x, you will get a 400 with inner2[bar] is missing, inner2[bar] is empty

Seems like this must have broken with #1625 @rnubel @avellable ?

@jnardone jnardone changed the title regression: "given" checks fail with nested parameters regression: parameters bound by given always applied/checked when nested Jul 13, 2017
@dblock dblock added the bug? label Jul 13, 2017
@rnubel
Copy link
Contributor

rnubel commented Jul 13, 2017

Thanks for the great write-up! #1661 should resolve this, and has some explanation as to why it regressed.

dblock pushed a commit that referenced this issue Jul 14, 2017
* Repro issue #1659

* [Fix] Handle deeply-nested dependencies with `given`.

Behind the scenes, each call to `requires` or other params DSL method
pushes an entry onto a flat list of validators. The nesting structure
that your parameters can take on is tracked as an up-tree separately
on each scope, but that relationship isn't used to traverse the validations.
So, when I moved the dependency checking out of `should_validate?` and into
the actual validation, the `given` dependency stopped taking effect after
you nested parameters more than one level deep underneath.

To restore the behavior, I made the validation check recursively upwards
to see if it should or should not validate that scope.

* Add changelog entry.
@dblock
Copy link
Member

dblock commented Jul 14, 2017

Closed via #1661

@dblock dblock closed this as completed Jul 14, 2017
jdurand pushed a commit to jdurand/grape that referenced this issue Jan 25, 2019
* Repro issue ruby-grape#1659

* [Fix] Handle deeply-nested dependencies with `given`.

Behind the scenes, each call to `requires` or other params DSL method
pushes an entry onto a flat list of validators. The nesting structure
that your parameters can take on is tracked as an up-tree separately
on each scope, but that relationship isn't used to traverse the validations.
So, when I moved the dependency checking out of `should_validate?` and into
the actual validation, the `given` dependency stopped taking effect after
you nested parameters more than one level deep underneath.

To restore the behavior, I made the validation check recursively upwards
to see if it should or should not validate that scope.

* Add changelog entry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants