Skip to content

Commit fe7f26d

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] Throw accurate failures when accessing removed services
1 parent b43bdf3 commit fe7f26d

33 files changed

+377
-77
lines changed

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

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class Container implements ResettableContainerInterface
5050
protected $aliases = array();
5151
protected $loading = array();
5252
protected $resolving = array();
53+
protected $syntheticIds = array();
5354

5455
/**
5556
* @internal
@@ -179,31 +180,34 @@ public function set($id, $service)
179180
throw new InvalidArgumentException('You cannot set service "service_container".');
180181
}
181182

182-
if (isset($this->aliases[$id])) {
183-
unset($this->aliases[$id]);
184-
}
185-
186-
$wasSet = isset($this->services[$id]);
187-
$this->services[$id] = $service;
188-
189-
if (null === $service) {
190-
unset($this->services[$id]);
191-
}
192-
193-
if (isset($this->privates[$id])) {
194-
if (null === $service) {
183+
if (isset($this->privates[$id]) || !(isset($this->fileMap[$id]) || isset($this->methodMap[$id]))) {
184+
if (isset($this->syntheticIds[$id]) || (!isset($this->privates[$id]) && !isset($this->getRemovedIds()[$id]))) {
185+
// no-op
186+
} elseif (null === $service) {
195187
@trigger_error(sprintf('The "%s" service is private, unsetting it is deprecated since Symfony 3.2 and will fail in 4.0.', $id), E_USER_DEPRECATED);
196188
unset($this->privates[$id]);
197189
} else {
198190
@trigger_error(sprintf('The "%s" service is private, replacing it is deprecated since Symfony 3.2 and will fail in 4.0.', $id), E_USER_DEPRECATED);
199191
}
200-
} elseif ($wasSet && (isset($this->fileMap[$id]) || isset($this->methodMap[$id]))) {
192+
} elseif (isset($this->services[$id])) {
201193
if (null === $service) {
202194
@trigger_error(sprintf('The "%s" service is already initialized, unsetting it is deprecated since Symfony 3.3 and will fail in 4.0.', $id), E_USER_DEPRECATED);
203195
} else {
204196
@trigger_error(sprintf('The "%s" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.', $id), E_USER_DEPRECATED);
205197
}
206198
}
199+
200+
if (isset($this->aliases[$id])) {
201+
unset($this->aliases[$id]);
202+
}
203+
204+
if (null === $service) {
205+
unset($this->services[$id]);
206+
207+
return;
208+
}
209+
210+
$this->services[$id] = $service;
207211
}
208212

209213
/**
@@ -275,7 +279,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
275279
// calling $this->normalizeId($id) unless necessary.
276280
for ($i = 2;;) {
277281
if (isset($this->privates[$id])) {
278-
@trigger_error(sprintf('The "%s" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.', $id), E_USER_DEPRECATED);
282+
@trigger_error(sprintf('The "%s" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.', $id), E_USER_DEPRECATED);
279283
}
280284
if (isset($this->aliases[$id])) {
281285
$id = $this->aliases[$id];
@@ -325,6 +329,12 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
325329
if (!$id) {
326330
throw new ServiceNotFoundException($id);
327331
}
332+
if (isset($this->syntheticIds[$id])) {
333+
throw new ServiceNotFoundException($id, null, null, array(), sprintf('The "%s" service is synthetic, it needs to be set at boot time before it can be used.', $id));
334+
}
335+
if (isset($this->getRemovedIds()[$id])) {
336+
throw new ServiceNotFoundException($id, null, null, array(), sprintf('The "%s" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.', $id));
337+
}
328338

329339
$alternatives = array();
330340
foreach ($this->getServiceIds() as $knownId) {
@@ -397,6 +407,16 @@ public function getServiceIds()
397407
return array_unique(array_merge($ids, array_keys($this->methodMap), array_keys($this->fileMap), array_keys($this->services)));
398408
}
399409

410+
/**
411+
* Gets service ids that existed at compile time.
412+
*
413+
* @return array
414+
*/
415+
public function getRemovedIds()
416+
{
417+
return array();
418+
}
419+
400420
/**
401421
* Camelizes a string.
402422
*

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
121121

122122
private $autoconfiguredInstanceof = array();
123123

124+
private $removedIds = array();
125+
124126
public function __construct(ParameterBagInterface $parameterBag = null)
125127
{
126128
parent::__construct($parameterBag);
@@ -517,7 +519,7 @@ public function set($id, $service)
517519
throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a compiled container is not allowed.', $id));
518520
}
519521

520-
unset($this->definitions[$id], $this->aliasDefinitions[$id]);
522+
unset($this->definitions[$id], $this->aliasDefinitions[$id], $this->removedIds[$id]);
521523

522524
parent::set($id, $service);
523525
}
@@ -529,7 +531,10 @@ public function set($id, $service)
529531
*/
530532
public function removeDefinition($id)
531533
{
532-
unset($this->definitions[$this->normalizeId($id)]);
534+
if (isset($this->definitions[$id = $this->normalizeId($id)])) {
535+
unset($this->definitions[$id]);
536+
$this->removedIds[$id] = true;
537+
}
533538
}
534539

