From bfadd6c51c95570a87a1cf95852992e506e51778 Mon Sep 17 00:00:00 2001 From: Erick Dyck Date: Wed, 23 Oct 2024 22:52:15 +0200 Subject: [PATCH] #9 Added various validations --- src/AbstractFactory/DynamicFactory.php | 11 +- src/AbstractFactory/ReflectionFactory.php | 24 +++-- src/Exception/ContainerException.php | 4 +- src/Exception/NotFoundException.php | 4 +- src/FactoryInterface.php | 6 +- src/InvokableFactory.php | 2 +- src/ServiceManager.php | 100 +++++++++++++++--- .../AbstractFactory/ReflectionFactoryTest.php | 10 +- test/Asset/ClassWithoutDependencies.php | 7 +- test/Asset/ClassWithoutFactory.php | 11 +- .../ClassWithoutFactoryAndScalarTypeHint.php | 9 +- test/Asset/FooBar.php | 9 +- test/Asset/FooBarFactory.php | 2 +- test/Asset/FooBarFactoryWithOptions.php | 2 +- test/ServiceManagerTest.php | 17 ++- 15 files changed, 143 insertions(+), 75 deletions(-) diff --git a/src/AbstractFactory/DynamicFactory.php b/src/AbstractFactory/DynamicFactory.php index 4883185..80733fc 100644 --- a/src/AbstractFactory/DynamicFactory.php +++ b/src/AbstractFactory/DynamicFactory.php @@ -5,18 +5,20 @@ namespace BlackBonjour\ServiceManager\AbstractFactory; use BlackBonjour\ServiceManager\Exception\ContainerException; +use BlackBonjour\ServiceManager\FactoryInterface; use Psr\Container\ContainerInterface; /** * @author Erick Dyck * @since 18.09.2019 */ -class DynamicFactory implements AbstractFactoryInterface +final class DynamicFactory implements AbstractFactoryInterface { /** + * @inheritDoc * @throws ContainerException */ - public function __invoke(ContainerInterface $container, string $service, ?array $options = null) + public function __invoke(ContainerInterface $container, string $service, ?array $options = null): mixed { if ($this->canCreate($container, $service) === false) { throw new ContainerException(sprintf('Cannot create service "%s"!', $service)); @@ -25,6 +27,11 @@ public function __invoke(ContainerInterface $container, string $service, ?array $factoryClass = $service . 'Factory'; $factory = new $factoryClass(); + assert( + $factory instanceof FactoryInterface || is_callable($factory), + sprintf('Dynamic factory "%s" for service "%s" is not callable!', $factoryClass, $service), + ); + return $factory($container, $service, $options); } diff --git a/src/AbstractFactory/ReflectionFactory.php b/src/AbstractFactory/ReflectionFactory.php index c82f361..5e573df 100644 --- a/src/AbstractFactory/ReflectionFactory.php +++ b/src/AbstractFactory/ReflectionFactory.php @@ -18,7 +18,7 @@ * @author Erick Dyck * @since 30.09.2019 */ -class ReflectionFactory implements AbstractFactoryInterface +final class ReflectionFactory implements AbstractFactoryInterface { /** * @inheritDoc @@ -28,19 +28,20 @@ class ReflectionFactory implements AbstractFactoryInterface * @throws NotFoundExceptionInterface * @throws ReflectionException */ - public function __invoke(ContainerInterface $container, string $service, ?array $options = null) + public function __invoke(ContainerInterface $container, string $service, ?array $options = null): mixed { if ($this->canCreate($container, $service) === false) { throw new ContainerException(sprintf('Cannot create service "%s"!', $service)); } + /** @var class-string $service */ $reflectionClass = new ReflectionClass($service); $parameters = $reflectionClass->getConstructor()?->getParameters() ?? []; if ($parameters) { $resolvedParameters = array_map( - fn (ReflectionParameter $parameter) => $this->resolveParameter($parameter, $container, $service), - $parameters + fn(ReflectionParameter $parameter): mixed => $this->resolveParameter($parameter, $container, $service), + $parameters, ); return new $service(...$resolvedParameters); @@ -70,8 +71,11 @@ public function canCreate(ContainerInterface $container, string $service): bool * @throws NotFoundExceptionInterface * @throws ReflectionException */ - private function resolveParameter(ReflectionParameter $parameter, ContainerInterface $container, string $service) - { + private function resolveParameter( + ReflectionParameter $parameter, + ContainerInterface $container, + string $service, + ): mixed { $reflectionType = $parameter->getType(); $type = $reflectionType instanceof ReflectionNamedType ? $reflectionType->getName() @@ -90,8 +94,8 @@ private function resolveParameter(ReflectionParameter $parameter, ContainerInter sprintf( 'Unable to create service "%s": Cannot resolve parameter "%s" to a class or interface!', $service, - $parameter->getName() - ) + $parameter->getName(), + ), ); } @@ -111,8 +115,8 @@ private function resolveParameter(ReflectionParameter $parameter, ContainerInter 'Unable to create service "%s": Cannot resolve parameter "%s" using type hint "%s"!', $service, $parameter->getName(), - $type - ) + $type, + ), ); } } diff --git a/src/Exception/ContainerException.php b/src/Exception/ContainerException.php index 509bc13..5dcf234 100644 --- a/src/Exception/ContainerException.php +++ b/src/Exception/ContainerException.php @@ -11,6 +11,4 @@ * @author Erick Dyck * @since 13.05.2019 */ -class ContainerException extends Error implements ContainerExceptionInterface -{ -} +class ContainerException extends Error implements ContainerExceptionInterface {} diff --git a/src/Exception/NotFoundException.php b/src/Exception/NotFoundException.php index 10c185b..5daf3ae 100644 --- a/src/Exception/NotFoundException.php +++ b/src/Exception/NotFoundException.php @@ -11,6 +11,4 @@ * @author Erick Dyck * @since 13.05.2019 */ -class NotFoundException extends Error implements NotFoundExceptionInterface -{ -} +class NotFoundException extends Error implements NotFoundExceptionInterface {} diff --git a/src/FactoryInterface.php b/src/FactoryInterface.php index b2ba28f..2db9f44 100644 --- a/src/FactoryInterface.php +++ b/src/FactoryInterface.php @@ -15,9 +15,9 @@ interface FactoryInterface /** * Creates a new service. * - * @param ContainerInterface $container A container implementing PSR-11. - * @param string $service Name of the service to create a new instance of. - * @param array|null $options Some options that can be passed to build the service. + * @param ContainerInterface $container A container implementing PSR-11. + * @param string $service Name of the service to create a new instance of. + * @param array|null $options Some options that can be passed to build the service. */ public function __invoke(ContainerInterface $container, string $service, ?array $options = null); } diff --git a/src/InvokableFactory.php b/src/InvokableFactory.php index 9364b44..6a46f44 100644 --- a/src/InvokableFactory.php +++ b/src/InvokableFactory.php @@ -11,7 +11,7 @@ * @author Erick Dyck * @since 23.02.2023 */ -class InvokableFactory implements FactoryInterface +final class InvokableFactory implements FactoryInterface { /** * @inheritDoc diff --git a/src/ServiceManager.php b/src/ServiceManager.php index 31d7b92..cd4614e 100644 --- a/src/ServiceManager.php +++ b/src/ServiceManager.php @@ -19,28 +19,78 @@ /** * @author Erick Dyck * @since 13.05.2019 + * + * @implements ArrayAccess */ class ServiceManager implements ArrayAccess, ContainerInterface { - /** @var array */ + /** @var array */ + private array $abstractFactories; + + /** @var array */ + private array $factories; + + /** @var array */ private array $invokables; - /** @var FactoryInterface[]|callable[] */ + /** @var array */ private array $resolvedFactories = []; + + /** @var array */ private array $resolvedServices = []; + /** @var array */ + private array $services; + /** - * @param FactoryInterface[]|callable[]|string[] $factories - * @param AbstractFactoryInterface[] $abstractFactories - * @param string[] $invokables + * @param array $services + * @param array $factories + * @param array $abstractFactories + * @param array $invokables */ public function __construct( - private array $services = [], - private array $factories = [], - private array $abstractFactories = [], + array $services = [], + array $factories = [], + array $abstractFactories = [], array $invokables = [], ) { - $this->invokables = array_combine($invokables, $invokables); + // Validate services + foreach (array_keys($services) as $id) { + assert(is_string($id), sprintf('Service ID must be a string, "%s" given!', $id)); + } + + // Validate factories + foreach ($factories as $id => $factory) { + assert(is_string($id), sprintf('Service ID must be a string, "%s" given!', $id)); + assert( + $factory instanceof FactoryInterface + || is_callable($factory) + || (is_string($factory) && class_exists($factory)), + sprintf('Invalid factory provided for service "%s"!', $id), + ); + } + + // Validate abstract factories + foreach ($abstractFactories as $abstractFactory) { + assert( + $abstractFactory instanceof AbstractFactoryInterface, + sprintf('Abstract factories must implement %s!', AbstractFactoryInterface::class), + ); + } + + // Validate invokable classes + foreach ($invokables as $invokable) { + assert( + is_string($invokable) && class_exists($invokable), + sprintf('Invokable class "%s" does not exist!', $invokable), + ); + } + + // Set properties + $this->abstractFactories = $abstractFactories; + $this->factories = $factories; + $this->invokables = array_combine($invokables, $invokables); + $this->services = $services; } public function addAbstractFactory(AbstractFactoryInterface $abstractFactory): void @@ -48,34 +98,44 @@ public function addAbstractFactory(AbstractFactoryInterface $abstractFactory): v $this->abstractFactories[] = $abstractFactory; } - public function addFactory(string $id, string|callable $factory): void + public function addFactory(string $id, FactoryInterface|callable|string $factory): void { + if (is_string($factory) && class_exists($factory) === false) { + throw new ContainerException(sprintf('Factory "%s" does not exist!', $factory)); + } + $this->factories[$id] = $factory; } public function addInvokable(string $id): void { - $this->invokables[$id] ??= $id; + if (class_exists($id) === false) { + throw new ContainerException(sprintf('Class "%s" does not exist!', $id)); + } + + $this->invokables[$id] = $id; } - public function addService(string $id, $service): void + public function addService(string $id, mixed $service): void { $this->services[$id] = $service; } /** + * @param array|null $options + * * @throws ContainerException */ - public function createService(string $id, ?array $options = null) + public function createService(string $id, ?array $options = null): mixed { try { return $this->getFactory($id)($this, $id, $options); } catch (Throwable $t) { - throw new ContainerException(sprintf('Service "%s" could not be created!', $id), 0, $t); + throw new ContainerException(sprintf('Service "%s" could not be created!', $id), previous: $t); } } - public function get(string $id) + public function get(string $id): mixed { if (array_key_exists($id, $this->services)) { return $this->services[$id]; @@ -108,6 +168,8 @@ public function has(string $id): bool public function offsetExists(mixed $offset): bool { + assert(is_string($offset), 'Service ID must be of type string!'); + return $this->has($offset); } @@ -117,16 +179,22 @@ public function offsetExists(mixed $offset): bool */ public function offsetGet(mixed $offset): mixed { + assert(is_string($offset), 'Service ID must be of type string!'); + return $this->get($offset); } public function offsetSet(mixed $offset, mixed $value): void { + assert(is_string($offset), 'Service ID must be of type string!'); + $this->addService($offset, $value); } public function offsetUnset(mixed $offset): void { + assert(is_string($offset), 'Service ID must be of type string!'); + $this->removeService($offset); } @@ -137,7 +205,7 @@ public function removeService(string $id): void $this->invokables[$id], $this->resolvedFactories[$id], $this->resolvedServices[$id], - $this->services[$id] + $this->services[$id], ); } diff --git a/test/AbstractFactory/ReflectionFactoryTest.php b/test/AbstractFactory/ReflectionFactoryTest.php index b7314cb..ef451dc 100644 --- a/test/AbstractFactory/ReflectionFactoryTest.php +++ b/test/AbstractFactory/ReflectionFactoryTest.php @@ -36,7 +36,7 @@ public function testInvoke(): void ->expects(self::once()) ->method('get') ->with(FooBar::class) - ->willReturn($this->createMock(FooBar::class)); + ->willReturn(new FooBar('', '')); $factory = new ReflectionFactory(); @@ -50,8 +50,8 @@ public function testInvokeUnknownService(): void sprintf( 'Unable to create service "%s": Cannot resolve parameter "foo" using type hint "%s"!', ClassWithoutFactory::class, - FooBar::class - ) + FooBar::class, + ), ); $container = $this->createMock(ContainerInterface::class); @@ -68,8 +68,8 @@ public function testInvokeWithNonOptionalScalarParams(): void $this->expectExceptionMessage( sprintf( 'Unable to create service "%s": Cannot resolve parameter "id" to a class or interface!', - ClassWithoutFactoryAndScalarTypeHint::class - ) + ClassWithoutFactoryAndScalarTypeHint::class, + ), ); $container = $this->createMock(ContainerInterface::class); diff --git a/test/Asset/ClassWithoutDependencies.php b/test/Asset/ClassWithoutDependencies.php index a43667a..3109cab 100644 --- a/test/Asset/ClassWithoutDependencies.php +++ b/test/Asset/ClassWithoutDependencies.php @@ -4,10 +4,9 @@ namespace BlackBonjourTest\ServiceManager\Asset; -class ClassWithoutDependencies +final readonly class ClassWithoutDependencies { public function __construct( - public readonly int $id = 1, - ) { - } + public int $id = 1, + ) {} } diff --git a/test/Asset/ClassWithoutFactory.php b/test/Asset/ClassWithoutFactory.php index 04bae4f..c2a2f8a 100644 --- a/test/Asset/ClassWithoutFactory.php +++ b/test/Asset/ClassWithoutFactory.php @@ -8,12 +8,11 @@ * @author Erick Dyck * @since 30.09.2019 */ -class ClassWithoutFactory +final readonly class ClassWithoutFactory { public function __construct( - public readonly FooBar $foo, - public readonly array $bar, - public readonly int $baz = 123, - ) { - } + public FooBar $foo, + public array $bar, + public int $baz = 123, + ) {} } diff --git a/test/Asset/ClassWithoutFactoryAndScalarTypeHint.php b/test/Asset/ClassWithoutFactoryAndScalarTypeHint.php index 28ee574..ae5851c 100644 --- a/test/Asset/ClassWithoutFactoryAndScalarTypeHint.php +++ b/test/Asset/ClassWithoutFactoryAndScalarTypeHint.php @@ -4,11 +4,10 @@ namespace BlackBonjourTest\ServiceManager\Asset; -class ClassWithoutFactoryAndScalarTypeHint +final readonly class ClassWithoutFactoryAndScalarTypeHint { public function __construct( - public readonly int $id, - public readonly string $foo, - ) { - } + public int $id, + public string $foo, + ) {} } diff --git a/test/Asset/FooBar.php b/test/Asset/FooBar.php index 31bea55..b0cdf30 100644 --- a/test/Asset/FooBar.php +++ b/test/Asset/FooBar.php @@ -8,11 +8,10 @@ * @author Erick Dyck * @since 18.09.2019 */ -class FooBar +final readonly class FooBar { public function __construct( - public readonly string $foo, - public readonly string $bar, - ) { - } + public string $foo, + public string $bar, + ) {} } diff --git a/test/Asset/FooBarFactory.php b/test/Asset/FooBarFactory.php index 00f4ebd..2cd358d 100644 --- a/test/Asset/FooBarFactory.php +++ b/test/Asset/FooBarFactory.php @@ -11,7 +11,7 @@ * @author Erick Dyck * @since 18.09.2019 */ -class FooBarFactory implements FactoryInterface +final class FooBarFactory implements FactoryInterface { /** * @inheritDoc diff --git a/test/Asset/FooBarFactoryWithOptions.php b/test/Asset/FooBarFactoryWithOptions.php index cf36ded..616e402 100644 --- a/test/Asset/FooBarFactoryWithOptions.php +++ b/test/Asset/FooBarFactoryWithOptions.php @@ -11,7 +11,7 @@ * @author Erick Dyck * @since 18.09.2019 */ -class FooBarFactoryWithOptions implements FactoryInterface +final class FooBarFactoryWithOptions implements FactoryInterface { /** * @inheritDoc diff --git a/test/ServiceManagerTest.php b/test/ServiceManagerTest.php index 0b93217..2a74574 100644 --- a/test/ServiceManagerTest.php +++ b/test/ServiceManagerTest.php @@ -14,7 +14,6 @@ use PHPUnit\Framework\TestCase; use stdClass; use Throwable; -use TypeError; /** * @author Erick Dyck @@ -38,13 +37,6 @@ public function testAddFactory(): void self::assertInstanceOf(FooBar::class, $manager[FooBar::class]); } - public function testAddFactoryWithException(): void - { - $this->expectException(TypeError::class); - - (new ServiceManager())->addFactory(FooBar::class, 123); - } - public function testAddInvokable(): void { $manager = new ServiceManager(invokables: [ClassWithoutDependencies::class]); @@ -136,7 +128,12 @@ public function testRemoveService(): void public function testRequestingServiceWithInvalidFactory(): void { - $manager = new ServiceManager([], [FooBar::class => 123], []); + $manager = new ServiceManager( + services : [], + factories : [FooBar::class => 123], + abstractFactories: [], + invokables : [], + ); try { $manager[FooBar::class]; @@ -145,7 +142,7 @@ public function testRequestingServiceWithInvalidFactory(): void self::assertInstanceOf(ContainerException::class, $t->getPrevious()); self::assertEquals( sprintf('Factory for service "%s" is invalid!', FooBar::class), - $t->getPrevious()->getMessage() + $t->getPrevious()->getMessage(), ); } }