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

[Console] Ease validating input options/arguments #21041

Closed

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Dec 24, 2016

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR todo if accepted

This allows validating input definition from Command::configure() using validators instead of doing it manually in execute().
InputArgument/InputOption would now have a setValidator(callable $validator).
Validators can throw an exception in case of invalid value or must return the value, modified or not (e.g. prefixed or converted to an object), quite similar to the input validation of the question helper.

Simple example which prefixes a given argument value
protected function configure() 
{
    $arg = new InputArgument('bar');
    $arg->setValidator(function ($v) { return 'prefix_'.$v; });
    $this->getDefinition()->addArgument($arg);
}

protected function execute($input, $output)
{
    $output->writeln($input->getArgument('bar'));
}
$ bin/console test baz
prefix_baz
Example using the validator component
use Symfony\Component\Validator\Validation;
use Symfony\Component\Validator\Constraints\Email;

protected function configure() 
{
    $validator = Validation::createValidator();
    $option = new InputOption('email');
    $option->setValidator(function ($email) use ($validator) {
        if (0 !== count($validator->validate($email, new Email()))) {
            throw new \InvalidArgumentException('The "email" option must be a valid email address.');
        }

        return $email;
    });
    $this->getDefinition()->addOption($option);
}
$ bin/console test --email wrong
[InvalidArgumentException] The "email" option must be a valid email address.

This is a simpler alternative to #20899 which proposes "form like" commands including support for annotations, param converters, validation constraints... that is a bit too much imho but could still be added later.

@maidmaid
Copy link
Contributor

Good idea, but usage should be easier, like this:

$option = new InputOption('email');
$option->addConstraint(new Email());

@chalasr
Copy link
Member Author

chalasr commented Dec 28, 2016

@maidmaid Thanks for your comment.

The point of having callable validators is flexibility. Here validators can come from anywhere:

$option = new InputOption('email');
$option->setValidator(['External\EmailValidator', 'validate']);

And can process the value after validation:

// canonicalize
$option = new InputOption('email');
$option->setValidator(['My\CanonicalEmailGuesser', 'guessCanonical']);

// convert the value to an object
$option = new InputOption('email');
$option->setValidator(function (string $email): User {
    // will throw an exception if the user cannot be found
    return $this->userRepository->getUserByEmail($email);
});

// dynamic default value (e.g. in addition of a static default value)
$option = new InputOption('uuid');
$option->setValidator(function ($uuid): \Ramsey\UuidInterface { 
    return \Ramsey\Uuid::isValid($uuid) ? \Ramsey\Uuid::fromString($uuid) : \Ramsey\Uuid::uuid4();
});

// basic type cast
$option = new InputArgument('amount');
$option->setValidator(function ($v): int { return (int) $v; });

// strict check
$option = new InputArgument('service');
$option->setValidator(function ($service) { 
    if ('service_container' === $service) {
        throw new \InvalidArgumentException();
    } 
});

Yet your proposal is sexy but so less flexible: It would make the feature dependent of the Symfony Validator, which involve that the Input class has the Validator as soft dependency and so cannot be used without it. Of course, we would also loose the processing part.

Mixing validation and processing might not look ideal but imho it doesn't hurt, it's consistent with the QuestionHelper input validation and the Config validation. I do think all that stuff should be done in configure() (excepted for some edge cases), so the execute() code deals with final values.

Edit:

All input options and arguments are now passed to validators, allowing to validate an option/argument based on the values of other options/arguments:

$argument = new InputArgument('foo');
$option = new InputOption('bar', null, InputOption::VALUE_OPTIONAL);

$option->setValidator(function ($value, $options, $arguments) { 
    return 'dummy' === $arguments['foo'] ? 'dummy_'.$value : $value; 
});

@ogizanagi
Copy link
Member

ogizanagi commented Dec 28, 2016

Totally agree with @chalasr. However a better integration with the validator component can be proposed after this one is merged IMHO. For instance in order to handle the constraint violation list in a nice output automatically or applying constraints on an option/argument, by providing a base callable making use of the validation component.
But that should be in another PR.

@tjaari
Copy link

tjaari commented Dec 29, 2016

Great 👍

@chalasr chalasr force-pushed the console/input-definition-validators branch from 5ecbb2c to 05086cf Compare January 15, 2017 13:20
@chalasr chalasr force-pushed the console/input-definition-validators branch from 05086cf to 390dabf Compare January 15, 2017 14:56
@chalasr
Copy link
Member Author

chalasr commented Apr 6, 2017

Actually, I have something else in mind.

@chalasr chalasr closed this Apr 6, 2017
@chalasr chalasr deleted the console/input-definition-validators branch April 6, 2017 19:51
@chalasr chalasr restored the console/input-definition-validators branch April 6, 2017 19:51
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.

6 participants