From c716cf8a2d323b6d0e58eff1fcb430b123110f01 Mon Sep 17 00:00:00 2001 From: George Steel Date: Thu, 7 Jul 2022 00:08:11 +0100 Subject: [PATCH 1/3] Adds some test cases concerning validation groups and collections Signed-off-by: George Steel --- ...utFilterCollectionsValidationGroupTest.php | 200 ++++++++++++++++++ .../InputFilterStringValidationGroupTest.php | 78 +++++++ 2 files changed, 278 insertions(+) create mode 100644 test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php create mode 100644 test/ValidationGroup/InputFilterStringValidationGroupTest.php diff --git a/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php b/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php new file mode 100644 index 00000000..766bfac7 --- /dev/null +++ b/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php @@ -0,0 +1,200 @@ +setIsRequired(true); + + $first = new Input('first'); + $first->setRequired(true); + $second = new Input('second'); + $second->setRequired(true); + + $nestedFilter = new InputFilter(); + $nestedFilter->add($first); + $nestedFilter->add($second); + $collection->setInputFilter($nestedFilter); + + $this->inputFilter = new InputFilter(); + $this->inputFilter->add($collection, 'stuff'); + } + + /** @return array */ + public function collectionCountProvider(): array + { + return [ + 'Collection Count: None' => [null], + 'Collection Count: One' => [1], + 'Collection Count: Two' => [2], + 'Collection Count: Three' => [3], + 'Collection Count: Four' => [4], + ]; + } + + private function setCollectionCount(?int $count): void + { + if ($count === null) { + return; + } + + $collection = $this->inputFilter->get('stuff'); + self::assertInstanceOf(CollectionInputFilter::class, $collection); + $collection->setCount($count); + } + + /** @dataProvider collectionCountProvider */ + public function testIncompleteDataFailsValidation(?int $count): void + { + $this->setCollectionCount($count); + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo'], + ], + ]); + self::assertFalse($this->inputFilter->isValid()); + } + + /** @dataProvider collectionCountProvider */ + public function testCompleteDataPassesValidation(?int $count): void + { + $this->setCollectionCount($count); + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo', 'second' => 'Bar'], + ['first' => 'Foo', 'second' => 'Bar'], + ['first' => 'Foo', 'second' => 'Bar'], + ['first' => 'Foo', 'second' => 'Bar'], + ], + ]); + + self::assertTrue($this->inputFilter->isValid()); + } + + /** @dataProvider collectionCountProvider */ + public function testValidationFailsForCollectionItemValidity(?int $count): void + { + $this->setCollectionCount($count); + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo', 'second' => 'Bar'], + ['first' => '', 'second' => 'Bar'], + ['first' => 'Foo', 'second' => ''], + ['first' => '', 'second' => ''], + ], + ]); + + self::assertFalse($this->inputFilter->isValid()); + } + + /** @dataProvider collectionCountProvider */ + public function testValidationGroupWithCollectionInputFilter(?int $count): void + { + $this->setCollectionCount($count); + $collection = $this->inputFilter->get('stuff'); + self::assertInstanceOf(CollectionInputFilter::class, $collection); + $collection->getInputFilter()->setValidationGroup('first'); + + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ], + ]); + + self::assertTrue($this->inputFilter->isValid()); + } + + /** @dataProvider collectionCountProvider */ + public function testValidationGroupViaCollection(?int $count): void + { + $this->setCollectionCount($count); + $collection = $this->inputFilter->get('stuff'); + self::assertInstanceOf(CollectionInputFilter::class, $collection); + $collection->setValidationGroup([ + 0 => 'first', + 1 => 'second', + 2 => 'first', + 3 => 'first', + ]); + + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo'], + ['second' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ], + ]); + + self::assertTrue($this->inputFilter->isValid()); + } + + /** + * This test fails because of an undefined offset - the validation group must be set for elements 0 through 3 + * + * @dataProvider collectionCountProvider + */ + public function testValidationGroupViaCollectionMustSpecifyAllKeys(?int $count): void + { + $this->setCollectionCount($count); + $collection = $this->inputFilter->get('stuff'); + self::assertInstanceOf(CollectionInputFilter::class, $collection); + + $collection->setValidationGroup([ + 0 => 'first', + ]); + + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ], + ]); + + self::assertTrue($this->inputFilter->isValid()); + } + + /** @dataProvider collectionCountProvider */ + public function testValidationGroupViaTopLevelInputFilter(?int $count): void + { + $this->setCollectionCount($count); + $this->inputFilter->setValidationGroup([ + 'stuff' => [ + 0 => 'first', + 1 => 'second', + 2 => 'first', + 3 => 'first', + ], + ]); + + $this->inputFilter->setData([ + 'stuff' => [ + ['first' => 'Foo'], + ['second' => 'Foo'], + ['first' => 'Foo'], + ['first' => 'Foo'], + ], + ]); + + self::assertTrue($this->inputFilter->isValid()); + } +} diff --git a/test/ValidationGroup/InputFilterStringValidationGroupTest.php b/test/ValidationGroup/InputFilterStringValidationGroupTest.php new file mode 100644 index 00000000..406ce14d --- /dev/null +++ b/test/ValidationGroup/InputFilterStringValidationGroupTest.php @@ -0,0 +1,78 @@ +setRequired(true); + $first->getValidatorChain()->attach(new StringLength(['min' => 5])); + $second = new Input('second'); + $second->setRequired(true); + $second->getValidatorChain()->attach(new StringLength(['min' => 5])); + $third = new Input('third'); + $third->setRequired(true); + $third->getValidatorChain()->attach(new StringLength(['min' => 5])); + + $this->inputFilter = new InputFilter(); + $this->inputFilter->add($first); + $this->inputFilter->add($second); + $this->inputFilter->add($third); + } + + public function testValidationFailsForIncompleteInput(): void + { + $this->inputFilter->setData(['first' => 'Freddy']); + self::assertFalse($this->inputFilter->isValid()); + } + + public function testValidationSucceedsForCompleteInput(): void + { + $this->inputFilter->setData(['first' => 'Freddy', 'second' => 'Fruit Bat', 'third' => 'Muppet']); + self::assertTrue($this->inputFilter->isValid()); + } + + public function testValidationSucceedsForIncompleteInputWhenValidationGroupIsProvided(): void + { + $this->inputFilter->setValidationGroup('first'); + $this->inputFilter->setData(['first' => 'Freddy']); + + self::assertTrue($this->inputFilter->isValid()); + } + + public function testThatValidationGroupIsVariadic(): void + { + $this->inputFilter->setValidationGroup('first', 'second'); + $this->inputFilter->setData(['first' => 'Freddy', 'second' => 'Fruit Bat']); + + self::assertTrue($this->inputFilter->isValid()); + } + + public function testThatValidationGroupAcceptsListOfInputNames(): void + { + $this->inputFilter->setValidationGroup(['first', 'second']); + $this->inputFilter->setData(['first' => 'Freddy', 'second' => 'Fruit Bat']); + + self::assertTrue($this->inputFilter->isValid()); + } + + public function testValidationGroupWithUnknownInput(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('"doughnuts" was not found'); + $this->inputFilter->setValidationGroup(['doughnuts']); + } +} From 14e8d6ec12cda5c27422f629886e20bda4086c21 Mon Sep 17 00:00:00 2001 From: George Steel Date: Mon, 25 Jul 2022 12:55:44 +0100 Subject: [PATCH 2/3] Reverts the `is_string` check introduced in 2.16.0 for validation groups as it breaks existing behaviour for form collections. Signed-off-by: George Steel --- src/BaseInputFilter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BaseInputFilter.php b/src/BaseInputFilter.php index 7a0edf0c..d6364623 100644 --- a/src/BaseInputFilter.php +++ b/src/BaseInputFilter.php @@ -497,7 +497,7 @@ public function getMessages() protected function validateValidationGroup(array $inputs) { foreach ($inputs as $name) { - if (! is_string($name) || ! array_key_exists($name, $this->inputs)) { + if (! array_key_exists($name, $this->inputs)) { throw new Exception\InvalidArgumentException(sprintf( 'setValidationGroup() expects a list of valid input names; "%s" was not found', (string) $name From 73d7a191cc2de3083143bb83a736b2faba4aa859 Mon Sep 17 00:00:00 2001 From: George Steel Date: Mon, 25 Jul 2022 13:18:57 +0100 Subject: [PATCH 3/3] Alters test to expect the PHP notice as documentation of existing behaviour Signed-off-by: George Steel --- psalm-baseline.xml | 3 --- src/BaseInputFilter.php | 1 - ...nputFilterCollectionsValidationGroupTest.php | 17 +++++++++++++++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index a3a34b12..e3851f9e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -36,9 +36,6 @@ - - ! is_string($name) - $this->inputs diff --git a/src/BaseInputFilter.php b/src/BaseInputFilter.php index d6364623..924f8c06 100644 --- a/src/BaseInputFilter.php +++ b/src/BaseInputFilter.php @@ -21,7 +21,6 @@ use function is_array; use function is_int; use function is_object; -use function is_string; use function sprintf; class BaseInputFilter implements diff --git a/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php b/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php index 766bfac7..181d8094 100644 --- a/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php +++ b/test/ValidationGroup/InputFilterCollectionsValidationGroupTest.php @@ -9,6 +9,8 @@ use Laminas\InputFilter\InputFilter; use PHPUnit\Framework\TestCase; +use const PHP_VERSION_ID; + final class InputFilterCollectionsValidationGroupTest extends TestCase { private InputFilter $inputFilter; @@ -127,6 +129,7 @@ public function testValidationGroupViaCollection(?int $count): void $this->setCollectionCount($count); $collection = $this->inputFilter->get('stuff'); self::assertInstanceOf(CollectionInputFilter::class, $collection); + /** @psalm-suppress InvalidArgument */ $collection->setValidationGroup([ 0 => 'first', 1 => 'second', @@ -147,7 +150,7 @@ public function testValidationGroupViaCollection(?int $count): void } /** - * This test fails because of an undefined offset - the validation group must be set for elements 0 through 3 + * This test documents existing behaviour - the validation group must be set for elements 0 through 3 * * @dataProvider collectionCountProvider */ @@ -157,6 +160,7 @@ public function testValidationGroupViaCollectionMustSpecifyAllKeys(?int $count): $collection = $this->inputFilter->get('stuff'); self::assertInstanceOf(CollectionInputFilter::class, $collection); + /** @psalm-suppress InvalidArgument */ $collection->setValidationGroup([ 0 => 'first', ]); @@ -170,13 +174,22 @@ public function testValidationGroupViaCollectionMustSpecifyAllKeys(?int $count): ], ]); - self::assertTrue($this->inputFilter->isValid()); + if (PHP_VERSION_ID >= 80000) { + $this->expectWarning(); + $this->expectWarningMessage('Undefined array key 1'); + } else { + $this->expectNotice(); + $this->expectNoticeMessage('Undefined offset: 1'); + } + + $this->inputFilter->isValid(); } /** @dataProvider collectionCountProvider */ public function testValidationGroupViaTopLevelInputFilter(?int $count): void { $this->setCollectionCount($count); + /** @psalm-suppress InvalidArgument */ $this->inputFilter->setValidationGroup([ 'stuff' => [ 0 => 'first',