Skip to content

Commit d331e99

Browse files
fabpotfabpot
committed
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 symfony/symfony#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 ------- e113e7f812 [Validator] Add a Length::$allowEmptyString option to reject empty strings
2 parents d5f4afb + 7e7d3ed commit d331e99

File tree

6 files changed

+51
-16
lines changed

6 files changed

+51
-16
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* added the `compared_value_path` parameter in violations when using any
99
comparison constraint with the `propertyPath` option.
1010
* added support for checking an array of types in `TypeValidator`
11+
* added a new `allowEmptyString` option to the `Length` constraint to allow rejecting empty strings when `min` is set, by setting it to `false`.
1112

1213
4.3.0
1314
-----

‎Constraints/Length.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class Length extends Constraint
4141
public $min;
4242
public $charset = 'UTF-8';
4343
public $normalizer;
44+
public $allowEmptyString;
4445

4546
public function __construct($options = null)
4647
{
@@ -56,6 +57,13 @@ public function __construct($options = null)
5657

5758
parent::__construct($options);
5859

60+
if (null === $this->allowEmptyString) {
61+
$this->allowEmptyString = true;
62+
if (null !== $this->min) {
63+
@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);
64+
}
65+
}
66+
5967
if (null === $this->min && null === $this->max) {
6068
throw new MissingOptionsException(sprintf('Either option "min" or "max" must be given for constraint %s', __CLASS__), ['min', 'max']);
6169
}

‎Constraints/LengthValidator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function validate($value, Constraint $constraint)
3030
throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Length');
3131
}
3232

33-
if (null === $value || '' === $value) {
33+
if (null === $value || ('' === $value && $constraint->allowEmptyString)) {
3434
return;
3535
}
3636

‎Tests/Constraints/LengthTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class LengthTest extends TestCase
2121
{
2222
public function testNormalizerCanBeSet()
2323
{
24-
$length = new Length(['min' => 0, 'max' => 10, 'normalizer' => 'trim']);
24+
$length = new Length(['min' => 0, 'max' => 10, 'normalizer' => 'trim', 'allowEmptyString' => false]);
2525

2626
$this->assertEquals('trim', $length->normalizer);
2727
}
@@ -32,7 +32,7 @@ public function testNormalizerCanBeSet()
3232
*/
3333
public function testInvalidNormalizerThrowsException()
3434
{
35-
new Length(['min' => 0, 'max' => 10, 'normalizer' => 'Unknown Callable']);
35+
new Length(['min' => 0, 'max' => 10, 'normalizer' => 'Unknown Callable', 'allowEmptyString' => false]);
3636
}
3737

3838
/**
@@ -41,6 +41,6 @@ public function testInvalidNormalizerThrowsException()
4141
*/
4242
public function testInvalidNormalizerObjectThrowsException()
4343
{
44-
new Length(['min' => 0, 'max' => 10, 'normalizer' => new \stdClass()]);
44+
new Length(['min' => 0, 'max' => 10, 'normalizer' => new \stdClass(), 'allowEmptyString' => false]);
4545
}
4646
}

‎Tests/Constraints/LengthValidatorTest.php

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,26 +22,47 @@ protected function createValidator()
2222
return new LengthValidator();
2323
}
2424

25-
public function testNullIsValid()
25+
public function testLegacyNullIsValid()
2626
{
27-
$this->validator->validate(null, new Length(6));
27+
$this->validator->validate(null, new Length(['value' => 6, 'allowEmptyString' => false]));
2828

2929
$this->assertNoViolation();
3030
}
3131

32-
public function testEmptyStringIsValid()
32+
/**
33+
* @group legacy
34+
* @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.
35+
*/
36+
public function testLegacyEmptyStringIsValid()
3337
{
3438
$this->validator->validate('', new Length(6));
3539

3640
$this->assertNoViolation();
3741
}
3842

43+
public function testEmptyStringIsInvalid()
44+
{
45+
$this->validator->validate('', new Length([
46+
'value' => $limit = 6,
47+
'allowEmptyString' => false,
48+
'exactMessage' => 'myMessage',
49+
]));
50+
51+
$this->buildViolation('myMessage')
52+
->setParameter('{{ value }}', '""')
53+
->setParameter('{{ limit }}', $limit)
54+
->setInvalidValue('')
55+
->setPlural($limit)
56+
->setCode(Length::TOO_SHORT_ERROR)
57+
->assertRaised();
58+
}
59+
3960
/**
4061
* @expectedException \Symfony\Component\Validator\Exception\UnexpectedValueException
4162
*/
4263
public function testExpectsStringCompatibleType()
4364
{
44-
$this->validator->validate(new \stdClass(), new Length(5));
65+
$this->validator->validate(new \stdClass(), new Length(['value' => 5, 'allowEmptyString' => false]));
4566
}
4667

