Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Validator] Define which collection keys should be checked for uniqueness#42403
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedAug 6, 2021
It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you. Cheers! Carsonbot |
carsonbot commentedAug 7, 2021
Hey! I think@lmasforne has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
@wkania I just wanted to thank you for your great pull request description: you explained things well and showed some code examples, making the review much easier. Thanks! |
Would be very useful if this could work also with collection of DTOs, so I could provide property path where to look. |
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.
Here are some superficial comments, thanks for the PR. Please rebase also.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
You should check thenormalizer option. |
Thank you@wkania. |
…cked for uniqueness (wkania)This PR was squashed before being merged into the 6.1 branch.Discussion----------[Validator] Define which collection keys should be checked for uniqueness| Q | A| ------------- | ---| Branch? | 5.4| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |Fix#9888| License | MIT| Doc PR |symfony/symfony-docs#16713Currently, the validator checks each element of the collection as a whole. We already have a custom normalizer (which is great), but it would be nice to be able to check for uniqueness certain [collection](https://symfony.com/doc/current/reference/constraints/Collection.html) keys.For example, some fields in the collection element can be identifiers. They should be unique within the collection, even when the rest of the element data are different.Current state:- validates that all the elements of the given collection are uniqueNew state:- preserve the current state,- all old tests pass (no changes in them),- no breaking changes,- define which collection fields should be checked for uniqueness (optional),- fields are optional in each element of the collection. Use [collection constraints](https://symfony.com/doc/current/reference/constraints/Collection.html) if they are requiredExamples:1. Basic example. Each translation of the same resource must be in a different language.```phpuse Symfony\Component\Validator\Constraints as Assert;/** *@Assert\Count(min=1), *@Assert\Unique(fields={"language"}), *@Assert\Collection( * fields = { * "language" = { *@Assert\NotBlank, *@Assert\Length(min = 2, max = 2), *@Assert\Language * }, * "title" = { *@Assert\NotBlank, *@Assert\Length(max = 255) * }, * "description" = { *@Assert\NotBlank, *@Assert\Length(max = 255) * } * } * ) */public array $translations = [];```2. An example where Optional is recognizable. Items with the id are changed and without are new.```phpuse Symfony\Component\Validator\Constraints as Assert;use Symfony\Component\Validator\Constraints\Optional;/** *@Assert\Unique(fields={"id"}), *@Assert\Collection( * fields = { * "id" =@Assert\Optional({ *@Assert\Uuid * }), * "name" = { *@Assert\NotBlank, *@Assert\Length(max = 255) * } * } * ) */public array $items = [];```3. An example with composite uniqueness```phpuse Symfony\Component\Validator\Constraints as Assert;/** *@Assert\Unique(fields={"latitude", "longitude"}), *@Assert\Collection( * fields = { * "latitude" = { *@Assert\NotBlank * }, * "longitude" = { *@Assert\NotBlank * }, * "poi" = { *@Assert\Length(max = 255) * } * } * ) */public array $coordinates = [];```Commits-------0e8f4ce [Validator] Define which collection keys should be checked for uniqueness
…ys (Brajk19)This PR was squashed before being merged into the 6.4 branch.Discussion----------[Validator] UniqueValidator - normalize before reducing keys| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues || License | MITIn#42403 checking for uniqueness of certain collection keys was enabled. Method `UniqueValidator::reduceElementKeys` removes all keys which are not specified.Problem is that this happens before normalization, which in my opinion is not great because that method accepts array argument and if i have some object (DTO), TypeError will be thrown.Example:```phpclass ParentDTO{ /** * `@var` ChildDTO[] */ #[Assert\Unique( normalizer: [ChildDTO::class, 'normalize'] fields: 'id' )] public array $children;}``````phpclass ChildDTO{ public string $id; public string $name; public static function normalize(self $obj): array { return [ 'id' => $obj->id, 'name' => $obj->name ]; }}```Because normalization will happen after `reduceElementKeys` this will be thrown:`TypeError: Symfony\Component\Validator\Constraints\UniqueValidator::reduceElementKeys(): Argument#2 ($element) must be of type array, ...\ChildDTO given, called in .../UniqueValidator.php on line 48`If `$element = $normalizer($element);` is executed before `reduceElementKeys` it would enable using Assert\Unique with array of objects when correctly normalizedCommits-------77df90b [Validator] UniqueValidator - normalize before reducing keys
…ys (Brajk19)This PR was squashed before being merged into the 6.4 branch.Discussion----------[Validator] UniqueValidator - normalize before reducing keys| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues || License | MITInsymfony/symfony#42403 checking for uniqueness of certain collection keys was enabled. Method `UniqueValidator::reduceElementKeys` removes all keys which are not specified.Problem is that this happens before normalization, which in my opinion is not great because that method accepts array argument and if i have some object (DTO), TypeError will be thrown.Example:```phpclass ParentDTO{ /** * `@var` ChildDTO[] */ #[Assert\Unique( normalizer: [ChildDTO::class, 'normalize'] fields: 'id' )] public array $children;}``````phpclass ChildDTO{ public string $id; public string $name; public static function normalize(self $obj): array { return [ 'id' => $obj->id, 'name' => $obj->name ]; }}```Because normalization will happen after `reduceElementKeys` this will be thrown:`TypeError: Symfony\Component\Validator\Constraints\UniqueValidator::reduceElementKeys(): Argument#2 ($element) must be of type array, ...\ChildDTO given, called in .../UniqueValidator.php on line 48`If `$element = $normalizer($element);` is executed before `reduceElementKeys` it would enable using Assert\Unique with array of objects when correctly normalizedCommits-------77df90b959 [Validator] UniqueValidator - normalize before reducing keys
Uh oh!
There was an error while loading.Please reload this page.
Currently, the validator checks each element of the collection as a whole. We already have a custom normalizer (which is great), but it would be nice to be able to check for uniqueness certaincollection keys.
For example, some fields in the collection element can be identifiers. They should be unique within the collection, even when the rest of the element data are different.
Current state:
New state:
Examples: