Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[Validator] Add constraint on unique elements collection(Assert\Unique) #26555

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 25, 2019
Merged

[Validator] Add constraint on unique elements collection(Assert\Unique) #26555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc66683
Add UniqueCollection constraint and validator
Mar 15, 2018
d0eb13e
Rebase and update to latest CS
nicolas-grekas Mar 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Rebase and update to latest CS
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Mar 13, 2019
commit d0eb13e55a02375f6652715567080a6f5bdc0cce
1 change: 1 addition & 0 deletions 1 src/Symfony/Component/Validator/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* added UATP cards support to `CardSchemeValidator`
* added option `allowNull` to NotBlank constraint
* added `Json` constraint
* added `Unique` constraint

4.2.0
-----
Expand Down
6 changes: 3 additions & 3 deletions 6 src/Symfony/Component/Validator/Constraints/Unique.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
*/
class Unique extends Constraint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UniqueCollection?

{
const IS_NOT_UNIQUE = '7911c98d-b845-4da0-94b7-a8dac36bc55a';
public const IS_NOT_UNIQUE = '7911c98d-b845-4da0-94b7-a8dac36bc55a';

protected static $errorNames = array(
protected static $errorNames = [
self::IS_NOT_UNIQUE => 'IS_NOT_UNIQUE',
);
];

public $message = 'This collection should contain only unique elements';
}
10 changes: 5 additions & 5 deletions 10 src/Symfony/Component/Validator/Constraints/UniqueValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Exception\UnexpectedValueException;

/**
* @author Yevgeniy Zholkevskiy <zhenya.zholkevskiy@gmail.com>
Expand All @@ -33,13 +33,13 @@ public function validate($value, Constraint $constraint)
return;
}

if (!is_array($value) && !$value instanceof \IteratorAggregate) {
throw new UnexpectedTypeException($value, 'IteratorAggregate');
if (!\is_array($value) && !$value instanceof \IteratorAggregate) {
throw new UnexpectedValueException($value, 'array|IteratorAggregate');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not iterable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see review chain

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too until recently. Apparently this is considered too unsafe, see #24577 (comment) and #25506

?

was it a blocker for this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can pass a broken/unrewindbable iterator as aggregate just as easy. But i cant pass a working iterator/iterable directly. That's poor DX IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consequencies of this validator being applied on non-rewindable iterators are much bigger than in #25506 so yes (generally, after validator validates the data, data is processed)

}

$collectionElements = array();
$collectionElements = [];
foreach ($value as $element) {
if (in_array($element, $collectionElements, true)) {
if (\in_array($element, $collectionElements, true)) {
$this->context->buildViolation($constraint->message)
->setParameter('{{ value }}', $this->formatValue($value))
->setCode(Unique::IS_NOT_UNIQUE)
Expand Down
48 changes: 24 additions & 24 deletions 48 src/Symfony/Component/Validator/Tests/Constraints/UniqueValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected function createValidator()
}

/**
* @expectedException \Symfony\Component\Validator\Exception\UnexpectedTypeException
* @expectedException \Symfony\Component\Validator\Exception\UnexpectedValueException
*/
public function testExpectsUniqueConstraintCompatibleType()
{
Expand All @@ -42,29 +42,29 @@ public function testValidValues($value)

public function getValidValues()
{
return array(
yield 'null' => array(array(null)),
yield 'empty array' => array(array()),
yield 'single integer' => array(array(5)),
yield 'single string' => array(array('a')),
yield 'single object' => array(array(new \stdClass())),
yield 'unique booleans' => array(array(true, false)),
yield 'unique integers' => array(array(1, 2, 3, 4, 5, 6)),
yield 'unique floats' => array(array(0.1, 0.2, 0.3)),
yield 'unique strings' => array(array('a', 'b', 'c')),
yield 'unique arrays' => array(array(array(1, 2), array(2, 4), array(4, 6))),
yield 'unique objects' => array(array(new \stdClass(), new \stdClass())),
);
return [
yield 'null' => [[null]],
yield 'empty array' => [[]],
yield 'single integer' => [[5]],
yield 'single string' => [['a']],
yield 'single object' => [[new \stdClass()]],
yield 'unique booleans' => [[true, false]],
yield 'unique integers' => [[1, 2, 3, 4, 5, 6]],
yield 'unique floats' => [[0.1, 0.2, 0.3]],
yield 'unique strings' => [['a', 'b', 'c']],
yield 'unique arrays' => [[[1, 2], [2, 4], [4, 6]]],
yield 'unique objects' => [[new \stdClass(), new \stdClass()]],
];
}

/**
* @dataProvider getInvalidValues
*/
public function testInvalidValues($value)
{
$constraint = new Unique(array(
$constraint = new Unique([
'message' => 'myMessage',
));
]);
$this->validator->validate($value, $constraint);

$this->buildViolation('myMessage')
Expand All @@ -77,13 +77,13 @@ public function getInvalidValues()
{
$object = new \stdClass();

return array(
yield 'not unique booleans' => array(array(true, true)),
yield 'not unique integers' => array(array(1, 2, 3, 3)),
yield 'not unique floats' => array(array(0.1, 0.2, 0.1)),
yield 'not unique string' => array(array('a', 'b', 'a')),
yield 'not unique arrays' => array(array(array(1, 1), array(2, 3), array(1, 1))),
yield 'not unique objects' => array(array($object, $object)),
);
return [
yield 'not unique booleans' => [[true, true]],
yield 'not unique integers' => [[1, 2, 3, 3]],
yield 'not unique floats' => [[0.1, 0.2, 0.1]],
yield 'not unique string' => [['a', 'b', 'a']],
yield 'not unique arrays' => [[[1, 1], [2, 3], [1, 1]]],
yield 'not unique objects' => [[$object, $object]],
];
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.