From d2b4901a432c4a05874764814d9a2b95f8fa9abf Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 4 Jul 2018 15:24:49 +0200 Subject: [PATCH] [DI] Improve exception messages by hiding the hidden ids they contain --- ...xceptionOnInvalidReferenceBehaviorPass.php | 58 ++++++++++++++++++- .../Compiler/PassConfig.php | 1 + .../Compiler/ServiceLocatorTagPass.php | 1 + .../DependencyInjection/ServiceLocator.php | 17 ++++++ ...tionOnInvalidReferenceBehaviorPassTest.php | 33 +++++++++++ .../Tests/ServiceLocatorTest.php | 15 +++++ 6 files changed, 122 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php index b13f2bdc5f..7e6016bf3e 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Reference; @@ -22,15 +23,66 @@ use Symfony\Component\DependencyInjection\Reference; */ class CheckExceptionOnInvalidReferenceBehaviorPass extends AbstractRecursivePass { + private $serviceLocatorContextIds = array(); + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + $this->serviceLocatorContextIds = array(); + foreach ($container->findTaggedServiceIds('container.service_locator_context') as $id => $tags) { + $this->serviceLocatorContextIds[$id] = $tags[0]['id']; + $container->getDefinition($id)->clearTag('container.service_locator_context'); + } + + try { + return parent::process($container); + } finally { + $this->serviceLocatorContextIds = array(); + } + } + protected function processValue($value, $isRoot = false) { if (!$value instanceof Reference) { return parent::processValue($value, $isRoot); } - if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE >= $value->getInvalidBehavior() && !$this->container->has($id = (string) $value)) { - throw new ServiceNotFoundException($id, $this->currentId); + if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE < $value->getInvalidBehavior() || $this->container->has($id = (string) $value)) { + return $value; } - return $value; + $currentId = $this->currentId; + $graph = $this->container->getCompiler()->getServiceReferenceGraph(); + + if (isset($this->serviceLocatorContextIds[$currentId])) { + $currentId = $this->serviceLocatorContextIds[$currentId]; + $locator = $this->container->getDefinition($this->currentId)->getFactory()[0]; + + foreach ($locator->getArgument(0) as $k => $v) { + if ($v->getValues()[0] === $value) { + if ($k !== $id) { + $currentId = $k.'" in the container provided to "'.$currentId; + } + throw new ServiceNotFoundException($id, $currentId); + } + } + } + + if ('.' === $currentId[0] && $graph->hasNode($currentId)) { + foreach ($graph->getNode($currentId)->getInEdges() as $edge) { + if (!$edge->getValue() instanceof Reference || ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE < $edge->getValue()->getInvalidBehavior()) { + continue; + } + $sourceId = $edge->getSourceNode()->getId(); + + if ('.' !== $sourceId[0]) { + $currentId = $sourceId; + break; + } + } + } + + throw new ServiceNotFoundException($id, $currentId); } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index 0d8ba9c047..c80871524e 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -83,6 +83,7 @@ class PassConfig new RemoveAbstractDefinitionsPass(), new RemoveUnusedDefinitionsPass(), new InlineServiceDefinitionsPass(new AnalyzeServiceReferencesPass()), + new AnalyzeServiceReferencesPass(), new DefinitionErrorExceptionPass(), new CheckExceptionOnInvalidReferenceBehaviorPass(), new ResolveHotPathPass(), diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php index 3969c74fcb..af9fe34314 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php @@ -103,6 +103,7 @@ final class ServiceLocatorTagPass extends AbstractRecursivePass $container->register($id .= '.'.$callerId, ServiceLocator::class) ->setPublic(false) ->setFactory(array(new Reference($locatorId), 'withContext')) + ->addTag('container.service_locator_context', array('id' => $callerId)) ->addArgument($callerId) ->addArgument(new Reference('service_container')); } diff --git a/src/Symfony/Component/DependencyInjection/ServiceLocator.php b/src/Symfony/Component/DependencyInjection/ServiceLocator.php index 324cccab98..1078695cde 100644 --- a/src/Symfony/Component/DependencyInjection/ServiceLocator.php +++ b/src/Symfony/Component/DependencyInjection/ServiceLocator.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection; use Psr\Container\ContainerInterface as PsrContainerInterface; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException; use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; @@ -62,6 +63,22 @@ class ServiceLocator implements PsrContainerInterface $this->loading[$id] = $id; try { return $this->factories[$id](); + } catch (RuntimeException $e) { + if (!$this->externalId) { + throw $e; + } + $what = sprintf('service "%s" required by "%s"', $id, $this->externalId); + $message = preg_replace('/service "\.service_locator\.[^"]++"/', $what, $e->getMessage()); + + if ($e->getMessage() === $message) { + $message = sprintf('Cannot resolve %s: %s', $what, $message); + } + + $r = new \ReflectionProperty($e, 'message'); + $r->setAccessible(true); + $r->setValue($e, $message); + + throw $e; } finally { unset($this->loading[$id]); } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckExceptionOnInvalidReferenceBehaviorPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckExceptionOnInvalidReferenceBehaviorPassTest.php index a3fbfcf101..c71c9dbb67 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckExceptionOnInvalidReferenceBehaviorPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckExceptionOnInvalidReferenceBehaviorPassTest.php @@ -14,7 +14,10 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Argument\BoundArgument; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass; use Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass; +use Symfony\Component\DependencyInjection\Compiler\InlineServiceDefinitionsPass; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -82,6 +85,36 @@ class CheckExceptionOnInvalidReferenceBehaviorPassTest extends TestCase $this->addToAssertionCount(1); } + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException + * @expectedExceptionMessage The service "foo" in the container provided to "bar" has a dependency on a non-existent service "baz". + */ + public function testWithErroredServiceLocator() + { + $container = new ContainerBuilder(); + + ServiceLocatorTagPass::register($container, array('foo' => new Reference('baz')), 'bar'); + + (new AnalyzeServiceReferencesPass())->process($container); + (new InlineServiceDefinitionsPass())->process($container); + $this->process($container); + } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException + * @expectedExceptionMessage The service "bar" has a dependency on a non-existent service "foo". + */ + public function testWithErroredHiddenService() + { + $container = new ContainerBuilder(); + + ServiceLocatorTagPass::register($container, array('foo' => new Reference('foo')), 'bar'); + + (new AnalyzeServiceReferencesPass())->process($container); + (new InlineServiceDefinitionsPass())->process($container); + $this->process($container); + } + private function process(ContainerBuilder $container) { $pass = new CheckExceptionOnInvalidReferenceBehaviorPass(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php b/src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php index 56fac643eb..9d8692bc73 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php @@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Container; +use Symfony\Component\DependencyInjection\Exception\RuntimeException; use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\DependencyInjection\ServiceSubscriberInterface; @@ -126,6 +127,20 @@ class ServiceLocatorTest extends TestCase $this->assertSame('baz', $locator('bar')); $this->assertNull($locator('dummy'), '->__invoke() should return null on invalid service'); } + + /** + * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException + * @expectedExceptionMessage Invalid service "foo" required by "external-id". + */ + public function testRuntimeException() + { + $locator = new ServiceLocator(array( + 'foo' => function () { throw new RuntimeException('Invalid service ".service_locator.abcdef".'); }, + )); + + $locator = $locator->withContext('external-id', new Container()); + $locator->get('foo'); + } } class SomeServiceSubscriber implements ServiceSubscriberinterface