535540
/**
@@ -793,6 +798,16 @@ public function getServiceIds()
793798
return array_unique(array_merge(array_keys($this->getDefinitions()), array_keys($this->aliasDefinitions), parent::getServiceIds()));
794799
}
795800

801+
/**
802+
* Gets removed service or alias ids.
803+
*
804+
* @return array
805+
*/
806+
public function getRemovedIds()
807+
{
808+
return $this->removedIds;
809+
}
810+
796811
/**
797812
* Adds the service aliases.
798813
*
@@ -841,7 +856,7 @@ public function setAlias($alias, $id)
841856
throw new InvalidArgumentException(sprintf('An alias can not reference itself, got a circular reference on "%s".', $alias));
842857
}
843858

844-
unset($this->definitions[$alias]);
859+
unset($this->definitions[$alias], $this->removedIds[$alias]);
845860

846861
return $this->aliasDefinitions[$alias] = $id;
847862
}
@@ -853,7 +868,10 @@ public function setAlias($alias, $id)
853868
*/
854869
public function removeAlias($alias)
855870
{
856-
unset($this->aliasDefinitions[$this->normalizeId($alias)]);
871+
if (isset($this->aliasDefinitions[$alias = $this->normalizeId($alias)])) {
872+
unset($this->aliasDefinitions[$alias]);
873+
$this->removedIds[$alias] = true;
874+
}
857875
}
858876

859877
/**
@@ -981,7 +999,7 @@ public function setDefinition($id, Definition $definition)
981999

9821000
$id = $this->normalizeId($id);
9831001

984-
unset($this->aliasDefinitions[$id]);
1002+
unset($this->aliasDefinitions[$id], $this->removedIds[$id]);
9851003

9861004
return $this->definitions[$id] = $definition;
9871005
}

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,16 @@ public function dump(array $options = array())
172172
173173
EOF;
174174
$files = array();
175+
176+
if ($ids = array_keys($this->container->getRemovedIds())) {
177+
sort($ids);
178+
$c = "<?php\n\nreturn array(\n";
179+
foreach ($ids as $id) {
180+
$c .= ' '.$this->export($id)." => true,\n";
181+
}
182+
$files['removed-ids.php'] = $c .= ");\n";
183+
}
184+
175185
foreach ($this->generateServiceFiles() as $file => $c) {
176186
$files[$file] = $fileStart.$c;
177187
}
@@ -888,6 +898,7 @@ public function __construct()
888898
}
889899

890900
$code .= $this->addNormalizedIds();
901+
$code .= $this->addSyntheticIds();
891902
$code .= $this->addMethodMap();
892903
$code .= $this->asFiles ? $this->addFileMap() : '';
893904
$code .= $this->addPrivateServices();
@@ -896,6 +907,8 @@ public function __construct()
896907
}
897908

898909
EOF;
910+
$code .= $this->addRemovedIds();
911+
899912
if ($this->container->isCompiled()) {
900913
$code .= <<<EOF
901914
@@ -978,6 +991,58 @@ private function addNormalizedIds()
978991
return $code ? " \$this->normalizedIds = array(\n".$code." );\n" : '';
979992
}
980993

994+
/**
995+
* Adds the syntheticIds definition.
996+
*
997+
* @return string
998+
*/
999+
private function addSyntheticIds()
1000+
{
1001+
$code = '';
1002+
$definitions = $this->container->getDefinitions();
1003+
ksort($definitions);
1004+
foreach ($definitions as $id => $definition) {
1005+
if ($definition->isSynthetic() && 'service_container' !== $id) {
1006+
$code .= ' '.$this->export($id)." => true,\n";
1007+
}
1008+
}
1009+
1010+
return $code ? " \$this->syntheticIds = array(\n{$code} );\n" : '';
1011+
}
1012+
1013+
/**
1014+
* Adds the removedIds definition.
1015+
*
1016+
* @return string
1017+
*/
1018+
private function addRemovedIds()
1019+
{
1020+
if (!$ids = $this->container->getRemovedIds()) {
1021+
return '';
1022+
}
1023+
if ($this->asFiles) {
1024+
$code = "require __DIR__.'/removed-ids.php'";
1025+
} else {
1026+
$code = '';
1027+
$ids = array_keys($ids);
1028+
sort($ids);
1029+
foreach ($ids as $id) {
1030+
$code .= ' '.$this->export($id)." => true,\n";
1031+
}
1032+
1033+
$code = "array(\n{$code} )";
1034+
}
1035+
1036+
return <<<EOF
1037+
1038+
public function getRemovedIds()
1039+
{
1040+
return {$code};
1041+
}
1042+
1043+
EOF;
1044+
}
1045+
9811046
/**
9821047
* Adds the methodMap property definition.
9831048
*

‎src/Symfony/Component/DependencyInjection/Exception/ServiceNotFoundException.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ class ServiceNotFoundException extends InvalidArgumentException implements NotFo
2424
private $sourceId;
2525
private $alternatives;
2626

27-
public function __construct($id, $sourceId = null, \Exception $previous = null, array $alternatives = array())
27+
public function __construct($id, $sourceId = null, \Exception $previous = null, array $alternatives = array(), $msg = null)
2828
{
29-
if (null === $sourceId) {
29+
if (null !== $msg) {
30+
// no-op
31+
} elseif (null === $sourceId) {
3032
$msg = sprintf('You have requested a non-existent service "%s".', $id);
3133
} else {
3234
$msg = sprintf('The service "%s" has a dependency on a non-existent service "%s".', $sourceId, $id);

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,16 +333,28 @@ public function testGetCircularReference()
333333

334334
/**
335335
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
336-
* @expectedExceptionMessage You have requested a non-existent service "request".
336+
* @expectedExceptionMessage The "request" service is synthetic, it needs to be set at boot time before it can be used.
337337
*/
338338
public function testGetSyntheticServiceThrows()
339339
{
340-
require_once __DIR__.'/Fixtures/php/services9.php';
340+
require_once __DIR__.'/Fixtures/php/services9_compiled.php';
341341

342342
$container = new \ProjectServiceContainer();
343343
$container->get('request');
344344
}
345345

