[Validator] Add a constraint to sequentially validate a set of constraints #34456

ogizanagi · 2019-11-19T13:55:23Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	N/A
License	MIT
Doc PR	Todo

Follows #20017 (comment) given some feedbacks about the suggested feature.

/**
 * @var string
 *
 * @Assert\Sequentially({
 *     @Assert\Type("string"),
 *     @Assert\Length(min="4"),
 *     @Assert\Regex("[a-z]"),
 *     @SomeCustomConstraintWithHeavyExternalCalls(),
 * })
 */
public $foo;

This new Sequentially constraint solves - with less power but better DX - some of the use-cases of the GroupSequence feature, allowing to interrupt the validation of some constraints if a previous one in the list failed before. Constraints are validated in given order, and the first violation raised will prevent other constraint validators to be executed.
It can either prevent unexpected type exceptions thrown by further constraints or heavy & unnecessary calls to a database or external services if the value to validate already doesn't match some of the basic requirements.

…n value to mocked validate method calls (ogizanagi) This PR was merged into the 3.4 branch. Discussion ---------- [Validator] ConstraintValidatorTestCase: add missing return value to mocked validate method calls | Q | A | ------------- | --- | Branch? | 3.4  | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Tickets | N/A  | License | MIT | Doc PR | N/A Spotted while working on #34456. Not sure it should really qualify as a bugfix, but the `ContextualValidatorInterface::validate` method is expected to return the instance. If [chaining in a validator](https://github.com/symfony/symfony/pull/34456/files#diff-0e6e3106aa637d750d47e86a14cef8d4R43), trying to use this test methods would throw an error, trying to call a method on `null`. Commits ------- 8d1f326 [Validator] ConstraintValidatorTestCase: add missing return value to mocked validate method calls

javiereguiluz · 2019-11-27T16:01:07Z

I like this idea a lot (and the PR explains it perfectly! thanks for that). Just as a very minor comment: given that we already use GroupSequence ... what if we use the same word ("sequence") for this too?

@Assert\Sequentially(...)  ->  @Assert\Sequence(...)

ogizanagi · 2019-11-27T16:23:55Z

@javiereguiluz : I hesitated to explain this point in the PR, but I preferred to avoid naming it Sequence as in the original proposition, because I think it's too much connotated as "validating a number sequence" or an array with sequence as indexes. It's also easiest to understand it'll validate the nested constraint sequentially. But I'm fine with Sequence if it's only me :)

src/Symfony/Component/Validator/Constraints/SequentiallyValidator.php

nicolas-grekas · 2020-02-07T13:33:23Z

rebase needed
is there any blocker?

oleg-andreyev

Already used in one of my projects so far no issue detected.

…aints

ogizanagi · 2020-02-07T16:17:41Z

Rebased
There is no blocker to me. Thanks for the ping

fabpot · 2020-02-08T07:14:37Z

Thank you @ogizanagi.

@var

…a set of constraints (ogizanagi) This PR was merged into the 5.1-dev branch. Discussion ---------- [Validator] Add a constraint to sequentially validate a set of constraints | Q | A | ------------- | --- | Branch? | master  | Bug fix? | no | New feature? | yes  | Deprecations? | no  | Tickets | N/A  | License | MIT | Doc PR | Todo Follows #20017 (comment) given some feedbacks about the suggested feature. ```php /** * @var string * * @Assert\Sequentially({ * @Assert\Type("string"), * @Assert\Length(min="4"), * @Assert\Regex("[a-z]"), * @SomeCustomConstraintWithHeavyExternalCalls(), * }) */ public $foo; ``` This new `Sequentially` constraint solves - with less power but better DX - some of the use-cases of the `GroupSequence` feature, allowing to interrupt the validation of some constraints if a previous one in the list failed before. Constraints are validated in given order, and the first violation raised will prevent other constraint validators to be executed. It can either prevent unexpected type exceptions thrown by further constraints or heavy & unnecessary calls to a database or external services if the value to validate already doesn't match some of the basic requirements. Commits ------- dfd9038 [Validator] Add a constraint to sequentially validate a set of constraints

courtney-miles · 2020-03-08T20:50:39Z

Sorry if this feedback is too late, but I think it would be helpful to have a non-sequential constraint also.

Using the original example of checking the type, prior to check the length and then the pattern, and finally using the value in a query -- it is technically acceptable to check the pattern despite having an incorrect length.

