-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] [WIP] Validate controller arguments using constraints #47425
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
Closed
Closed
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
465d9aa
Added the \Attribute::TARGET_PARAMETER constant to almost all constra…
artyuum 55ecfec
Added ConstraintAttributeListener
artyuum 3de1141
Implemented EventSubscriberInterface in ConstraintAttributeListener
artyuum da6c368
Fixed wrong service id in validator.php for ConstraintAttributeListener
artyuum f5c1252
Updated CHANGELOG.md
artyuum f01d56b
Revert "Updated CHANGELOG.md"
artyuum 6df2a74
Revert "Added the \Attribute::TARGET_PARAMETER constant to almost all…
artyuum 225b58e
Added ControllerArgument constraint
artyuum caf3d31
Now using the new ControllerArgument constraint in ConstraintAttribut…
artyuum c6924f8
Renamed ConstraintAttributeListener to ControllerArgumentConstraintAt…
artyuum 34ed8a9
Updated service id to match listener class name
artyuum dff2f5b
Removed unneeded foreach()
artyuum daecd8d
Merge branch 'symfony:6.2' into ft/validate-controller-arguments
artyuum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
src/Symfony/Component/Validator/Constraints/ControllerArgument.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Validator\Constraints; | ||
|
||
/** | ||
* @author Dynèsh Hassanaly <artyum@protonmail.com> | ||
*/ | ||
#[\Attribute(\Attribute::TARGET_PARAMETER)] | ||
class ControllerArgument extends Composite | ||
{ | ||
public $constraints = []; | ||
|
||
public function __construct(mixed $constraints = null, array $groups = null, mixed $payload = null) | ||
{ | ||
parent::__construct($constraints ?? [], $groups, $payload); | ||
} | ||
|
||
public function getDefaultOption(): ?string | ||
{ | ||
return 'constraints'; | ||
} | ||
|
||
public function getRequiredOptions(): array | ||
{ | ||
return ['constraints']; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
protected function getCompositeOption(): string | ||
{ | ||
return 'constraints'; | ||
} | ||
} |
44 changes: 44 additions & 0 deletions
44
src/Symfony/Component/Validator/Constraints/ControllerArgumentValidator.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Validator\Constraints; | ||
|
||
use Symfony\Component\Validator\Constraint; | ||
use Symfony\Component\Validator\ConstraintValidator; | ||
use Symfony\Component\Validator\Exception\UnexpectedTypeException; | ||
use Symfony\Component\Validator\Exception\UnexpectedValueException; | ||
|
||
/** | ||
* @author Dynèsh Hassanaly <artyum@protonmail.com> | ||
*/ | ||
class ControllerArgumentValidator extends ConstraintValidator | ||
{ | ||
/** | ||
* @inheritDoc | ||
*/ | ||
public function validate(mixed $value, Constraint $constraint) | ||
{ | ||
if (!$constraint instanceof ControllerArgument) { | ||
throw new UnexpectedTypeException($constraint, ControllerArgument::class); | ||
} | ||
|
||
if (null === $value) { | ||
return; | ||
} | ||
|
||
$context = $this->context; | ||
$validator = $context->getValidator()->inContext($context); | ||
|
||
foreach ($constraint->constraints as $c) { | ||
$validator->validate($value, $c); | ||
} | ||
} | ||
} |
76 changes: 76 additions & 0 deletions
76
...mfony/Component/Validator/EventListener/ControllerArgumentConstraintAttributeListener.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Validator\EventListener; | ||
|
||
use Symfony\Component\EventDispatcher\EventSubscriberInterface; | ||
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; | ||
use Symfony\Component\HttpKernel\KernelEvents; | ||
use Symfony\Component\Validator\Constraint; | ||
use Symfony\Component\Validator\Constraints\ControllerArgument; | ||
use Symfony\Component\Validator\Exception\ValidationFailedException; | ||
use Symfony\Component\Validator\Validator\ValidatorInterface; | ||
|
||
/** | ||
* Handles the validator constraint attributes on controller's arguments. | ||
* | ||
* @author Dynèsh Hassanaly <artyum@protonmail.com> | ||
*/ | ||
class ControllerArgumentConstraintAttributeListener implements EventSubscriberInterface | ||
{ | ||
public function __construct(private readonly ValidatorInterface $validator) {} | ||
|
||
public function onKernelControllerArguments(ControllerArgumentsEvent $event): void | ||
{ | ||
$controller = $event->getController(); | ||
$arguments = $event->getArguments(); | ||
$reflectionMethod = $this->getReflectionMethod($controller); | ||
|
||
foreach ($reflectionMethod->getParameters() as $index => $reflectionParameter) { | ||
$reflectionAttributes = $reflectionParameter->getAttributes(ControllerArgument::class); | ||
|
||
if (!$reflectionAttributes) { | ||
continue; | ||
} | ||
|
||
// this attribute cannot be repeated, so we will always one item in this array | ||
$reflectionAttribute = $reflectionAttributes[0]; | ||
|
||
/** @var Constraint $constraint */ | ||
$constraint = $reflectionAttribute->newInstance(); | ||
$value = $arguments[$index]; | ||
$violations = $this->validator->validate($value, $constraint); | ||
|
||
if ($violations->count() > 0) { | ||
throw new ValidationFailedException($value, $violations); | ||
} | ||
} | ||
} | ||
|
||
public static function getSubscribedEvents(): array | ||
{ | ||
return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 10]]; | ||
} | ||
|
||
private function getReflectionMethod(callable $controller): \ReflectionMethod | ||
{ | ||
if (is_array($controller)) { | ||
$class = $controller[0]; | ||
$method = $controller[1]; | ||
} else { | ||
/** @var object $controller */ | ||
$class = $controller; | ||
$method = '__invoke'; | ||
} | ||
|
||
return new \ReflectionMethod($class, $method); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how I'm extracting the controller class + method from the event.
If you have a better way I'll take it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the new
ControllerEvent::getControllerReflector()
methodThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method seems to be available only in
ControllerEvent
and I'm listening onControllerArgumentsEvent
😕There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's worth it, it could be easy to expose:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be useful, yes. Should I create another PR to make that change and merge it into the current PR or can I change that method scope in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @nicolas-grekas