-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Validator] Add #[ExtendsValidationFor] 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
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?
src/Symfony/Component/Validator/DependencyInjection/AttributeMetadataPass.php
Outdated
Show resolved
Hide resolved
66be469 to
8b38d5a
Compare
|
Thank you @alexandre-daubois and @stof, I addressed your comments. |
8b38d5a to
21cab23
Compare
21cab23 to
86c1cc6
Compare
#[ValidationFor] to declare new constraints for a class#[ExtendsValidationFor] to declare new constraints for a class
86c1cc6 to
12a9d93
Compare
alexandre-daubois
left a comment
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.
The feature is a bit puzzling to me (having a class that “lives” somewhat on its own and is never actually used in the code except by the container), but the use case is relevant so looks fine 👍
12a9d93 to
e56f537
Compare
…ct classes for resource definitions (nicolas-grekas) This PR was merged into the 7.4 branch. Discussion ---------- [DependencyInjection] Parse attributes found on abstract classes for resource definitions | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This PR improves support for resource definitions by allowing to parse attributes on abstract classes. This can be useful for PRs like #61563 and #61545, so that the attribute proposed there could be set on abstract classes. Commits ------- 6571f7b [DependencyInjection] Parse attributes found on abstract classes for resource definitions
e56f537 to
e884a76
Compare
|
We have the same issue when removing XML and Yaml for Validation we require a way that a Project can add new validations to third party classes of @sulu. We are fine with whatever alternative Symfony provides our CMS users, personally I would maybe go more with something like: class CustomUserValidationProvider
{
#[UserValidationProviderFor(User::class)]
public static function loadValidatorMetadata(ClassMetadata $metadata): void
{
$metadata->addPropertyConstraint('name', new NotBlank());
}
}Very similar how you could already have |
|
@alexander-schranz that would be a separate feature, which can totally be added in parallel if projects prefer using the PHP metadata API than adding attributes. |
|
@alexander-schranz thanks for chiming in and for the proposal. |
|
Thank you @nicolas-grekas. |
…re new serialization attributes for a class (nicolas-grekas) This PR was merged into the 7.4 branch. Discussion ---------- [Serializer] Add `#[ExtendsSerializationFor]` to declare new serialization attributes for a class | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This PR builds on #61532 It's a sibling of #61545 I propose to add a `#[ExtendsSerializationFor]` attribute that allows adding serialization attributes 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-serialization-of-api At the moment, the only way to achieve this is by declaring the new attributes in the (hardcoded) `config/serialization/` folder, using either xml or yaml. No attributes. With this PR, one will be able to define those extra serialization attributes 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 serialization attributes; target = the existing class to add serialization attributes to.) ```php #[ExtendsSerializationFor(TargetClass::class)] abstract class SourceClass { #[Groups(['my_app'])] #[SerializedName('fullName')] public string $name = ''; #[Groups(['my_app'])] public string $email = ''; #[Groups(['my_app'])] #[MaxDepth(2)] public Category $category; } ``` (I made the class abstract because it's not supposed to be instantiated - but it's not mandatory.) Here are the basics of how this works: 1. During container compilation, classes marked with `#[ExtendsSerializationFor(Target::class)]` are collected and validated: the container checks that members declared on the source exist on the target. If not, a `MappingException` is thrown. 2. The serializer is configured to map the target to its source classes. 3. At runtime, when loading serialization metadata for the target, attributes (groups, serialized names, max depth, etc.) are read from both the target and its mapped source classes and applied accordingly. Commits ------- 386f949 [Serializer] Add `#[ExtendsSerializationFor]` to declare new serialization attributes for a class
…hods private (xabbuh) This PR was merged into the 7.4 branch. Discussion ---------- [Serializer][Validator] make `doLoadClassMetadata()` methods private | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | | License | MIT I don't see a good reason to have them callable from the outside. This was probably a mistake in #61545/#61563. Commits ------- 9653488 make doLoadClassMetadata() methods private
|
Unfortunately this broke LiveComponents that have Validator constraints defined on properties. Explained in symfony/ux#3194. |
This PR builds on #61528
I propose to add a
#[ExtendsValidationFor]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.)
(I made the class abstract because it's not supposed to be instantiated - but it's not mandatory.)
Here are the basics of how this works:
#[ExtendsValidationFor(Target::class)]are collected and validated: the container checks that members declared on the source exist on the target. If not, aMappingExceptionis thrown.