8000 [RFC][OptionsResolver] Constraints · Issue #36820 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[RFC][OptionsResolver] Constraints #36820

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
zajca opened this issue May 15, 2020 · 10 comments · Fixed by #40240
Closed

[RFC][OptionsResolver] Constraints #36820

zajca opened this issue May 15, 2020 · 10 comments · Fixed by #40240
Labels
Feature OptionsResolver

Comments

@zajca
Copy link
zajca commented May 15, 2020

Description
OptionConfigurator would have new method checkConstraints(Constraint[] $constraints). User can set validation constraints that will be checked by OptionResolver.
Similar like form fields can have constraints.
In OptionsResolver would be new method addConstraints(string $option, Constraint[] $constraints) and resolve() would be created Validator a validate all options.

Example

$resolver = new OptionsResolver();
$resolver->define('email')
    ->required()
    ->allowedTypes('string')
    ->checkConstraints([
        new Assert\NotBlank(),
        new Assert\Email(),
    ]);

$resolver->resolve(['email' => 'invalidEmail']);
// resolve would throw InvalidOptionsException with ConstraintViolationListInterface
@yceruto
Copy link
Member
yceruto commented May 15, 2020 •
edited
Loading

I'm 👎 on this proposal. This would coupling two components unnecessarily IMHO. Instead you can check these constraints in setAllowedValues() method in project side directly.

@ro0NL
Copy link
Contributor
ro0NL commented May 19, 2020

the validator is a service, optionsresolver is not. This makes coupling the components a bit harder as well. I suggest to leverage #31466 in userland.

@wouterj
Copy link
Member
wouterj commented May 19, 2020

I'm not sure if we need such constraints in the options resolver, but would it make sense to allow a custom validation callable? This would work nice with the introduction of Validation::createCallable().

@stof
Copy link
Member
stof commented May 19, 2020

setAllowedValues already allows to use a custom callback to decide whether the value is allowed.

@zajca
Copy link
Author
zajca commented May 19, 2020

I did not counted with setAllowedValues that is feasible, but it's such a pain (callable wouldn't make it much better).

        $resolver->setAllowedValues('name', static function ($value){
            $validator = Validation::createValidator();
            $violations = $validator->validate($value, [
                new Assert\Length(['min' => 10])
            ]);

            return 0 === count($violations);
        });

Actually better is to create OptionsResolver, resolve parameters and than create constraints and validate.

@wouterj
Copy link
Member
wouterj commented May 19, 2020

Callable does if I'm correct:

$resolver->setAllowedValues('name', Validation::createCallable([
    new Assert\Length(['min' => 10 ]),
]));

@jvasseur
Copy link
Contributor

@wouterj I don't think it works because Validation::createCallable creates a callback that throws on invalid input while setAllowedValues expect a callback that returns a boolean that indicate if the input is valid or not.

@ro0NL
Copy link
Contributor
ro0NL commented May 23, 2020

then converting the validation exception to a bool value out-of-the-box, sounds nice :)

@carsonbot
Copy link

Thank you for this suggestion.
There has not been a lot of activity here for a while. Would you still like to see this feature?

@wouterj wouterj mentioned this issue Feb 18, 2021
Merged
@wouterj
Copy link
Member
wouterj commented Feb 18, 2021

@carsonbot alright, here you have some activity: #40240 😉

@carsonbot carsonbot removed the Stalled label Feb 18, 2021
@fabpot fabpot closed this as completed Mar 12, 2021
fabpot added a commit that referenced this issue Mar 12, 2021
@fabpot
feature #40240 [Validator] Add Validation::createIsValidCallable() th…
6c0102c 
…at returns a boolean instead of exception (wouterj)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Validator] Add Validation::createIsValidCallable() that returns a boolean instead of exception

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #36820
| License       | MIT
| Doc PR        | tbd

This adds a `Validator::createValidCallable()` (I'm very open for other name suggestions) that returns a boolean instead of exceptions. This allows usingit in places where booleans are expected, for instance in the referenced OptionsResolver case:

```php
$resolver->setAllowedValues('name', Validation::createValidCallable(
    new Assert\Length(['min' => 10 ])
));
```

Commits
-------

e731f5f [Validator] Add createValidCallable() that returns a boolean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature OptionsResolver
Projects
None yet
Milestone
No milestone
9 participants
@fabpot @javiereguiluz @zajca @stof @wouterj @jvasseur @ro0NL @yceruto @carsonbot
0