Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #25381 [DI] Add context to service-not-found exceptions thrown by…
… service locators (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Add context to service-not-found exceptions thrown by service locators

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes (DX)
| New feature?  | yes (...)
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25342, #25196
| License       | MIT
| Doc PR        | -

Here hopefully is the fully-context aware message you're looking for @weaverryan @curry684.

![image](https://user-images.githubusercontent.com/243674/33726013-1db38a34-db74-11e7-91dd-ca9d53e58891.png)

Commits
-------

9512f26 [DI] Add context to service-not-found exceptions thrown by service locators
  • Loading branch information
fabpot committed Dec 11, 2017
2 parents d21aa19 + 9512f26 commit 5b66c26
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 20 deletions.
Expand Up @@ -94,7 +94,7 @@ protected function processValue($value, $isRoot = false)
throw new InvalidArgumentException(sprintf('Service %s not exist in the map returned by "%s::getSubscribedServices()" for service "%s".', $message, $class, $this->currentId));
}

$value->addTag('container.service_subscriber.locator', array('id' => (string) ServiceLocatorTagPass::register($this->container, $subscriberMap)));
$value->addTag('container.service_subscriber.locator', array('id' => (string) ServiceLocatorTagPass::register($this->container, $subscriberMap, $this->currentId)));

return parent::processValue($value);
}
Expand Down
Expand Up @@ -72,10 +72,11 @@ protected function processValue($value, $isRoot = false)
/**
* @param ContainerBuilder $container
* @param Reference[] $refMap
* @param string|null $callerId
*
* @return Reference
*/
public static function register(ContainerBuilder $container, array $refMap)
public static function register(ContainerBuilder $container, array $refMap, $callerId = null)
{
foreach ($refMap as $id => $ref) {
if (!$ref instanceof Reference) {
Expand All @@ -94,6 +95,18 @@ public static function register(ContainerBuilder $container, array $refMap)
$container->setDefinition($id, $locator);
}

if (null !== $callerId) {
$locatorId = $id;
// Locators are shared when they hold the exact same list of factories;
// to have them specialized per consumer service, we use a cloning factory
// to derivate customized instances from the prototype one.
$container->register($id .= '.'.$callerId, ServiceLocator::class)
->setPublic(false)
->setFactory(array(new Reference($locatorId), 'withContext'))
->addArgument($callerId)
->addArgument(new Reference('service_container'));
}

return new Reference($id);
}
}
90 changes: 84 additions & 6 deletions src/Symfony/Component/DependencyInjection/ServiceLocator.php
Expand Up @@ -22,6 +22,9 @@
class ServiceLocator implements PsrContainerInterface
{
private $factories;
private $loading = array();
private $externalId;
private $container;

/**
* @param callable[] $factories
Expand All @@ -45,23 +48,98 @@ public function has($id)
public function get($id)
{
if (!isset($this->factories[$id])) {
throw new ServiceNotFoundException($id, null, null, array_keys($this->factories));
throw new ServiceNotFoundException($id, end($this->loading) ?: null, null, array(), $this->createServiceNotFoundMessage($id));
}

if (true === $factory = $this->factories[$id]) {
throw new ServiceCircularReferenceException($id, array($id, $id));
if (isset($this->loading[$id])) {
$ids = array_values($this->loading);
$ids = array_slice($this->loading, array_search($id, $ids));
$ids[] = $id;

throw new ServiceCircularReferenceException($id, $ids);
}

$this->factories[$id] = true;
$this->loading[$id] = $id;
try {
return $factory();
return $this->factories[$id]();
} finally {
$this->factories[$id] = $factory;
unset($this->loading[$id]);
}
}

public function __invoke($id)
{
return isset($this->factories[$id]) ? $this->get($id) : null;
}

/**
* @internal
*/
public function withContext($externalId, Container $container)
{
$locator = clone $this;
$locator->externalId = $externalId;
$locator->container = $container;

return $locator;
}

private function createServiceNotFoundMessage($id)
{
if ($this->loading) {
return sprintf('The service "%s" has a dependency on a non-existent service "%s". This locator %s', end($this->loading), $id, $this->formatAlternatives());
}

$class = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS, 3);
$class = isset($class[2]['object']) ? get_class($class[2]['object']) : null;
$externalId = $this->externalId ?: $class;

$msg = sprintf('Service "%s" not found: ', $id);

if (!$this->container) {
$class = null;
} elseif ($this->container->has($id) || isset($this->container->getRemovedIds()[$id])) {
$msg .= 'even though it exists in the app\'s container, ';
} else {
try {
$this->container->get($id);
$class = null;
} catch (ServiceNotFoundException $e) {
if ($e->getAlternatives()) {
$msg .= sprintf(' did you mean %s? Anyway, ', $this->formatAlternatives($e->getAlternatives(), 'or'));
} else {
$class = null;
}
}
}
if ($externalId) {
$msg .= sprintf('the container inside "%s" is a smaller service locator that %s', $externalId, $this->formatAlternatives());
} else {
$msg .= sprintf('the current service locator %s', $this->formatAlternatives());
}

if (!$class) {
// no-op
} elseif (is_subclass_of($class, ServiceSubscriberInterface::class)) {
$msg .= sprintf(' Unless you need extra laziness, try using dependency injection instead. Otherwise, you need to declare it using "%s::getSubscribedServices()".', preg_replace('/([^\\\\]++\\\\)++/', '', $class));
} else {
$msg .= 'Try using dependency injection instead.';
}

return $msg;
}

