Skip to content

Commit 9512f26

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] Add context to service-not-found exceptions thrown by service locators
1 parent ecf54d5 commit 9512f26

File tree

6 files changed

+165
-20
lines changed

6 files changed

+165
-20
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ protected function processValue($value, $isRoot = false)
9494
throw new InvalidArgumentException(sprintf('Service %s not exist in the map returned by "%s::getSubscribedServices()" for service "%s".', $message, $class, $this->currentId));
9595
}
9696

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

9999
return parent::processValue($value);
100100
}

‎src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,11 @@ protected function processValue($value, $isRoot = false)
7272
/**
7373
* @param ContainerBuilder $container
7474
* @param Reference[] $refMap
75+
* @param string|null $callerId
7576
*
7677
* @return Reference
7778
*/
78-
public static function register(ContainerBuilder $container, array $refMap)
79+
public static function register(ContainerBuilder $container, array $refMap, $callerId = null)
7980
{
8081
foreach ($refMap as $id => $ref) {
8182
if (!$ref instanceof Reference) {
@@ -94,6 +95,18 @@ public static function register(ContainerBuilder $container, array $refMap)
9495
$container->setDefinition($id, $locator);
9596
}
9697

98+
if (null !== $callerId) {
99+
$locatorId = $id;
100+
// Locators are shared when they hold the exact same list of factories;
101+
// to have them specialized per consumer service, we use a cloning factory
102+
// to derivate customized instances from the prototype one.
103+
$container->register($id .= '.'.$callerId, ServiceLocator::class)
104+
->setPublic(false)
105+
->setFactory(array(new Reference($locatorId), 'withContext'))
106+
->addArgument($callerId)
107+
->addArgument(new Reference('service_container'));
108+
}
109+
97110
return new Reference($id);
98111
}
99112
}

‎src/Symfony/Component/DependencyInjection/ServiceLocator.php

Lines changed: 84 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
class ServiceLocator implements PsrContainerInterface
2323
{
2424
private $factories;
25+
private $loading = array();
26+
private $externalId;
27+
private $container;
2528

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

51-
if (true === $factory = $this->factories[$id]) {
52-
throw new ServiceCircularReferenceException($id, array($id, $id));
54+
if (isset($this->loading[$id])) {
55+
$ids = array_values($this->loading);
56+
$ids = array_slice($this->loading, array_search($id, $ids));
57+
$ids[] = $id;
58+
59+
throw new ServiceCircularReferenceException($id, $ids);
5360
}
5461

55-
$this->factories[$id] = true;
62+
$this->loading[$id] = $id;
5663
try {
57-
return $factory();
64+
return $this->factories[$id]();
5865
} finally {
59-
$this->factories[$id] = $factory;
66+
unset($this->loading[$id]);
6067
}
6168
}
6269

6370
public function __invoke($id)
6471
{
6572
return isset($this->factories[$id]) ? $this->get($id) : null;
6673
}
74+
75+
/**
76+
* @internal
77+
*/
78+
public function withContext($externalId, Container $container)
79+
{
80+
$locator = clone $this;
81+
$locator->externalId = $externalId;
82+
$locator->container = $container;
83+
84+
return $locator;
85+
}
86+
87+
private function createServiceNotFoundMessage($id)
88+
{
89+
if ($this->loading) {
90+
return sprintf('The service "%s" has a dependency on a non-existent service "%s". This locator %s', end($this->loading), $id, $this->formatAlternatives());
91+
}
92+
93+
$class = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT | DEBUG_BACKTRACE_IGNORE_ARGS, 3);
94+
$class = isset($class[2]['object']) ? get_class($class[2]['object']) : null;
95+
$externalId = $this->externalId ?: $class;
96+
97+
$msg = sprintf('Service "%s" not found: ', $id);
98+
99+
if (!$this->container) {
100+
$class = null;
101+
} elseif ($this->container->has($id) || isset($this->container->getRemovedIds()[$id])) {
102+
$msg .= 'even though it exists in the app\'s container, ';
103+
} else {
104+
try {
105+
$this->container->get($id);
106+
$class = null;
107+
} catch (ServiceNotFoundException $e) {
108+
if ($e->getAlternatives()) {
109+
$msg .= sprintf(' did you mean %s? Anyway, ', $this->formatAlternatives($e->getAlternatives(), 'or'));
110+
} else {
111+
$class = null;
112+
}
113+
}
114+
}
115+
if ($externalId) {
116+
$msg .= sprintf('the container inside "%s" is a smaller service locator that %s', $externalId, $this->formatAlternatives());
117+
} else {
118+
$msg .= sprintf('the current service locator %s', $this->formatAlternatives());
119+
}
120+
121+
if (!$class) {
122+
// no-op
123+
} elseif (is_subclass_of($class, ServiceSubscriberInterface::class)) {
124+
$msg .= sprintf(' Unless you need extra laziness, try using dependency injection instead. Otherwise, you need to declare it using "%s::getSubscribedServices()".', preg_replace('/([^\\\\]++\\\\)++/', '', $class));
125+
} else {
126+
$msg .= 'Try using dependency injection instead.';
127+
}
128+
129+
return $msg;
130+
}
131+
132+
private function formatAlternatives(array $alternatives = null, $separator = 'and')
133+
{
134+
$format = '"%s"%s';
135+
if (null === $alternatives) {
136+
if (!$alternatives = array_keys($this->factories)) {
137+
return 'is empty...';
138+
}
139+
$format = sprintf('only knows about the %s service%s.', $format, 1 < count($alternatives) ? 's' : '');
140+
}
141+
$last = array_pop($alternatives);
142+
143+
return sprintf($format, $alternatives ? implode('", "', $alternatives) : $last, $alternatives ? sprintf(' %s "%s"', $separator, $last) : '');
144+
}
67145
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function testNoAttributes()
8585
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
8686
);
8787

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

