8000 [Validator] Removes exception when validating non-traversable value with Composite constraints by rchoquet · Pull Request #23040 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Validator] Removes exception when validating non-traversable value with Composite constraints #23040

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

Closed
wants to merge 1 commit into from
Closed

[Validator] Removes exception when validating non-traversable value with Composite constraints #23040

wants to merge 1 commit into from

Conversation

rchoquet
Copy link
@rchoquet rchoquet commented Jun 2, 2017
Q A
Branch? 2.8
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #16412
License MIT
Doc PR

This PR aims to solve a problem (might be considered a bug) we encountered when validating some API input.
As stated in the related issue, when validating a value that is not an array-like type against a composite constraint (either All or Collection), we get an exception.

This obviously does not cause much harm when used with the Form component where you control the data structure but is quite an issue when you must validate freely formatted content.

One of the obvious solution would have been to add a Type Constraint together with the Composite constraint but the constraints not being lazy executed, the exception is thrown anyway.

@rchoquet rchoquet changed the title [Validator] Removes exception when validating non-traversable value with Composite contraints [Validator] Removes exception when validating non-traversable value with Composite constraints Jun 2, 2017
@nicolas-grekas nicolas-grekas added this to the 2.8 milestone Jun 3, 2017
@fabpot
Copy link
Member
fabpot commented Jul 4, 2017

Is it really just on 2.8, or does 2.7 have the same issue?

@xabbuh
Copy link
Member
xabbuh commented Jul 4, 2017

I am against changing this only for one particular constraint. This basically affects all validators (see for example #12312, #13560 and #14943).

@rchoquet
Copy link
Author
rchoquet commented Jul 4, 2017

@xabbuh Make sense, I didn't even notice there were such opened issues.

I can give it a try by implementing a draft from the ideas at #12312, or maybe there's some WIP on it ?

@xabbuh
Copy link
Member
xabbuh commented Jul 4, 2017

One thought I had was to catch the UnexpectedTypeException inside the validator and attach a constraint violation globally there when such an exception is caught.

@rchoquet
Copy link
Author
rchoquet commented Jul 4, 2017

It seems to be the most convenient way to do it, but don't we lose the property path information for this violation ? And by doing so we can't customize the error message easily (as it won't be bound to a constraint anymore).

@nicolas-grekas
Copy link
Member

I'm closing because the PR is old and another approach is needed per the discussion. Feel free to resubmit of course. Thanks for the proposal.

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

Successfully merging this pull request may close these issues.

5 participants
0