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

[Validator] add Validation::createCallable() #31466

Open
wants to merge 2 commits into
base: 4.4
from

Conversation

@janvernieuwe
Copy link

janvernieuwe commented May 10, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

This is an initial PR to check/discuss the implementation of a callable validator.
If there is interest in merging this, I will gladly update the docs and such.

The use case is mainly for validation of console questions, since the default validation has been removed in the latest version and i could not find an easy solution to replace it (if there already is some solution for this, i'm not aware of it) and the question helper uses callables.
This small class allows the standard symfony validators to be used in console questions, or any other location that requires a callable validator.

Example use case:

$io = new SymfonyStyle($input, $output);
$required = new CallableValidator([new NotBlank()]);
$wsdl = $io->ask('Wsdl location (URL or path to file)', null, $required);

As said before, this is by no means the final version, but I would like to know if there is interest in merging this (and receive some feedback about the implementation) before I put any more effort into this.

Copy link
Contributor

Simperfit left a comment

You should remove the two unrelated commits

@janvernieuwe janvernieuwe requested review from dunglas, lyrixx and sroze as code owners May 13, 2019
@janvernieuwe janvernieuwe force-pushed the janvernieuwe:callable-validator branch from df4a4a0 to 1edd0a0 May 13, 2019
@janvernieuwe janvernieuwe force-pushed the janvernieuwe:callable-validator branch from 1edd0a0 to 94d121f May 13, 2019
@janvernieuwe janvernieuwe changed the base branch from master to 4.3 May 13, 2019
@janvernieuwe

This comment has been minimized.

Copy link
Author

janvernieuwe commented May 13, 2019

Updated it, guess my fork got out of sync

@ro0NL

This comment has been minimized.

Copy link
Contributor

ro0NL commented Jun 4, 2019

instead of a new CallableValidator, which isn't really a validator from the component pov (as it's not a true ValidatorInterface implementation) ... what about Validation::createCallable($constraints): Closure

@lyrixx lyrixx changed the title Callable validator [Console] Callable validator Jun 4, 2019
@lyrixx lyrixx changed the base branch from 4.3 to 4.4 Jun 4, 2019
@janvernieuwe

This comment has been minimized.

Copy link
Author

janvernieuwe commented Jun 7, 2019

@ro0NL that can work too, it should be pretty easy as something like this in the Validation class:

    public static function createCallable(Constraint...$constraints): callable
    {
        $validator = self::createValidator();

        return static function ($value) use ($constraints, $validator) {
            $violations = $validator->validate($value, $constraints);
            if (0 !== $violations->count()) {
                throw new LogicException((string)$violations);
            }

            return $value;
        };
    }

Don't know if it is desirable to create a new Validator instance for each new callable though, but it shouldn't be too bad.

@janvernieuwe janvernieuwe force-pushed the janvernieuwe:callable-validator branch 2 times, most recently from 8b0bb22 to 6fe4e80 Jul 26, 2019
@janvernieuwe janvernieuwe force-pushed the janvernieuwe:callable-validator branch from 6fe4e80 to 015b0c8 Jul 26, 2019
@janvernieuwe

This comment has been minimized.

Copy link
Author

janvernieuwe commented Jul 26, 2019

Failures on travis don't seem to be related to any of my code changes as far as i can see...

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
@nicolas-grekas nicolas-grekas changed the title [Console] Callable validator [Validator] add Validation::createCallable() Nov 5, 2019
/**
* This class cannot be instantiated.
*/
private function __construct()

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 5, 2019

Member

this should be moved back where is was

/**
* Create a callable chain of constraints.
*
* @param Constraint ...$constraints

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 5, 2019

Member

should be removed

*
* @param Constraint ...$constraints
*
* @return callable

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 5, 2019

Member

should be removed

}
/**
* Create a callable chain of constraints.

This comment has been minimized.

Copy link
@nicolas-grekas
return static function ($value) use ($constraints, $validator) {
$violations = $validator->validate($value, $constraints);
if (0 !== $violations->count()) {
throw new LogicException((string) $violations);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 5, 2019

Member

This should be a RuntimeException to me, or better: a class derived from RuntimeException modelled after ValidationFailedException from the Messenger component. WDYTN

@nicolas-grekas nicolas-grekas modified the milestones: 4.4, next Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.