9191
public function testWithAttributes()
@@ -115,7 +115,7 @@ public function testWithAttributes()
115115
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
116116
);
117117

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

121121
/**

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_subscriber.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public function getRemovedIds()
4545
'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
4646
'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\CustomDefinition' => true,
4747
'service_locator.jmktfsv' => true,
48+
'service_locator.jmktfsv.foo_service' => true,
4849
);
4950
}
5051

@@ -82,15 +83,15 @@ protected function getTestServiceSubscriberService()
8283
*/
8384
protected function getFooServiceService()
8485
{
85-
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 () {
86+
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 () {
8687
$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 ?: '_'});
8788
}, 'Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\TestServiceSubscriber' => function () {
8889
$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 ?: '_'});
8990
}, 'bar' => function () {
9091
$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 ?: '_'});
9192
}, 'baz' => function () {
9293
$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 ?: '_'});
93-
})));
94+
})), 'withContext'), 'foo_service', $this));
9495
}
9596

9697
/**

‎src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@
1212
namespace Symfony\Component\DependencyInjection\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
15+
use Symfony\Component\DependencyInjection\Container;
1616
use Symfony\Component\DependencyInjection\ServiceLocator;
17+
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
1718

1819
class ServiceLocatorTest extends TestCase
1920
{
@@ -59,7 +60,7 @@ public function testGetDoesNotMemoize()
5960

6061
/**
6162
* @expectedException \Psr\Container\NotFoundExceptionInterface
62-
* @expectedExceptionMessage You have requested a non-existent service "dummy". Did you mean one of these: "foo", "bar"?
63+
* @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.
6364
*/
6465
public function testGetThrowsOnUndefinedService()
6566
{
@@ -68,13 +69,50 @@ public function testGetThrowsOnUndefinedService()
6869
'bar' => function () { return 'baz'; },
6970
));
7071

71-
try {
72-
$locator->get('dummy');
73-
} catch (ServiceNotFoundException $e) {
74-
$this->assertSame(array('foo', 'bar'), $e->getAlternatives());
72+
$locator->get('dummy');
73+
}
74+
75+
/**
76+
* @expectedException \Psr\Container\NotFoundExceptionInterface
77+
* @expectedExceptionMessage The service "foo" has a dependency on a non-existent service "bar". This locator only knows about the "foo" service.
78+
*/
79+
public function testThrowsOnUndefinedInternalService()
80+
{
81+
$locator = new ServiceLocator(array(
82+
'foo' => function () use (&$locator) { return $locator->get('bar'); },
83+
));
84+
85+
$locator->get('foo');
86+
}
87+
88+
/**
89+
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException
90+
* @expectedExceptionMessage Circular reference detected for service "bar", path: "bar -> baz -> bar".
91+
*/
92+
public function testThrowsOnCircularReference()
93+
{
94+
$locator = new ServiceLocator(array(
95+
'foo' => function () use (&$locator) { return $locator->get('bar'); },
96+
'bar' => function () use (&$locator) { return $locator->get('baz'); },
97+
'baz' => function () use (&$locator) { return $locator->get('bar'); },
98+
));
7599

76-
throw $e;
77-
}
100+
$locator->get('foo');
101+
}
102+
103+
/**
104+
* @expectedException \Psr\Container\NotFoundExceptionInterface
105+
* @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()".
106+
*/
107+
public function testThrowsInServiceSubscriber()
108+
{
109+
$container = new Container();
110+
$container->set('foo', new \stdClass());
111+
$subscriber = new SomeServiceSubscriber();
112+
$subscriber->container = new ServiceLocator(array('bar' => function () {}));
113+
$subscriber->container = $subscriber->container->withContext('caller', $container);
114+
115+
$subscriber->getFoo();
78116
}
79117

80118
public function testInvoke()
@@ -89,3 +127,18 @@ public function testInvoke()
89127
$this->assertNull($locator('dummy'), '->__invoke() should return null on invalid service');
90128
}
91129
}
130+
131+
class SomeServiceSubscriber implements ServiceSubscriberinterface
132+
{
133+
public $container;
134+
135+
public function getFoo()
136+
{
137+
return $this->container->get('foo');
138+
}
139+
140+
public static function getSubscribedServices()
141+
{
142+
return array('bar' => 'stdClass');
143+
}
144+
}

0 commit comments

Comments
 (0)