From d6c3818f6fc20db25dc41d76f50f1c74aeb8bd37 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 28 Feb 2018 10:40:59 +0000 Subject: [PATCH 1/2] Added failing test where cyclic dependencies should be detected by ReflectionBasedAbstractFactory --- .../ReflectionBasedAbstractFactoryTest.php | 24 +++++++++++++++++++ ...coratorWithCyclicDependencyOnInterface.php | 15 ++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 test/AbstractFactory/TestAsset/DecoratorWithCyclicDependencyOnInterface.php diff --git a/test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php b/test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php index 254d2563..970d8304 100644 --- a/test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php +++ b/test/AbstractFactory/ReflectionBasedAbstractFactoryTest.php @@ -11,7 +11,10 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Zend\ServiceManager\AbstractFactory\ReflectionBasedAbstractFactory; +use Zend\ServiceManager\Exception\InvalidServiceException; use Zend\ServiceManager\Exception\ServiceNotFoundException; +use Zend\ServiceManager\ServiceManager; +use ZendTest\ServiceManager\AbstractFactory\TestAsset\SampleInterface; use function sprintf; @@ -173,4 +176,25 @@ public function testFactoryWillUseDefaultValueForTypeHintedArgument() $this->assertInstanceOf(TestAsset\ClassWithTypehintedDefaultValue::class, $instance); $this->assertNull($instance->value); } + + public function testFactoryThrowsExceptionWhenCyclicDependencyDetectedForDecorators(): void + { + $serviceManager = new ServiceManager(); + $serviceManager->setAlias( + TestAsset\SampleInterface::class, + TestAsset\DecoratorWithCyclicDependencyOnInterface::class + ); + $serviceManager->addAbstractFactory(ReflectionBasedAbstractFactory::class); + $serviceManager->setFactory( + TestAsset\DecoratorWithCyclicDependencyOnInterface::class, + ReflectionBasedAbstractFactory::class + ); + + $this->expectException(InvalidServiceException::class); + $this->expectExceptionMessage(sprintf( + 'Unable to create service "%s"; a cyclic dependency was detected', + TestAsset\SampleInterface::class + )); + $serviceManager->get(SampleInterface::class); + } } diff --git a/test/AbstractFactory/TestAsset/DecoratorWithCyclicDependencyOnInterface.php b/test/AbstractFactory/TestAsset/DecoratorWithCyclicDependencyOnInterface.php new file mode 100644 index 00000000..1571c0c7 --- /dev/null +++ b/test/AbstractFactory/TestAsset/DecoratorWithCyclicDependencyOnInterface.php @@ -0,0 +1,15 @@ + Date: Wed, 28 Feb 2018 10:41:49 +0000 Subject: [PATCH 2/2] Added reflection-based implementation of alias resolution to check for cyclic dependencies in ReflectionBasedAbstractFactory --- .../ReflectionBasedAbstractFactory.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/AbstractFactory/ReflectionBasedAbstractFactory.php b/src/AbstractFactory/ReflectionBasedAbstractFactory.php index 6df35677..3c4752f8 100644 --- a/src/AbstractFactory/ReflectionBasedAbstractFactory.php +++ b/src/AbstractFactory/ReflectionBasedAbstractFactory.php @@ -10,8 +10,11 @@ use Psr\Container\ContainerInterface; use ReflectionClass; use ReflectionParameter; +use ReflectionProperty; +use Zend\ServiceManager\Exception\InvalidServiceException; use Zend\ServiceManager\Exception\ServiceNotFoundException; use Zend\ServiceManager\Factory\AbstractFactoryInterface; +use Zend\ServiceManager\ServiceManager; use function array_map; use function class_exists; @@ -197,8 +200,10 @@ private function resolveParameterWithConfigService(ContainerInterface $container * @param ContainerInterface $container * @param string $requestedName * @return mixed + * @throws \Zend\ServiceManager\Exception\InvalidServiceException * @throws ServiceNotFoundException If type-hinted parameter cannot be * resolved to a service in the container. + * @throws \ReflectionException */ private function resolveParameter(ReflectionParameter $parameter, ContainerInterface $container, $requestedName) { @@ -223,6 +228,19 @@ private function resolveParameter(ReflectionParameter $parameter, ContainerInter $type = $this->aliases[$type] ?? $type; if ($container->has($type)) { + if ($container instanceof ServiceManager) { + $aliasProperty = new ReflectionProperty($container, 'aliases'); + $aliasProperty->setAccessible(true); + $serviceManagerAliases = $aliasProperty->getValue($container); + $unaliasedService = array_search($requestedName, $serviceManagerAliases, true); + + if (false !== $unaliasedService && $unaliasedService === $type) { + throw new InvalidServiceException(sprintf( + 'Unable to create service "%s"; a cyclic dependency was detected', + $unaliasedService + )); + } + } return $container->get($type); }