Skip to content
Permalink
Browse files

feature #31528 [Validator] Add a Length::$allowEmptyString option to …

…reject empty strings (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[Validator] Add a Length::$allowEmptyString option to reject empty strings

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | N/A   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | Todo (change the warning on top of https://symfony.com/doc/current/reference/constraints/Length.html)

which defaults to `true` in 4.4 but will trigger a deprecation if not set explicitly
in order to make the default `false` in 5.0.

While it could be solved now thanks to #29641 by using both `@Length(min=1)` & `@notblank(allowNull=true)` constraints,
as expressed in #27876 (comment) and following comments, the `@Length(min=1)` behavior doesn't match our expectations when reading it: it feels logical to invalidate empty strings, but it actually doesn't.
Hence the proposal of making the behavior of rejecting empty strings the default in 5.0.

In my opinion, the flag could even be removed later.

Commits
-------

e113e7f [Validator] Add a Length::$allowEmptyString option to reject empty strings
  • Loading branch information...
fabpot committed Jul 3, 2019
2 parents ea0656a + e113e7f commit 4e32643bdffc0f9f4fffaebb5272c9d8e0666863
@@ -107,3 +107,7 @@ Validator

* Deprecated passing an `ExpressionLanguage` instance as the second argument of `ExpressionValidator::__construct()`.
Pass it as the first argument instead.
* The `Length` constraint expects the `allowEmptyString` option to be defined
when the `min` option is used.
Set it to `true` to keep the current behavior and `false` to reject empty strings.
In 5.0, it'll become optional and will default to `false`.
@@ -2,6 +2,9 @@
namespace Symfony\Bridge\Doctrine\Tests\Fixtures;
use Symfony\Component\Validator\Constraints as Assert;
use Symfony\Component\Validator\Mapping\ClassMetadata;
/**
* Class BaseUser.
*/
@@ -46,4 +49,15 @@ public function getUsername()
{
return $this->username;
}
public static function loadValidatorMetadata(ClassMetadata $metadata): void
{
$allowEmptyString = property_exists(Assert\Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
$metadata->addPropertyConstraint('username', new Assert\Length([
'min' => 2,
'max' => 120,
'groups' => ['Registration'],
] + $allowEmptyString));
}
}
@@ -14,6 +14,7 @@
use Doctrine\ORM\Mapping as ORM;
use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity;
use Symfony\Component\Validator\Constraints as Assert;
use Symfony\Component\Validator\Mapping\ClassMetadata;
/**
* @ORM\Entity
@@ -36,13 +37,11 @@ class DoctrineLoaderEntity extends DoctrineLoaderParentEntity
/**
* @ORM\Column(length=20)
* @Assert\Length(min=5)
*/
public $mergedMaxLength;
/**
* @ORM\Column(length=20)
* @Assert\Length(min=1, max=10)
*/
public $alreadyMappedMaxLength;
@@ -69,4 +68,12 @@ class DoctrineLoaderEntity extends DoctrineLoaderParentEntity
/** @ORM\Column(type="simple_array", length=100) */
public $simpleArrayField = [];
public static function loadValidatorMetadata(ClassMetadata $metadata): void
{
$allowEmptyString = property_exists(Assert\Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
$metadata->addPropertyConstraint('mergedMaxLength', new Assert\Length(['min' => 5] + $allowEmptyString));
$metadata->addPropertyConstraint('alreadyMappedMaxLength', new Assert\Length(['min' => 1, 'max' => 10] + $allowEmptyString));
}
}
@@ -9,11 +9,6 @@
<constraint name="NotBlank">
<option name="groups">Registration</option>
</constraint>
<constraint name="Length">
<option name="min">2</option>
<option name="max">120</option>
<option name="groups">Registration</option>
</constraint>
</property>
</class>
</constraint-mapping>
@@ -40,6 +40,7 @@ public function testLoadClassMetadata()
}
$validator = Validation::createValidatorBuilder()
->addMethodMapping('loadValidatorMetadata')
->enableAnnotationMapping()
->addLoader(new DoctrineLoader(DoctrineTestHelper::createTestEntityManager(), '{^Symfony\\\\Bridge\\\\Doctrine\\\\Tests\\\\Fixtures\\\\DoctrineLoader}'))
->getValidator()
@@ -142,6 +143,7 @@ public function testFieldMappingsConfiguration()
}
$validator = Validation::createValidatorBuilder()
->addMethodMapping('loadValidatorMetadata')
->enableAnnotationMapping()
->addXmlMappings([__DIR__.'/../Resources/validator/BaseUser.xml'])
->addLoader(
@@ -57,13 +57,15 @@ public function testValidConstraint()
public function testGroupSequenceWithConstraintsOption()
{
$allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
$form = Forms::createFormFactoryBuilder()
->addExtension(new ValidatorExtension(Validation::createValidator()))
->getFormFactory()
->create(FormTypeTest::TESTED_TYPE, null, (['validation_groups' => new GroupSequence(['First', 'Second'])]))
->add('field', TextTypeTest::TESTED_TYPE, [
'constraints' => [
new Length(['min' => 10, 'groups' => ['First']]),
new Length(['min' => 10, 'groups' => ['First']] + $allowEmptyString),
new Email(['groups' => ['Second']]),
],
])
@@ -61,11 +61,13 @@ protected function setUp()
public function guessRequiredProvider()
{
$allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
return [
[new NotNull(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
[new NotBlank(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
[new IsTrue(), new ValueGuess(true, Guess::HIGH_CONFIDENCE)],
[new Length(10), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
[new Length(['min' => 10, 'max' => 10] + $allowEmptyString), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
[new Range(['min' => 1, 'max' => 20]), new ValueGuess(false, Guess::LOW_CONFIDENCE)],
];
}
@@ -101,7 +103,9 @@ public function testGuessMaxLengthForConstraintWithMaxValue()
public function testGuessMaxLengthForConstraintWithMinValue()
{
$constraint = new Length(['min' => '2']);
$allowEmptyString = property_exists(Length::class, 'allowEmptyString') ? ['allowEmptyString' => true] : [];
$constraint = new Length(['min' => '2'] + $allowEmptyString);
$result = $this->guesser->guessMaxLengthForConstraint($constraint);
$this->assertNull($result);
@@ -8,6 +8,7 @@ CHANGELOG
* added the `compared_value_path` parameter in violations when using any
comparison constraint with the `propertyPath` option.
* added support for checking an array of types in `TypeValidator`
* added a new `allowEmptyString` option to the `Length` constraint to allow rejecting empty strings when `min` is set, by setting it to `false`.

4.3.0
-----
@@ -41,6 +41,7 @@ class Length extends Constraint
public $min;
public $charset = 'UTF-8';
public $normalizer;
public $allowEmptyString;
public function __construct($options = null)
{
@@ -56,6 +57,13 @@ public function __construct($options = null)
parent::__construct($options);
if (null === $this->allowEmptyString) {
$this->allowEmptyString = true;
if (null !== $this->min) {
@trigger_error(sprintf('Using the "%s" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.', self::class), E_USER_DEPRECATED);
}
}
if (null === $this->min && null === $this->max) {
throw new MissingOptionsException(sprintf('Either option "min" or "max" must be given for constraint %s', __CLASS__), ['min', 'max']);
}
@@ -30,7 +30,7 @@ public function validate($value, Constraint $constraint)
throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Length');
}
if (null === $value || '' === $value) {
if (null === $value || ('' === $value && $constraint->allowEmptyString)) {
return;
}
@@ -21,7 +21,7 @@ class LengthTest extends TestCase
{
public function testNormalizerCanBeSet()
{
$length = new Length(['min' => 0, 'max' => 10, 'normalizer' => 'trim']);
$length = new Length(['min' => 0, 'max' => 10, 'normalizer' => 'trim', 'allowEmptyString' => false]);
$this->assertEquals('trim', $length->normalizer);
}
@@ -32,7 +32,7 @@ public function testNormalizerCanBeSet()
*/
public function testInvalidNormalizerThrowsException()
{
new Length(['min' => 0, 'max' => 10, 'normalizer' => 'Unknown Callable']);
new Length(['min' => 0, 'max' => 10, 'normalizer' => 'Unknown Callable', 'allowEmptyString' => false]);
}
/**
@@ -41,6 +41,6 @@ public function testInvalidNormalizerThrowsException()
*/
public function testInvalidNormalizerObjectThrowsException()
{
new Length(['min' => 0, 'max' => 10, 'normalizer' => new \stdClass()]);
new Length(['min' => 0, 'max' => 10, 'normalizer' => new \stdClass(), 'allowEmptyString' => false]);
}
}
@@ -22,26 +22,47 @@ protected function createValidator()
return new LengthValidator();
}
public function testNullIsValid()
public function testLegacyNullIsValid()
{
$this->validator->validate(null, new Length(6));
$this->validator->validate(null, new Length(['value' => 6, 'allowEmptyString' => false]));
$this->assertNoViolation();
}
public function testEmptyStringIsValid()
/**
* @group legacy
* @expectedDeprecation Using the "Symfony\Component\Validator\Constraints\Length" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.
*/
public function testLegacyEmptyStringIsValid()
{
$this->validator->validate('', new Length(6));
$this->assertNoViolation();
}
public function testEmptyStringIsInvalid()
{
$this->validator->validate('', new Length([
'value' => $limit = 6,
'allowEmptyString' => false,
'exactMessage' => 'myMessage',
]));
$this->buildViolation('myMessage')
->setParameter('{{ value }}', '""')
->setParameter('{{ limit }}', $limit)
->setInvalidValue('')
->setPlural($limit)
->setCode(Length::TOO_SHORT_ERROR)
->assertRaised();
}
/**
* @expectedException \Symfony\Component\Validator\Exception\UnexpectedValueException
*/
public function testExpectsStringCompatibleType()
{
$this->validator->validate(new \stdClass(), new Length(5));
$this->validator->validate(new \stdClass(), new Length(['value' => 5, 'allowEmptyString' => false]));
}
public function getThreeOrLessCharacters()
@@ -109,7 +130,7 @@ public function getThreeCharactersWithWhitespaces()
*/
public function testValidValuesMin($value)
{
$constraint = new Length(['min' => 5]);
$constraint = new Length(['min' => 5, 'allowEmptyString' => false]);
$this->validator->validate($value, $constraint);
$this->assertNoViolation();
@@ -131,7 +152,7 @@ public function testValidValuesMax($value)
*/
public function testValidValuesExact($value)
{
$constraint = new Length(4);
$constraint = new Length(['value' => 4, 'allowEmptyString' => false]);
$this->validator->validate($value, $constraint);
$this->assertNoViolation();
@@ -142,7 +163,7 @@ public function testValidValuesExact($value)
*/
public function testValidNormalizedValues($value)
{
$constraint = new Length(['min' => 3, 'max' => 3, 'normalizer' => 'trim']);
$constraint = new Length(['min' => 3, 'max' => 3, 'normalizer' => 'trim', 'allowEmptyString' => false]);
$this->validator->validate($value, $constraint);
$this->assertNoViolation();
@@ -156,6 +177,7 @@ public function testInvalidValuesMin($value)
$constraint = new Length([
'min' => 4,
'minMessage' => 'myMessage',
'allowEmptyString' => false,
]);
$this->validator->validate($value, $constraint);
@@ -199,6 +221,7 @@ public function testInvalidValuesExactLessThanFour($value)
'min' => 4,
'max' => 4,
'exactMessage' => 'myMessage',
'allowEmptyString' => false,
]);
$this->validator->validate($value, $constraint);
@@ -221,6 +244,7 @@ public function testInvalidValuesExactMoreThanFour($value)
'min' => 4,
'max' => 4,
'exactMessage' => 'myMessage',
'allowEmptyString' => false,
]);
$this->validator->validate($value, $constraint);
@@ -244,6 +268,7 @@ public function testOneCharset($value, $charset, $isValid)
'max' => 1,
'charset' => $charset,
'charsetMessage' => 'myMessage',
'allowEmptyString' => false,
]);
$this->validator->validate($value, $constraint);
@@ -262,15 +287,15 @@ public function testOneCharset($value, $charset, $isValid)
public function testConstraintDefaultOption()
{
$constraint = new Length(5);
$constraint = new Length(['value' => 5, 'allowEmptyString' => false]);
$this->assertEquals(5, $constraint->min);
$this->assertEquals(5, $constraint->max);
}
public function testConstraintAnnotationDefaultOption()
{
$constraint = new Length(['value' => 5, 'exactMessage' => 'message']);
$constraint = new Length(['value' => 5, 'exactMessage' => 'message', 'allowEmptyString' => false]);
$this->assertEquals(5, $constraint->min);
$this->assertEquals(5, $constraint->max);
@@ -103,7 +103,7 @@ public function testRelationBetweenChildAAndChildB()
public function testCollectionConstraintValidateAllGroupsForNestedConstraints()
{
$this->metadata->addPropertyConstraint('data', new Collection(['fields' => [
'one' => [new NotBlank(['groups' => 'one']), new Length(['min' => 2, 'groups' => 'two'])],
'one' => [new NotBlank(['groups' => 'one']), new Length(['min' => 2, 'groups' => 'two', 'allowEmptyString' => false])],
'two' => [new NotBlank(['groups' => 'two'])],
]]));
@@ -121,16 +121,17 @@ public function testAllConstraintValidateAllGroupsForNestedConstraints()
{
$this->metadata->addPropertyConstraint('data', new All(['constraints' => [
new NotBlank(['groups' => 'one']),
new Length(['min' => 2, 'groups' => 'two']),
new Length(['min' => 2, 'groups' => 'two', 'allowEmptyString' => false]),
]]));
$entity = new Entity();
$entity->data = ['one' => 't', 'two' => ''];
$violations = $this->validator->validate($entity, null, ['one', 'two']);
$this->assertCount(2, $violations);
$this->assertCount(3, $violations);
$this->assertInstanceOf(NotBlank::class, $violations->get(0)->getConstraint());
$this->assertInstanceOf(Length::class, $violations->get(1)->getConstraint());
$this->assertInstanceOf(Length::class, $violations->get(2)->getConstraint());
}
}

0 comments on commit 4e32643

Please sign in to comment.
You can’t perform that action at this time.