4768
public function getThreeOrLessCharacters()
@@ -109,7 +130,7 @@ public function getThreeCharactersWithWhitespaces()
109130
*/
110131
public function testValidValuesMin($value)
111132
{
112-
$constraint = new Length(['min' => 5]);
133+
$constraint = new Length(['min' => 5, 'allowEmptyString' => false]);
113134
$this->validator->validate($value, $constraint);
114135

115136
$this->assertNoViolation();
@@ -131,7 +152,7 @@ public function testValidValuesMax($value)
131152
*/
132153
public function testValidValuesExact($value)
133154
{
134-
$constraint = new Length(4);
155+
$constraint = new Length(['value' => 4, 'allowEmptyString' => false]);
135156
$this->validator->validate($value, $constraint);
136157

137158
$this->assertNoViolation();
@@ -142,7 +163,7 @@ public function testValidValuesExact($value)
142163
*/
143164
public function testValidNormalizedValues($value)
144165
{
145-
$constraint = new Length(['min' => 3, 'max' => 3, 'normalizer' => 'trim']);
166+
$constraint = new Length(['min' => 3, 'max' => 3, 'normalizer' => 'trim', 'allowEmptyString' => false]);
146167
$this->validator->validate($value, $constraint);
147168

148169
$this->assertNoViolation();
@@ -156,6 +177,7 @@ public function testInvalidValuesMin($value)
156177
$constraint = new Length([
157178
'min' => 4,
158179
'minMessage' => 'myMessage',
180+
'allowEmptyString' => false,
159181
]);
160182

161183
$this->validator->validate($value, $constraint);
@@ -199,6 +221,7 @@ public function testInvalidValuesExactLessThanFour($value)
199221
'min' => 4,
200222
'max' => 4,
201223
'exactMessage' => 'myMessage',
224+
'allowEmptyString' => false,
202225
]);
203226

204227
$this->validator->validate($value, $constraint);
@@ -221,6 +244,7 @@ public function testInvalidValuesExactMoreThanFour($value)
221244
'min' => 4,
222245
'max' => 4,
223246
'exactMessage' => 'myMessage',
247+
'allowEmptyString' => false,
224248
]);
225249

226250
$this->validator->validate($value, $constraint);
@@ -244,6 +268,7 @@ public function testOneCharset($value, $charset, $isValid)
244268
'max' => 1,
245269
'charset' => $charset,
246270
'charsetMessage' => 'myMessage',
271+
'allowEmptyString' => false,
247272
]);
248273

249274
$this->validator->validate($value, $constraint);
@@ -262,15 +287,15 @@ public function testOneCharset($value, $charset, $isValid)
262287

263288
public function testConstraintDefaultOption()
264289
{
265-
$constraint = new Length(5);
290+
$constraint = new Length(['value' => 5, 'allowEmptyString' => false]);
266291

267292
$this->assertEquals(5, $constraint->min);
268293
$this->assertEquals(5, $constraint->max);
269294
}
270295

271296
public function testConstraintAnnotationDefaultOption()
272297
{
273-
$constraint = new Length(['value' => 5, 'exactMessage' => 'message']);
298+
$constraint = new Length(['value' => 5, 'exactMessage' => 'message', 'allowEmptyString' => false]);
274299

275300
$this->assertEquals(5, $constraint->min);
276301
$this->assertEquals(5, $constraint->max);

‎Tests/Validator/RecursiveValidatorTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public function testRelationBetweenChildAAndChildB()
103103
public function testCollectionConstraintValidateAllGroupsForNestedConstraints()
104104
{
105105
$this->metadata->addPropertyConstraint('data', new Collection(['fields' => [
106-
'one' => [new NotBlank(['groups' => 'one']), new Length(['min' => 2, 'groups' => 'two'])],
106+
'one' => [new NotBlank(['groups' => 'one']), new Length(['min' => 2, 'groups' => 'two', 'allowEmptyString' => false])],
107107
'two' => [new NotBlank(['groups' => 'two'])],
108108
]]));
109109

@@ -121,16 +121,17 @@ public function testAllConstraintValidateAllGroupsForNestedConstraints()
121121
{
122122
$this->metadata->addPropertyConstraint('data', new All(['constraints' => [
123123
new NotBlank(['groups' => 'one']),
124-
new Length(['min' => 2, 'groups' => 'two']),
124+
new Length(['min' => 2, 'groups' => 'two', 'allowEmptyString' => false]),
125125
]]));
126126

127127
$entity = new Entity();
128128
$entity->data = ['one' => 't', 'two' => ''];
129129

130130
$violations = $this->validator->validate($entity, null, ['one', 'two']);
131131

132-
$this->assertCount(2, $violations);
132+
$this->assertCount(3, $violations);
133133
$this->assertInstanceOf(NotBlank::class, $violations->get(0)->getConstraint());
134134
$this->assertInstanceOf(Length::class, $violations->get(1)->getConstraint());
135+
$this->assertInstanceOf(Length::class, $violations->get(2)->getConstraint());
135136
}
136137
}

0 commit comments

Comments
 (0)