346+
/**
347+
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
348+
* @expectedExceptionMessage The "inlined" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.
349+
*/
350+
public function testGetRemovedServiceThrows()
351+
{
352+
require_once __DIR__.'/Fixtures/php/services9_compiled.php';
353+
354+
$container = new \ProjectServiceContainer();
355+
$container->get('inlined');
356+
}
357+
346358
public function testHas()
347359
{
348360
$sc = new ProjectServiceContainer();
@@ -505,7 +517,7 @@ public function testCheckExistenceOfAnInternalPrivateServiceIsDeprecated()
505517

506518
/**
507519
* @group legacy
508-
* @expectedDeprecation The "internal" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
520+
* @expectedDeprecation The "internal" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
509521
*/
510522
public function testRequestAnInternalSharedPrivateServiceIsDeprecated()
511523
{

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -287,37 +287,17 @@ public function testFrozenContainerWithoutAliases()
287287

288288
/**
289289
* @group legacy
290-
* @expectedDeprecation The "bar" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
290+
* @expectedDeprecation The "decorator_service" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
291291
*/
292292
public function testOverrideServiceWhenUsingADumpedContainer()
293293
{
294-
require_once self::$fixturesPath.'/php/services9.php';
295-
require_once self::$fixturesPath.'/includes/foo.php';
294+
require_once self::$fixturesPath.'/php/services9_compiled.php';
296295

297296
$container = new \ProjectServiceContainer();
298-
$container->setParameter('foo_bar', 'foo_bar');
299-
$container->get('bar');
300-
$container->set('bar', $bar = new \stdClass());
301-
302-
$this->assertSame($bar, $container->get('bar'), '->set() overrides an already defined service');
303-
}
304-
305-
/**
306-
* @group legacy
307-
* @expectedDeprecation The "bar" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
308-
*/
309-
public function testOverrideServiceWhenUsingADumpedContainerAndServiceIsUsedFromAnotherOne()
310-
{
311-
require_once self::$fixturesPath.'/php/services9.php';
312-
require_once self::$fixturesPath.'/includes/foo.php';
313-
require_once self::$fixturesPath.'/includes/classes.php';
314-
315-
$container = new \ProjectServiceContainer();
316-
$container->setParameter('foo_bar', 'foo_bar');
317-
$container->get('bar');
318-
$container->set('bar', $bar = new \stdClass());
297+
$container->get('decorator_service');
298+
$container->set('decorator_service', $decorator = new \stdClass());
319299

320-
$this->assertSame($bar, $container->get('foo')->bar, '->set() overrides an already defined service');
300+
$this->assertSame($decorator, $container->get('decorator_service'), '->set() overrides an already defined service');
321301
}
322302

323303
/**
@@ -799,14 +779,14 @@ public function testDumpHandlesLiteralClassWithRootNamespace()
799779

800780
/**
801781
* @group legacy
802-
* @expectedDeprecation The "private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
803-
* @expectedDeprecation The "private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
804-
* @expectedDeprecation The "decorated_private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
805-
* @expectedDeprecation The "decorated_private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
806-
* @expectedDeprecation The "private_not_inlined" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
807-
* @expectedDeprecation The "private_not_removed" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
808-
* @expectedDeprecation The "private_child" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
809-
* @expectedDeprecation The "private_parent" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
782+
* @expectedDeprecation The "private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
783+
* @expectedDeprecation The "private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
784+
* @expectedDeprecation The "decorated_private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
785+
* @expectedDeprecation The "decorated_private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
786+
* @expectedDeprecation The "private_not_inlined" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
787+
* @expectedDeprecation The "private_not_removed" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
788+
* @expectedDeprecation The "private_child" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
789+
* @expectedDeprecation The "private_parent" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
810790
*/
811791
public function testLegacyPrivateServices()
812792
{

0 commit comments

Comments
 (0)