From 3a9c0c152c5156e11169e7e414a632a80e9e9ab1 Mon Sep 17 00:00:00 2001 From: spiralbot Date: Mon, 14 Aug 2023 15:06:17 +0000 Subject: [PATCH] Merge pull request #961 from msmakouz/feature/filters-improvement --- composer.json | 3 +- src/Attribute/Setter.php | 11 +++- src/Exception/SetterException.php | 13 ++++ src/Model/FilterProvider.php | 6 +- .../Interceptor/ValidateFilterInterceptor.php | 6 +- src/Model/Mapper/CasterInterface.php | 14 +++++ src/Model/Mapper/CasterRegistry.php | 36 +++++++++++ src/Model/Mapper/CasterRegistryInterface.php | 17 +++++ src/Model/Mapper/DefaultCaster.php | 20 ++++++ src/Model/Mapper/EnumCaster.php | 30 +++++++++ src/Model/Mapper/Mapper.php | 36 +++++++++++ src/Model/Mapper/UuidCaster.php | 46 ++++++++++++++ src/Model/Schema/AttributeMapper.php | 47 +++++++++++--- src/Model/Schema/InputMapper.php | 12 +++- tests/Fixtures/Status.php | 11 ++++ tests/Fixtures/UserFilter.php | 21 +++++++ tests/Mapper/CasterRegistryTest.php | 30 +++++++++ tests/Mapper/DefaultCasterTest.php | 27 ++++++++ tests/Mapper/EnumCasterTest.php | 38 ++++++++++++ tests/Mapper/UuidCasterTest.php | 37 +++++++++++ tests/Model/FilterProviderTest.php | 62 ++++++++++++++++--- tests/Model/Schema/AttributeMapperTest.php | 7 ++- 22 files changed, 501 insertions(+), 29 deletions(-) create mode 100644 src/Exception/SetterException.php create mode 100644 src/Model/Mapper/CasterInterface.php create mode 100644 src/Model/Mapper/CasterRegistry.php create mode 100644 src/Model/Mapper/CasterRegistryInterface.php create mode 100644 src/Model/Mapper/DefaultCaster.php create mode 100644 src/Model/Mapper/EnumCaster.php create mode 100644 src/Model/Mapper/Mapper.php create mode 100644 src/Model/Mapper/UuidCaster.php create mode 100644 tests/Fixtures/Status.php create mode 100644 tests/Fixtures/UserFilter.php create mode 100644 tests/Mapper/CasterRegistryTest.php create mode 100644 tests/Mapper/DefaultCasterTest.php create mode 100644 tests/Mapper/EnumCasterTest.php create mode 100644 tests/Mapper/UuidCasterTest.php diff --git a/composer.json b/composer.json index 6fb90b7..590ce2f 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,8 @@ "mockery/mockery": "^1.5", "nyholm/psr7": "^1.5", "phpunit/phpunit": "^10.1", - "vimeo/psalm": "^5.9" + "vimeo/psalm": "^5.9", + "ramsey/uuid": "^4.7" }, "autoload": { "psr-4": { diff --git a/src/Attribute/Setter.php b/src/Attribute/Setter.php index 72fcc93..7fcad5c 100644 --- a/src/Attribute/Setter.php +++ b/src/Attribute/Setter.php @@ -6,6 +6,7 @@ use Attribute; use Spiral\Attributes\NamedArgumentConstructor; +use Spiral\Filters\Exception\SetterException; /** * Use setters to typecast the incoming value before passing it to the property. @@ -17,10 +18,10 @@ * #[\Spiral\Filters\Attribute\Setter(filter: [Foo::class, 'bar'])] */ #[Attribute(Attribute::TARGET_PROPERTY | Attribute::IS_REPEATABLE), NamedArgumentConstructor] -final class Setter +class Setter { public readonly \Closure $filter; - private array $args; + protected array $args; public function __construct(callable $filter, mixed ...$args) { @@ -30,6 +31,10 @@ public function __construct(callable $filter, mixed ...$args) public function updateValue(mixed $value): mixed { - return ($this->filter)($value, ...$this->args); + try { + return ($this->filter)($value, ...$this->args); + } catch (\Throwable $e) { + throw new SetterException($e); + } } } diff --git a/src/Exception/SetterException.php b/src/Exception/SetterException.php new file mode 100644 index 0000000..a151665 --- /dev/null +++ b/src/Exception/SetterException.php @@ -0,0 +1,13 @@ +createFilterInstance($name); - [$mappingSchema, $errors, $setters] = $attributeMapper->map($filter, $input); + [$mappingSchema, $errors, $setters, $optionalFilters] = $attributeMapper->map($filter, $input); if ($filter instanceof HasFilterDefinition) { $mappingSchema = \array_merge( @@ -49,6 +48,9 @@ public function createFilter(string $name, InputInterface $input): FilterInterfa \assert($schemaBuilder instanceof Builder); $schema = $schemaBuilder->makeSchema($name, $mappingSchema); + foreach ($optionalFilters as $optionalFilter) { + $schema[$optionalFilter][Builder::SCHEMA_OPTIONAL] = true; + } [$data, $inputErrors] = $inputMapper->map($schema, $input, $setters); $errors = \array_merge($errors, $inputErrors); diff --git a/src/Model/Interceptor/ValidateFilterInterceptor.php b/src/Model/Interceptor/ValidateFilterInterceptor.php index a01fd4d..4f724bc 100644 --- a/src/Model/Interceptor/ValidateFilterInterceptor.php +++ b/src/Model/Interceptor/ValidateFilterInterceptor.php @@ -5,7 +5,6 @@ namespace Spiral\Filters\Model\Interceptor; use Psr\Container\ContainerInterface; -use Spiral\Core\Container; use Spiral\Core\CoreInterceptorInterface; use Spiral\Core\CoreInterface; use Spiral\Filters\Model\FilterBag; @@ -44,6 +43,11 @@ public function process(string $controller, string $action, array $parameters, C ); } + if (($bag->errors ?? []) !== []) { + $errorMapper = new ErrorMapper($bag->schema); + throw new ValidationException($errorMapper->mapErrors($bag->errors), $parameters['context'] ?? null); + } + return $filter; } diff --git a/src/Model/Mapper/CasterInterface.php b/src/Model/Mapper/CasterInterface.php new file mode 100644 index 0000000..6df987a --- /dev/null +++ b/src/Model/Mapper/CasterInterface.php @@ -0,0 +1,14 @@ + */ + private array $casters = []; + + public function __construct(array $casters = []) + { + foreach ($casters as $caster) { + $this->register($caster); + } + } + + public function register(CasterInterface $caster): void + { + $this->casters[$caster::class] = $caster; + } + + /** + * @return array + */ + public function getCasters(): array + { + return \array_values($this->casters); + } + + public function getDefault(): CasterInterface + { + return new DefaultCaster(); + } +} diff --git a/src/Model/Mapper/CasterRegistryInterface.php b/src/Model/Mapper/CasterRegistryInterface.php new file mode 100644 index 0000000..256b610 --- /dev/null +++ b/src/Model/Mapper/CasterRegistryInterface.php @@ -0,0 +1,17 @@ + + */ + public function getCasters(): array; + + public function getDefault(): CasterInterface; +} diff --git a/src/Model/Mapper/DefaultCaster.php b/src/Model/Mapper/DefaultCaster.php new file mode 100644 index 0000000..154154e --- /dev/null +++ b/src/Model/Mapper/DefaultCaster.php @@ -0,0 +1,20 @@ +setValue($filter, $value); + } +} diff --git a/src/Model/Mapper/EnumCaster.php b/src/Model/Mapper/EnumCaster.php new file mode 100644 index 0000000..c712069 --- /dev/null +++ b/src/Model/Mapper/EnumCaster.php @@ -0,0 +1,30 @@ +getName()); + } + + public function setValue(FilterInterface $filter, \ReflectionProperty $property, mixed $value): void + { + /** + * @var \ReflectionNamedType $type + */ + $type = $property->getType(); + + /** + * @var class-string<\BackedEnum> $enum + */ + $enum = $type->getName(); + + $property->setValue($filter, $enum::from($value)); + } +} diff --git a/src/Model/Mapper/Mapper.php b/src/Model/Mapper/Mapper.php new file mode 100644 index 0000000..2c01b94 --- /dev/null +++ b/src/Model/Mapper/Mapper.php @@ -0,0 +1,36 @@ +getType(); + if ($type instanceof \ReflectionNamedType && !$type->isBuiltin()) { + foreach ($this->registry->getCasters() as $setter) { + if ($setter->supports($type)) { + $setter->setValue($filter, $property, $value); + return; + } + } + } + + $this->registry->getDefault()->setValue($filter, $property, $value); + } +} diff --git a/src/Model/Mapper/UuidCaster.php b/src/Model/Mapper/UuidCaster.php new file mode 100644 index 0000000..d3ac3fe --- /dev/null +++ b/src/Model/Mapper/UuidCaster.php @@ -0,0 +1,46 @@ +interfaceExists === null) { + $this->interfaceExists = \interface_exists(UuidInterface::class); + } + + return $this->interfaceExists && $this->implements($type->getName(), UuidInterface::class); + } + + public function setValue(FilterInterface $filter, \ReflectionProperty $property, mixed $value): void + { + $property->setValue($filter, \Ramsey\Uuid\Uuid::fromString($value)); + } + + private function implements(string $haystack, string $interface): bool + { + if ($haystack === $interface) { + return true; + } + + foreach ((array)\class_implements($haystack) as $implements) { + if ($implements === $interface) { + return true; + } + + if (self::implements($implements, $interface)) { + return true; + } + } + + return false; + } +} diff --git a/src/Model/Schema/AttributeMapper.php b/src/Model/Schema/AttributeMapper.php index 2469081..cafeda3 100644 --- a/src/Model/Schema/AttributeMapper.php +++ b/src/Model/Schema/AttributeMapper.php @@ -9,10 +9,12 @@ use Spiral\Filters\Attribute\NestedArray; use Spiral\Filters\Attribute\NestedFilter; use Spiral\Filters\Attribute\Setter; +use Spiral\Filters\Exception\SetterException; use Spiral\Filters\Model\FilterInterface; use Spiral\Filters\Model\FilterProviderInterface; use Spiral\Filters\Exception\ValidationException; use Spiral\Filters\InputInterface; +use Spiral\Filters\Model\Mapper\Mapper; /** * @internal @@ -21,27 +23,31 @@ final class AttributeMapper { public function __construct( private readonly FilterProviderInterface $provider, - private readonly ReaderInterface $reader + private readonly ReaderInterface $reader, + private readonly Mapper $mapper ) { } /** - * Map input data into filter properties with attributes - * - * @return array{0: array, 1: array, 2: array} + * @return array{0: array, 1: array, 2: array, 3: array} */ public function map(FilterInterface $filter, InputInterface $input): array { $errors = []; $schema = []; $setters = []; + $optionalFilters = []; $class = new \ReflectionClass($filter); foreach ($class->getProperties() as $property) { /** @var object $attribute */ foreach ($this->reader->getPropertyMetadata($property) as $attribute) { if ($attribute instanceof AbstractInput) { - $this->setValue($filter, $property, $attribute->getValue($input, $property)); + try { + $this->setValue($filter, $property, $attribute->getValue($input, $property)); + } catch (SetterException $e) { + $errors[$property->getName()] = $e->getMessage(); + } $schema[$property->getName()] = $attribute->getSchema($property); } elseif ($attribute instanceof NestedFilter) { $prefix = $attribute->prefix ?? $property->name; @@ -51,9 +57,18 @@ public function map(FilterInterface $filter, InputInterface $input): array $input->withPrefix($prefix) ); - $this->setValue($filter, $property, $value); + try { + $this->setValue($filter, $property, $value); + } catch (SetterException $e) { + $errors[$property->getName()] = $e->getMessage(); + } } catch (ValidationException $e) { - $errors[$prefix] = $e->errors; + if ($this->allowsNull($property)) { + $this->setValue($filter, $property, null); + $optionalFilters[] = $property->getName(); + } else { + $errors[$prefix] = $e->errors; + } } $schema[$property->getName()] = $attribute->getSchema($property); @@ -71,12 +86,17 @@ public function map(FilterInterface $filter, InputInterface $input): array $input->withPrefix($prefix . '.' . $key) ); } catch (ValidationException $e) { + /** @psalm-suppress InvalidArrayOffset */ $errors[$property->getName()][$key] = $e->errors; } } } - $this->setValue($filter, $property, $propertyValues); + try { + $this->setValue($filter, $property, $propertyValues); + } catch (SetterException $e) { + $errors[$property->getName()] = $e->getMessage(); + } $schema[$property->getName()] = [$attribute->class, $prefix . '.*']; } elseif ($attribute instanceof Setter) { $setters[$property->getName()][] = $attribute; @@ -84,7 +104,7 @@ public function map(FilterInterface $filter, InputInterface $input): array } } - return [$schema, $errors, $setters]; + return [$schema, $errors, $setters, $optionalFilters]; } private function setValue(FilterInterface $filter, \ReflectionProperty $property, mixed $value): void @@ -99,6 +119,13 @@ private function setValue(FilterInterface $filter, \ReflectionProperty $property $value = $setter->updateValue($value); } - $property->setValue($filter, $value); + $this->mapper->setValue($filter, $property, $value); + } + + private function allowsNull(\ReflectionProperty $property): bool + { + $type = $property->getType(); + + return $type === null || $type->allowsNull(); } } diff --git a/src/Model/Schema/InputMapper.php b/src/Model/Schema/InputMapper.php index ac7528b..72905e3 100644 --- a/src/Model/Schema/InputMapper.php +++ b/src/Model/Schema/InputMapper.php @@ -5,6 +5,7 @@ namespace Spiral\Filters\Model\Schema; use Spiral\Filters\Attribute\Setter; +use Spiral\Filters\Exception\SetterException; use Spiral\Filters\Model\FilterProviderInterface; use Spiral\Filters\Exception\ValidationException; use Spiral\Filters\InputInterface; @@ -28,7 +29,11 @@ public function map(array $mappingSchema, InputInterface $input, array $setters if ($value !== null) { /** @var Setter $setter */ foreach ($setters[$field] ?? [] as $setter) { - $value = $setter->updateValue($value); + try { + $value = $setter->updateValue($value); + } catch (SetterException $e) { + $errors[$field] = $e->getMessage(); + } } $result[$field] = $value; @@ -42,9 +47,12 @@ public function map(array $mappingSchema, InputInterface $input, array $setters try { $result[$field] = $this->provider->createFilter($nested, $input->withPrefix($map[Builder::SCHEMA_ORIGIN])); } catch (ValidationException $e) { + if ($map[Builder::SCHEMA_OPTIONAL]) { + $result[$field] = null; + continue; + } $errors[$field] = $e->errors; } - continue; } diff --git a/tests/Fixtures/Status.php b/tests/Fixtures/Status.php new file mode 100644 index 0000000..2503ab5 --- /dev/null +++ b/tests/Fixtures/Status.php @@ -0,0 +1,11 @@ +assertCount(0, $registry->getCasters()); + + $setter = $this->createMock(CasterInterface::class); + $registry->register($setter); + $this->assertCount(1, $registry->getCasters()); + $this->assertSame([$setter], $registry->getCasters()); + } + + public function testGetDefault(): void + { + $registry = new CasterRegistry(); + $this->assertInstanceOf(DefaultCaster::class, $registry->getDefault()); + } +} diff --git a/tests/Mapper/DefaultCasterTest.php b/tests/Mapper/DefaultCasterTest.php new file mode 100644 index 0000000..8d297ca --- /dev/null +++ b/tests/Mapper/DefaultCasterTest.php @@ -0,0 +1,27 @@ +assertTrue((new DefaultCaster())->supports($this->createMock(\ReflectionNamedType::class))); + } + + public function testSetValue(): void + { + $setter = new DefaultCaster(); + $filter = $this->createMock(AddressFilter::class); + $property = new \ReflectionProperty($filter, 'city'); + + $setter->setValue($filter, $property, 'foo'); + $this->assertSame('foo', $property->getValue($filter)); + } +} diff --git a/tests/Mapper/EnumCasterTest.php b/tests/Mapper/EnumCasterTest.php new file mode 100644 index 0000000..81189de --- /dev/null +++ b/tests/Mapper/EnumCasterTest.php @@ -0,0 +1,38 @@ +assertSame($expected, (new EnumCaster())->supports($ref->getType())); + } + + public function testSetValue(): void + { + $setter = new EnumCaster(); + $filter = $this->createMock(UserFilter::class); + $property = new \ReflectionProperty($filter, 'status'); + + $setter->setValue($filter, $property, 'active'); + $this->assertEquals(Status::Active, $property->getValue($filter)); + } + + public static function supportsDataProvider(): \Traversable + { + $ref = new \ReflectionClass(UserFilter::class); + + yield 'enum' => [$ref->getProperty('status'), true]; + yield 'uuid' => [$ref->getProperty('groupUuid'), false]; + } +} diff --git a/tests/Mapper/UuidCasterTest.php b/tests/Mapper/UuidCasterTest.php new file mode 100644 index 0000000..9bef41e --- /dev/null +++ b/tests/Mapper/UuidCasterTest.php @@ -0,0 +1,37 @@ +assertSame($expected, (new UuidCaster())->supports($ref->getType())); + } + + public function testSetValue(): void + { + $setter = new UuidCaster(); + $filter = $this->createMock(UserFilter::class); + $property = new \ReflectionProperty($filter, 'groupUuid'); + + $setter->setValue($filter, $property, '11111111-1111-1111-1111-111111111111'); + $this->assertSame('11111111-1111-1111-1111-111111111111', $property->getValue($filter)->toString()); + } + + public static function supportsDataProvider(): \Traversable + { + $ref = new \ReflectionClass(UserFilter::class); + + yield 'enum' => [$ref->getProperty('status'), false]; + yield 'uuid' => [$ref->getProperty('groupUuid'), true]; + } +} diff --git a/tests/Model/FilterProviderTest.php b/tests/Model/FilterProviderTest.php index 8eeae2d..2670ccd 100644 --- a/tests/Model/FilterProviderTest.php +++ b/tests/Model/FilterProviderTest.php @@ -6,19 +6,37 @@ use Nyholm\Psr7\ServerRequest; use Psr\Http\Message\ServerRequestInterface; +use Ramsey\Uuid\UuidInterface; use Spiral\Attributes\Factory; +use Spiral\Attributes\ReaderInterface; use Spiral\Filter\InputScope; use Spiral\Filters\Model\FilterProvider; use Spiral\Filters\Model\FilterProviderInterface; use Spiral\Filters\Model\Interceptor\Core; -use Spiral\Filters\Model\Schema\AttributeMapper; +use Spiral\Filters\Model\Mapper\EnumCaster; +use Spiral\Filters\Model\Mapper\CasterRegistry; +use Spiral\Filters\Model\Mapper\CasterRegistryInterface; +use Spiral\Filters\Model\Mapper\UuidCaster; use Spiral\Http\Request\InputManager; use Spiral\Tests\Filters\BaseTestCase; use Spiral\Tests\Filters\Fixtures\LogoutFilter; use Spiral\Tests\Filters\Fixtures\SomeFilter; +use Spiral\Tests\Filters\Fixtures\Status; +use Spiral\Tests\Filters\Fixtures\UserFilter; final class FilterProviderTest extends BaseTestCase { + public function setUp(): void + { + parent::setUp(); + + $this->container->bindSingleton(ReaderInterface::class, (new Factory())->create()); + $this->container->bindSingleton( + CasterRegistryInterface::class, + static fn () => new CasterRegistry([new EnumCaster(), new UuidCaster()]) + ); + } + public function testCreateNestedFilterWithIdenticalPropertyNames(): void { $request = new ServerRequest('POST', '/'); @@ -34,12 +52,11 @@ public function testCreateNestedFilterWithIdenticalPropertyNames(): void $inputManager = new InputManager($this->container); $input = new InputScope($inputManager); - $provider = new FilterProvider($this->container, $this->container, new Core()); + $provider = $this->container->make(FilterProvider::class, [ + 'core' => new Core(), + ]); $this->container->bind(FilterProviderInterface::class, $provider); - $mapper = new AttributeMapper($provider, (new Factory())->create()); - $this->container->bind(AttributeMapper::class, $mapper); - /** @var SomeFilter $filter */ $filter = $provider->createFilter(SomeFilter::class, $input); @@ -59,15 +76,42 @@ public function testCreateFilterWithInputAttribute(): void $inputManager = new InputManager($this->container); $input = new InputScope($inputManager); - $provider = new FilterProvider($this->container, $this->container, new Core()); + $provider = $this->container->make(FilterProvider::class, [ + 'core' => new Core(), + ]); $this->container->bind(FilterProviderInterface::class, $provider); - $mapper = new AttributeMapper($provider, (new Factory())->create()); - $this->container->bind(AttributeMapper::class, $mapper); - /** @var LogoutFilter $filter */ $filter = $provider->createFilter(LogoutFilter::class, $input); $this->assertSame('some', $filter->token); } + + public function testCreateFilterWithEnumAndUuid(): void + { + $request = new ServerRequest('POST', '/'); + $request = $request->withParsedBody([ + 'name' => 'John', + 'status' => 'active', + 'groupUuid' => 'f0a0b2c0-5b4b-4a5c-8d3e-6f7a8f9b0c1d', + ]); + $this->container->bind(ServerRequestInterface::class, $request); + + $inputManager = new InputManager($this->container); + $input = new InputScope($inputManager); + + $provider = $this->container->make(FilterProvider::class, [ + 'core' => new Core(), + ]); + $this->container->bind(FilterProviderInterface::class, $provider); + + /** @var UserFilter $filter */ + $filter = $provider->createFilter(UserFilter::class, $input); + + $this->assertSame('John', $filter->name); + $this->assertInstanceOf(Status::class, $filter->status); + $this->assertEquals(Status::Active, $filter->status); + $this->assertInstanceOf(UuidInterface::class, $filter->groupUuid); + $this->assertSame('f0a0b2c0-5b4b-4a5c-8d3e-6f7a8f9b0c1d', $filter->groupUuid->toString()); + } } diff --git a/tests/Model/Schema/AttributeMapperTest.php b/tests/Model/Schema/AttributeMapperTest.php index 9f47435..7823877 100644 --- a/tests/Model/Schema/AttributeMapperTest.php +++ b/tests/Model/Schema/AttributeMapperTest.php @@ -15,6 +15,10 @@ use Spiral\Filters\Attribute\NestedFilter; use Spiral\Filters\Model\FilterInterface; use Spiral\Filters\Model\FilterProviderInterface; +use Spiral\Filters\Model\Mapper\EnumCaster; +use Spiral\Filters\Model\Mapper\Mapper; +use Spiral\Filters\Model\Mapper\CasterRegistry; +use Spiral\Filters\Model\Mapper\UuidCaster; use Spiral\Filters\Model\Schema\AttributeMapper; use Spiral\Filters\Exception\ValidationException; use Spiral\Filters\InputInterface; @@ -31,7 +35,8 @@ public function setUp(): void $this->mapper = new AttributeMapper( $this->provider = m::mock(FilterProviderInterface::class), - new AttributeReader() + new AttributeReader(), + new Mapper(new CasterRegistry([new UuidCaster(), new EnumCaster()])) ); }