From 306618f3277c3eba73cb52aeefa35d5ce45884aa Mon Sep 17 00:00:00 2001 From: matt Date: Mon, 4 Mar 2024 19:03:12 +0000 Subject: [PATCH] Add visitor for FileInput --- README.md | 66 +++-- psalm-baseline.xml | 11 + src/ConfigProvider.php | 19 +- .../AbstractInputVisitorFactory.php | 3 +- src/InputFilter/FileInputStyle.php | 12 + src/InputFilter/FileInputVisitor.php | 144 +++++++++++ src/InputFilter/FileInputVisitorFactory.php | 38 +++ src/Validator/FileValidatorVisitor.php | 24 +- .../FileInputVisitorFactoryTest.php | 68 +++++ test/InputFilter/FileInputVisitorTest.php | 232 ++++++++++++++++++ 10 files changed, 584 insertions(+), 33 deletions(-) create mode 100644 src/InputFilter/FileInputStyle.php create mode 100644 src/InputFilter/FileInputVisitor.php create mode 100644 src/InputFilter/FileInputVisitorFactory.php create mode 100644 test/InputFilter/FileInputVisitorFactoryTest.php create mode 100644 test/InputFilter/FileInputVisitorTest.php diff --git a/README.md b/README.md index 01b9ed7..00b9814 100644 --- a/README.md +++ b/README.md @@ -54,27 +54,6 @@ validators attached to the `InputFilter`? This command introspects your form's input filter and generates the most specific array shape possible. -## Beware - -If your code changes forms at run time - adding and removing elements, changing `required` properties or populating -`` options - this tool won't know. The array shape it generates can serve as a good starting point, but will +need manual tweaks. + +This tool aims to cover all the filters or validators installed when you `composer require laminas/laminas-form`. If it +encounters one it doesn't know about, it silently ignores it. See the [Customisation] section for pointers on handling +these. + +### `Cannot get type for 'foo'` + +It is possible to build an input filter with a combination of filters and validators that can never produce a result. +For instance, a `Boolean` filter with the `casting` option set to `true` will ony ever output a `bool` type. If you +follow that with a `Barcode` validator, the element can never validate. When the command encounters a situation like that +it will report a "Cannot get type" error. + +If you see this error when parsing an existing form that has been functioning fine for years, you've hit a bug. Please +raise an issue with a _small_ example form that reproduces the error. Or, better yet, create a PR with a failing +test :smiley: + ## Custom filters and validators To come, once things settle down... diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 9777f33..dc9eb1d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -86,6 +86,17 @@ + + + + + + + + + + + diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index 0c02ae5..ca498ca 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -24,6 +24,8 @@ use Kynx\Laminas\FormShape\Form\FormVisitorFactory; use Kynx\Laminas\FormShape\InputFilter\ArrayInputVisitor; use Kynx\Laminas\FormShape\InputFilter\ArrayInputVisitorFactory; +use Kynx\Laminas\FormShape\InputFilter\FileInputVisitor; +use Kynx\Laminas\FormShape\InputFilter\FileInputVisitorFactory; use Kynx\Laminas\FormShape\InputFilter\InputFilterVisitor; use Kynx\Laminas\FormShape\InputFilter\InputFilterVisitorFactory; use Kynx\Laminas\FormShape\InputFilter\InputVisitor; @@ -86,6 +88,7 @@ * input-visitors: InputVisitorArray, * filter?: array, * validator: array, + * input: array, * } * @psalm-type FormShapeConfigurationArray = array{ * laminas-cli: array, @@ -158,6 +161,7 @@ private function getLaminasFormShapeConfig(): array ], 'input-visitors' => [ ArrayInputVisitor::class, + FileInputVisitor::class, InputVisitor::class, ], 'filter' => [ @@ -204,6 +208,12 @@ private function getLaminasFormShapeConfig(): array ], ], ], + 'input' => [ + 'file' => [ + 'laminas' => true, + 'psr-7' => true, + ], + ], ]; } @@ -224,18 +234,19 @@ private function getDependencyConfig(): array AllowListVisitor::class => AllowListVisitorFactory::class, ArrayInputVisitor::class => ArrayInputVisitorFactory::class, ExplodeVisitor::class => ExplodeVisitorFactory::class, + FileInputVisitor::class => FileInputVisitorFactory::class, FileValidatorVisitor::class => FileValidatorVisitorFactory::class, + FileWriter::class => FileWriterFactory::class, FormLocator::class => FormLocatorFactory::class, FormProcessor::class => FormProcessorFactory::class, - NetteCodeGenerator::class => NetteCodeGeneratorFactory::class, - PrettyPrinter::class => PrettyPrinterFactory::class, - PsalmTypeCommand::class => PsalmTypeCommandFactory::class, - FileWriter::class => FileWriterFactory::class, FormVisitor::class => FormVisitorFactory::class, InArrayVisitor::class => InArrayVisitorFactory::class, InputFilterVisitor::class => InputFilterVisitorFactory::class, InputVisitor::class => InputVisitorFactory::class, + NetteCodeGenerator::class => NetteCodeGeneratorFactory::class, NonEmptyStringVisitor::class => NonEmptyStringVisitorFactory::class, + PrettyPrinter::class => PrettyPrinterFactory::class, + PsalmTypeCommand::class => PsalmTypeCommandFactory::class, RegexVisitor::class => RegexVisitorFactory::class, TypeNamer::class => TypeNamerFactory::class, ], diff --git a/src/InputFilter/AbstractInputVisitorFactory.php b/src/InputFilter/AbstractInputVisitorFactory.php index ca6ed95..9a15634 100644 --- a/src/InputFilter/AbstractInputVisitorFactory.php +++ b/src/InputFilter/AbstractInputVisitorFactory.php @@ -6,6 +6,7 @@ use Kynx\Laminas\FormShape\ConfigProvider; use Kynx\Laminas\FormShape\FilterVisitorInterface; +use Kynx\Laminas\FormShape\InputVisitorInterface; use Kynx\Laminas\FormShape\ValidatorVisitorInterface; use Psr\Container\ContainerInterface; @@ -14,7 +15,7 @@ */ abstract readonly class AbstractInputVisitorFactory { - abstract public function __invoke(ContainerInterface $container): AbstractInputVisitor; + abstract public function __invoke(ContainerInterface $container): InputVisitorInterface; /** * @return array diff --git a/src/InputFilter/FileInputStyle.php b/src/InputFilter/FileInputStyle.php new file mode 100644 index 0000000..6c55901 --- /dev/null +++ b/src/InputFilter/FileInputStyle.php @@ -0,0 +1,12 @@ + $filterVisitors + * @param array $validatorVisitors + */ + public function __construct( + private FileInputStyle $style, + private array $filterVisitors, + private array $validatorVisitors + ) { + } + + public function visit(InputInterface $input): ?Union + { + if (! $input instanceof FileInput) { + return null; + } + + $initial = new Union($this->getInitialTypes()); + $union = $initial->getBuilder()->freeze(); + + $validators = $this->prependUploadValidator($input, array_map( + static fn (array $queueItem): ValidatorInterface => $queueItem['instance'], + $input->getValidatorChain()->getValidators() + )); + + foreach ($validators as $validator) { + $union = $this->visitValidators($validator, $union); + } + + if (! $input->continueIfEmpty() && ($input->allowEmpty() || ! $input->isRequired())) { + $union = TypeUtil::widen($union, $initial); + } + + foreach ($input->getFilterChain()->getIterator() as $filter) { + if (is_callable($filter) && ! $filter instanceof FilterInterface) { + $filter = new Callback($filter); + } + + $union = $this->visitFilters($filter, $union); + } + + if ($input->hasFallback()) { + $union = Type::combineUnionTypes($union, TypeUtil::toStrictUnion($input->getFallbackValue())); + } + + if ($union->getAtomicTypes() === []) { + throw InputVisitorException::cannotGetInputType($input); + } + + return $union; + } + + /** + * @return non-empty-array + */ + private function getInitialTypes(): array + { + $types = [new TNull(), new TString()]; + if ($this->style === FileInputStyle::Laminas || $this->style === FileInputStyle::Both) { + $types[] = FileValidatorVisitor::getUploadArray(); + } + if ($this->style === FileInputStyle::Psr7 || $this->style === FileInputStyle::Both) { + $types[] = new TNamedObject(UploadedFileInterface::class); + } + + return $types; + } + + /** + * @param array $validators + * @return array + */ + private function prependUploadValidator(FileInput $input, array $validators): array + { + if (! $input->getAutoPrependUploadValidator()) { + return $validators; + } + + $hasUploadValidator = (bool) array_filter( + $validators, + static fn (ValidatorInterface $validator): bool => $validator instanceof UploadFile + ); + + if ($hasUploadValidator) { + return $validators; + } + + if (! $input->continueIfEmpty() && $input->isRequired() && ! $input->allowEmpty()) { + array_unshift($validators, new UploadFile()); + } + + return $validators; + } + + private function visitValidators(ValidatorInterface $validator, Union $union): Union + { + foreach ($this->validatorVisitors as $visitor) { + $union = $visitor->visit($validator, $union); + } + + return $union; + } + + private function visitFilters(FilterInterface $filter, Union $union): Union + { + foreach ($this->filterVisitors as $visitor) { + $union = $visitor->visit($filter, $union); + } + + return $union; + } +} diff --git a/src/InputFilter/FileInputVisitorFactory.php b/src/InputFilter/FileInputVisitorFactory.php new file mode 100644 index 0000000..5b5e9d4 --- /dev/null +++ b/src/InputFilter/FileInputVisitorFactory.php @@ -0,0 +1,38 @@ +get('config') ?? []; + /** @var array $inputConfig */ + $inputConfig = $config['laminas-form-shape']['input']['file'] ?? []; + $fileConfig = array_merge(['laminas' => true, 'psr-7' => true], $inputConfig); + + $style = FileInputStyle::Both; + if ($fileConfig['laminas'] && ! $fileConfig['psr-7']) { + $style = FileInputStyle::Laminas; + } elseif (! $fileConfig['laminas'] && $fileConfig['psr-7']) { + $style = FileInputStyle::Psr7; + } + + return new FileInputVisitor( + $style, + $this->getFilterVisitors($container), + $this->getValidatorVisitors($container), + ); + } +} diff --git a/src/Validator/FileValidatorVisitor.php b/src/Validator/FileValidatorVisitor.php index f3858a6..3a8de97 100644 --- a/src/Validator/FileValidatorVisitor.php +++ b/src/Validator/FileValidatorVisitor.php @@ -62,13 +62,23 @@ public function visit(ValidatorInterface $validator, Union $previous): Union return $previous; } - return TypeUtil::narrow($previous, new Union([ - new TKeyedArray([ - 'name' => new Union([new TString()]), - 'tmp_name' => new Union([new TNonEmptyString()]), - 'type' => new Union([new TString()]), - ]), + return TypeUtil::narrow($previous, self::getUploadUnion()); + } + + public static function getUploadUnion(): Union + { + return new Union([ + self::getUploadArray(), new TNamedObject(UploadedFileInterface::class), - ])); + ]); + } + + public static function getUploadArray(): TKeyedArray + { + return new TKeyedArray([ + 'name' => new Union([new TString()]), + 'tmp_name' => new Union([new TNonEmptyString()]), + 'type' => new Union([new TString()]), + ]); } } diff --git a/test/InputFilter/FileInputVisitorFactoryTest.php b/test/InputFilter/FileInputVisitorFactoryTest.php new file mode 100644 index 0000000..38cda71 --- /dev/null +++ b/test/InputFilter/FileInputVisitorFactoryTest.php @@ -0,0 +1,68 @@ + $expected + */ + #[DataProvider('configProvider')] + public function testInvokeConfiguresStyle(array $config, array $expected): void + { + $expected = new Union($expected); + $container = self::createStub(ContainerInterface::class); + $container->method('get') + ->willReturnMap([ + ['config', $this->getConfig($config)], + ]); + + $factory = new FileInputVisitorFactory(); + $instance = $factory($container); + + $actual = $instance->visit(new FileInput('foo')); + self::assertEquals($expected, $actual); + } + + public static function configProvider(): array + { + $laminas = FileValidatorVisitor::getUploadArray(); + $psr7 = new TNamedObject(UploadedFileInterface::class); + + return [ + 'laminas' => [['laminas' => true, 'psr-7' => false], [$laminas]], + 'psr-7' => [['laminas' => false, 'psr-7' => true], [$psr7]], + 'both' => [['laminas' => true, 'psr-7' => true], [$laminas, $psr7]], + ]; + } + + private function getConfig(array $config): array + { + return [ + 'laminas-form-shape' => [ + 'filter-visitors' => [], + 'validator-visitors' => [ + FileValidatorVisitor::class, + ], + 'input' => [ + 'file' => $config, + ], + ], + ]; + } +} diff --git a/test/InputFilter/FileInputVisitorTest.php b/test/InputFilter/FileInputVisitorTest.php new file mode 100644 index 0000000..f17e50f --- /dev/null +++ b/test/InputFilter/FileInputVisitorTest.php @@ -0,0 +1,232 @@ +visit($input); + self::assertNull($actual); + } + + /** + * @param non-empty-array $expected + */ + #[DataProvider('styleProvider')] + public function testVisitReturnsInitialUnion(FileInputStyle $style, array $expected): void + { + $expected = new Union($expected); + $input = new FileInput('foo'); + $visitor = new FileInputVisitor($style, [], []); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + public static function styleProvider(): array + { + return [ + 'laminas' => [ + FileInputStyle::Laminas, + [ + new TNull(), + new TString(), + FileValidatorVisitor::getUploadArray(), + ], + ], + 'psr-7' => [ + FileInputStyle::Psr7, + [ + new TNull(), + new TString(), + new TNamedObject(UploadedFileInterface::class), + ], + ], + 'both' => [ + FileInputStyle::Both, + [ + new TNull(), + new TString(), + FileValidatorVisitor::getUploadArray(), + new TNamedObject(UploadedFileInterface::class), + ], + ], + ]; + } + + public function testVisitDoesNotPrependUploadFileValidator(): void + { + $mockVisitor = self::createMock(ValidatorVisitorInterface::class); + $mockVisitor->expects(self::never()) + ->method('visit'); + $visitor = new FileInputVisitor(FileInputStyle::Both, [], [$mockVisitor]); + $input = new FileInput('foo'); + $input->setAutoPrependUploadValidator(false); + + $actual = $visitor->visit($input); + self::assertInstanceOf(Union::class, $actual); + } + + public function testVisitDoesNotPrependDuplicateUploadFileValidator(): void + { + $expected = new Union(self::getInitialTypes()); + $mockVisitor = self::createMock(ValidatorVisitorInterface::class); + $mockVisitor->method('visit') + ->willReturnCallback( + static function (ValidatorInterface $validator, Union $previous) use ($expected): Union { + self::assertEquals($expected, $previous); + return $previous; + } + ); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [], [$mockVisitor, new FileValidatorVisitor()]); + $input = new FileInput('foo'); + $input->getValidatorChain()->attach(new UploadFile()); + + $actual = $visitor->visit($input); + self::assertInstanceOf(Union::class, $actual); + } + + public function testVisitPrependsUploadFileValidator(): void + { + $expected = new Union([new TNamedObject(UploadedFileInterface::class)]); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [], [new FileValidatorVisitor()]); + $input = new FileInput('foo'); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + /** + * @param non-empty-array $expected + */ + #[DataProvider('widenTypeProvider')] + public function testVisitWidensType(bool $continueIfEmpty, bool $allowEmpty, bool $required, array $expected): void + { + $expected = new Union($expected); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [], [new FileValidatorVisitor()]); + $input = new FileInput('foo'); + $input->setContinueIfEmpty($continueIfEmpty); + $input->setAllowEmpty($allowEmpty); + $input->setRequired($required); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + public static function widenTypeProvider(): array + { + $validated = [new TNamedObject(UploadedFileInterface::class)]; + + return [ + "continue, allow, required" => [true, true, true, self::getInitialTypes()], + "continue, allow, not required" => [true, true, false, self::getInitialTypes()], + "continue, don't allow, required" => [true, false, true, self::getInitialTypes()], + "continue, don't allow, not required" => [true, false, false, self::getInitialTypes()], + "don't continue, allow, required" => [false, true, true, self::getInitialTypes()], + "don't continue, allow, not required" => [false, true, false, self::getInitialTypes()], + "don't continue, don't allow, required" => [false, false, true, $validated], + "don't continue, don't allow, not required" => [false, false, false, self::getInitialTypes()], + ]; + } + + public function testVisitConvertsCallableToCallbackFilter(): void + { + $expected = new Union([new TString()]); + $callable = static fn (): string => 'test'; + $input = new FileInput('foo'); + $input->getFilterChain()->attach($callable); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [new CallbackVisitor()], []); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + public function testVisitValidatesThenFilters(): void + { + $expected = new Union([new TString()]); + $filterVisitor = self::createMock(FilterVisitorInterface::class); + $filterVisitor->method('visit') + ->willReturnCallback(static function (FilterInterface $filter, Union $previous): Union { + $expected = new Union([new TNamedObject(UploadedFileInterface::class)]); + self::assertEquals($expected, $previous); + return new Union([new TString()]); + }); + $filter = new Callback(static fn (): string => 'test'); + $input = new FileInput('foo'); + $input->getFilterChain()->attach($filter); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [$filterVisitor], [new FileValidatorVisitor()]); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + public function testVisitAddsFallbackValue(): void + { + $expected = new Union([new TNamedObject(UploadedFileInterface::class), new TLiteralInt(123)]); + $input = new FileInput('foo'); + $input->setFallbackValue(123); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [], [new FileValidatorVisitor()]); + + $actual = $visitor->visit($input); + self::assertEquals($expected, $actual); + } + + public function testVisitEmptyUnionThrowsException(): void + { + $input = new FileInput('foo'); + $input->getValidatorChain()->attach(new StringLength()); + $visitor = new FileInputVisitor(FileInputStyle::Psr7, [], [ + new FileValidatorVisitor(), + new StringLengthVisitor(), + ]); + + self::expectException(InputVisitorException::class); + self::expectExceptionMessage("Cannot get type"); + $visitor->visit($input); + } + + /** + * @return non-empty-array + */ + private static function getInitialTypes(): array + { + return [ + new TNull(), + new TString(), + new TNamedObject(UploadedFileInterface::class), + ]; + } +}