The following is to illustrate what I think should be ideally supported, by introducing a NonSequentially constraint which would allow Regex to be enforced even when Length fails.

/**
 * @var string
 *
 * @Assert\Sequentially({
 *     @Assert\Type("string"),
 *     @Assert\NonSequentially({
 *         @Assert\Length(min="4"),
 *         @Assert\Regex("[a-z]"),
 *     }),
 *     @SomeCustomConstraintWithHeavyExternalCalls(),
 * })
 */
public $foo;

Not that the NonSequentially constraint only makes sense when used in combination of the the Sequentially constraint.

I was looking to introduce a PR for a breakable constraint before discovering this one already existed. With that, I had in mind to name the constraints Interruptable and NonInterruptable (or some variation of.)

ogizanagi · 2020-03-08T21:15:46Z

@courtney-miles : That makes sense. With the Sequentially feature, I only intended to provide a way to easily fix situations in which trying to validate all the constraints despite of previous failures was causing a hard issue (getting an exception or causing a performance/resource usages penalty).
You're suggestion is legit to me, but is more about optimizing the feedbacks given to the user.
A current workaround would be to define your length+regex rule as a custom Compound constraint, so both would be validated:

/**
 * @var string
 *
 * @Assert\Sequentially({
 *     @Assert\Type("string"),
 *     @MyFooFormatRequirements(), <-- Length + Regex in a custom Compound constraint
 *     @SomeCustomConstraintWithHeavyExternalCalls(),
 * })
 */
public $foo;

If you think this NonSequentially constraint is worth it, I suggest you to open a PR for it :)

It'll basically work like the Compound one, allowing to provide the constraints in annotations directly instead of using inheritance+protected method.

ogizanagi added this to the next milestone Nov 19, 2019

carsonbot added Status: Needs Review Validator Feature labels Nov 19, 2019

This was referenced Nov 19, 2019

Collection validation does not stop after first violation #20017

Closed

[Validator] ConstraintValidatorTestCase: add missing return value to mocked validate method calls #34458

Merged

oleg-andreyev reviewed Dec 1, 2019

View reviewed changes

src/Symfony/Component/Validator/Constraints/SequentiallyValidator.php Outdated Show resolved Hide resolved

oleg-andreyev approved these changes Feb 7, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Feb 7, 2020

[Validator] Add a constraint to sequentially validate a set of constr…

dfd9038

…aints

nicolas-grekas approved these changes Feb 7, 2020

View reviewed changes

fabpot approved these changes Feb 8, 2020

View reviewed changes

fabpot mentioned this pull request Feb 8, 2020

[Validator] Add a constraint to sequentially validate a set of constrai… symfony/symfony-docs#13076

Closed

fabpot merged commit dfd9038 into symfony:master Feb 8, 2020

ogizanagi deleted the validator_sequence_constraint branch February 8, 2020 08:41

nicolas-grekas modified the milestones: next, 5.1 May 4, 2020

fabpot mentioned this pull request May 5, 2020

Release v5.1.0-BETA1 #36713

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Validator] Add a constraint to sequentially validate a set of constraints #34456

[Validator] Add a constraint to sequentially validate a set of constraints #34456

ogizanagi commented Nov 19, 2019 •

edited

Loading

javiereguiluz commented Nov 27, 2019

ogizanagi commented Nov 27, 2019

nicolas-grekas commented Feb 7, 2020

oleg-andreyev left a comment

ogizanagi commented Feb 7, 2020

fabpot commented Feb 8, 2020

courtney-miles commented Mar 8, 2020

ogizanagi commented Mar 8, 2020 •

edited

Loading

[Validator] Add a constraint to sequentially validate a set of constraints #34456

[Validator] Add a constraint to sequentially validate a set of constraints #34456

Conversation

ogizanagi commented Nov 19, 2019 • edited Loading

javiereguiluz commented Nov 27, 2019

ogizanagi commented Nov 27, 2019

nicolas-grekas commented Feb 7, 2020

oleg-andreyev left a comment

Choose a reason for hiding this comment

ogizanagi commented Feb 7, 2020

fabpot commented Feb 8, 2020

courtney-miles commented Mar 8, 2020

ogizanagi commented Mar 8, 2020 • edited Loading

ogizanagi commented Nov 19, 2019 •

edited

Loading

ogizanagi commented Mar 8, 2020 •

edited

Loading