private function formatAlternatives(array $alternatives = null, $separator = 'and')
{
$format = '"%s"%s';
if (null === $alternatives) {
if (!$alternatives = array_keys($this->factories)) {
return 'is empty...';
}
$format = sprintf('only knows about the %s service%s.', $format, 1 < count($alternatives) ? 's' : '');
}
$last = array_pop($alternatives);

return sprintf($format, $alternatives ? implode('", "', $alternatives) : $last, $alternatives ? sprintf(' %s "%s"', $separator, $last) : '');
}
}
Expand Up @@ -85,7 +85,7 @@ public function testNoAttributes()
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
);

$this->assertEquals($expected, $locator->getArgument(0));
$this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));
}

public function testWithAttributes()
Expand Down Expand Up @@ -115,7 +115,7 @@ public function testWithAttributes()
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
);

$this->assertEquals($expected, $locator->getArgument(0));
$this->assertEquals($expected, $container->getDefinition((string) $locator->getFactory()[0])->getArgument(0));
}

/**
Expand Down
Expand Up @@ -45,6 +45,7 @@ public function getRemovedIds()
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true,
'service_locator.jmktfsv' => true,
'service_locator.jmktfsv.foo_service' => true,
);
}

Expand Down Expand Up @@ -82,15 +83,15 @@ protected function getTestServiceSubscriberService()
*/
protected function getFooServiceService()
{
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber(new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => function () {
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber(\call_user_func(array(new \Symfony\Component\DependencyInjection\ServiceLocator(array('Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => function () {
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition()) && false ?: '_'});
}, 'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\TestServiceSubscriber' => function () {
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber()) && false ?: '_'});
}, 'bar' => function () {
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber()) && false ?: '_'});
}, 'baz' => function () {
$f = function (\Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition $v = null) { return $v; }; return $f(${($_ = isset($this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition']) ? $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] : $this->services['Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition()) && false ?: '_'});
})));
})), 'withContext'), 'foo_service', $this));
}

/**
Expand Down
Expand Up @@ -12,8 +12,9 @@
namespace Symfony\Component\DependencyInjection\Tests;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;

class ServiceLocatorTest extends TestCase
{
Expand Down Expand Up @@ -59,7 +60,7 @@ public function testGetDoesNotMemoize()

/**
* @expectedException \Psr\Container\NotFoundExceptionInterface
* @expectedExceptionMessage You have requested a non-existent service "dummy". Did you mean one of these: "foo", "bar"?
* @expectedExceptionMessage Service "dummy" not found: the container inside "Symfony\Component\DependencyInjection\Tests\ServiceLocatorTest" is a smaller service locator that only knows about the "foo" and "bar" services.
*/
public function testGetThrowsOnUndefinedService()
{
Expand All @@ -68,13 +69,50 @@ public function testGetThrowsOnUndefinedService()
'bar' => function () { return 'baz'; },
));

try {
$locator->get('dummy');
} catch (ServiceNotFoundException $e) {
$this->assertSame(array('foo', 'bar'), $e->getAlternatives());
$locator->get('dummy');
}

/**
* @expectedException \Psr\Container\NotFoundExceptionInterface
* @expectedExceptionMessage The service "foo" has a dependency on a non-existent service "bar". This locator only knows about the "foo" service.
*/
public function testThrowsOnUndefinedInternalService()
{
$locator = new ServiceLocator(array(
'foo' => function () use (&$locator) { return $locator->get('bar'); },
));

$locator->get('foo');
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
* @expectedExceptionMessage Circular reference detected for service "bar", path: "bar -> baz -> bar".
*/
public function testThrowsOnCircularReference()
{
$locator = new ServiceLocator(array(
'foo' => function () use (&$locator) { return $locator->get('bar'); },
'bar' => function () use (&$locator) { return $locator->get('baz'); },
'baz' => function () use (&$locator) { return $locator->get('bar'); },
));

throw $e;
}
$locator->get('foo');
}

/**
* @expectedException \Psr\Container\NotFoundExceptionInterface
* @expectedExceptionMessage Service "foo" not found: even though it exists in the app's container, the container inside "caller" is a smaller service locator that only knows about the "bar" service. Unless you need extra laziness, try using dependency injection instead. Otherwise, you need to declare it using "SomeServiceSubscriber::getSubscribedServices()".
*/
public function testThrowsInServiceSubscriber()
{
$container = new Container();
$container->set('foo', new \stdClass());
$subscriber = new SomeServiceSubscriber();
$subscriber->container = new ServiceLocator(array('bar' => function () {}));
$subscriber->container = $subscriber->container->withContext('caller', $container);

$subscriber->getFoo();
}

public function testInvoke()
Expand All @@ -89,3 +127,18 @@ public function testInvoke()
$this->assertNull($locator('dummy'), '->__invoke() should return null on invalid service');
}
}

class SomeServiceSubscriber implements ServiceSubscriberinterface
{
public $container;

public function getFoo()
{
return $this->container->get('foo');
}

public static function getSubscribedServices()
{
return array('bar' => 'stdClass');
}
}

0 comments on commit 5b66c26

Please sign in to comment.