Skip to content

[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 12 commits into from

Conversation

wkania
Copy link
Contributor

@wkania wkania commented Aug 6, 2021

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #9888
License MIT
Doc PR symfony/symfony-docs#16713

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 certain collection 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 unique

New 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 if they are required

Examples:

  1. Basic example. Each translation of the same resource must be in a different language.
use 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 = [];
  1. An example where Optional is recognizable. Items with the id are changed and without are new.
use 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 = [];
  1. An example with composite uniqueness
use 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 = [];  

@carsonbot
Copy link

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
Copy link

Hey!

I think @lmasforne has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@chalasr chalasr added this to the 5.4 milestone Aug 9, 2021
@javiereguiluz
Copy link
Member

@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!

@fabpot fabpot modified the milestones: 5.4, 6.1 Nov 3, 2021
@norkunas
Copy link
Contributor

Would be very useful if this could work also with collection of DTOs, so I could provide property path where to look.

Copy link
Member

@nicolas-grekas nicolas-grekas left a 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.

@wkania
Copy link
Contributor Author

wkania commented Apr 3, 2022

Would be very useful if this could work also with collection of DTOs, so I could provide property path where to look.

You should check the normalizer option.
Example here.

@fabpot
Copy link
Member

fabpot commented Apr 14, 2022

Thank you @wkania.

fabpot added a commit that referenced this pull request Apr 14, 2022
…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#16713

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 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 unique

New 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 required

Examples:

1. Basic example. Each translation of the same resource must be in a different language.
```php
use 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.
```php
use 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
```php
use 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
@fabpot fabpot closed this Apr 14, 2022
@fabpot fabpot mentioned this pull request Apr 15, 2022
fabpot added a commit that referenced this pull request Mar 9, 2024
…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       | MIT

In #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:

```php
class ParentDTO
{
    /**
     * `@var` ChildDTO[]
     */
    #[Assert\Unique(
        normalizer: [ChildDTO::class, 'normalize']
        fields: 'id'
    )]
    public array $children;
}
```

```php
class 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 normalized

Commits
-------

77df90b [Validator] UniqueValidator - normalize before reducing keys
symfony-splitter pushed a commit to symfony/validator that referenced this pull request Mar 9, 2024
…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       | MIT

In symfony/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:

```php
class ParentDTO
{
    /**
     * `@var` ChildDTO[]
     */
    #[Assert\Unique(
        normalizer: [ChildDTO::class, 'normalize']
        fields: 'id'
    )]
    public array $children;
}
```

```php
class 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 normalized

Commits
-------

77df90b959 [Validator] UniqueValidator - normalize before reducing keys
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.

[Validator] Improve support for collection validation
8 participants