Skip to content

[Validator] Add support for automatically sequencing constraints #45716

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Validator] Add support for automatically sequencing constraints #45716

wants to merge 1 commit into from

Conversation

fancyweb
Copy link
Contributor

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

I realized that I sequence 99% of my constraints when I use the validator component because it's more performant, prevents unexpected exceptions and allows to have less violations at a time. I'd love an opt-in mode where all the constraints would be sequenced automatically, to avoid writing endless cascades of the Sequentially constraint. It's especially useful when using the Collection constraint on an array for example.

Before:

new Collection([
    'foo' => new Sequentially([
        new NotNull(),
        new Type('array'),
        new All([
            new Sequentially([
                new NotNull(),
                new Type('array'),
                new Collection([
                    'key' => new Sequentially([
                        new NotNull(),
                        new Type('string'),
                        new NotIdenticalTo(''),
                    ]),
                    'value' => new Sequentially([
                        new NotNull(),
                        new Type('string'),
                    ]),
                ]),
            ]),
        ]),
    ]),
]);

After:

new Collection([
    'foo' => [
        new NotNull(),
        new Type('array'),
        new All([
            new NotNull(),
            new Type('array'),
            new Collection([
                'key' => [
                    new NotNull(),
                    new Type('string'),
                    new NotIdenticalTo(''),
                ],
                'value' => [
                    new NotNull(),
                    new Type('string'),
                ]),
            ]),
        ]),
    ]),
]);

Decorating the metadata directly could work but it would require way more changes in the code.

@carsonbot carsonbot added this to the 6.1 milestone Mar 11, 2022
@fancyweb fancyweb force-pushed the validator/auto-sequence-constraints branch 2 times, most recently from df32c57 to 96baf9f Compare March 11, 2022 15:38
@carsonbot
Copy link

Hey!

I think @HeahDude has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@fancyweb fancyweb force-pushed the validator/auto-sequence-constraints branch from 96baf9f to 580eef6 Compare April 12, 2022 10:13
stof
stof reviewed Apr 12, 2022
View reviewed changes
src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php
foreach ($metadata->findConstraints($group) as $constraint) {
$constraints = $metadata->findConstraints($group);
if ($this->autoSequenceConstraints && 1 < \count($constraints)) {
$constraints = [new Sequentially($constraints)];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this invalidate the cache we have below due to having a new constraint each time ?

@fancyweb fancyweb modified the milestones: 6.1, 6.2 Apr 26, 2022
@fabpot
Copy link
Member

fabpot commented Aug 14, 2022

@fancyweb Are you still interested in moving this PR forward?

@fancyweb fancyweb closed this Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof left review comments

Assignees
No one assigned
Labels
Feature Status: Needs Review Validator
Projects
None yet
Milestone
6.2
Development

Successfully merging this pull request may close these issues.
4 participants
@fancyweb @carsonbot @fabpot @stof