-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Validator] Add #[ValidationFor]
to declare new constraints for a class
#61545
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
base: 7.4
Are you sure you want to change the base?
Conversation
bbbe05c
to
66be469
Compare
The feature seems nice, but I'm still trying to grasp what it really does. If I understand correctly and if we take Sylius example, is the following code correct? use Sylius\Component\Product\Model\ProductTranslation;
#[ValidationFor(ProductTranslation::class)]
class MyProductTranslation
{
#[Assert\NotBlank(groups: ['my_app'])]
#[Assert\Length(min: 10, groups: ['my_app'])]
public string $name = '';
} Then on validation, Sylius' |
@alexandre-daubois you've got it right, provided you also configure the |
#[ValidationFor]
to declare new constraints for a class
Could we find a proper tribute name with something like |
@OskarStark that's not a service. |
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.
Does this work with class constraints? I don't think I saw support or tests for them. I wonder if they should be supported for feature completeness?
final class ValidationFor | ||
{ | ||
public function __construct( | ||
public string $class, |
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.
should be documented as class-string
$mappedClasses[$resolve($definition->getClass())] = true; | ||
$class = $resolve($definition->getClass()); | ||
foreach ($definition->getTag('validator.attribute_metadata') as $attributes) { | ||
if ($class !== $for = isset($attributes['for']) ? $resolve($attributes['for']) : $class) { |
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.
Do we really need to support DI parameters in the for
attribute ?
66be469
to
8b38d5a
Compare
Thank you @alexandre-daubois and @stof, I addressed your comments. |
8b38d5a
to
21cab23
Compare
This PR builds on #61528
I propose to add a
#[ValidationFor]
attribute that allows adding validation constraints to another class.This is typically needed for third party classes. For context, Sylius has a nice doc about this:
https://docs.sylius.com/the-customization-guide/customizing-validation
At the moment, the only way to achieve this is by declaring the new constraints in the (hardcoded)
config/validation/
folder, using either xml or yaml. No attributes.With this PR, one will be able to define those extra constraints using PHP attributes, set on classes that'd mirror the properties/getters of the targeted class. The compiler pass will ensure that all properties/getters declared in these source classes also exist in the target class. (source = the app's class that declares the new constraints; target = the existing class to add constraints to.)
Here are the basics of how this works:
#[ValidationFor(Target::class)]
are collected and validated: the container checks that members declared on the source exist on the target. If not, aMappingException
is thrown.