Skip to content

Commit dd82f76

Browse files
weaverryanweaverryan
committed
Fixing a bug where a removed service might still cause an autowiring exception to be thrown
1 parent 5473373 commit dd82f76

File tree

4 files changed

+79
-12
lines changed

4 files changed

+79
-12
lines changed

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,31 @@ public function process(ContainerBuilder $container)
3636
return;
3737
}
3838

39-
$inlinedIds = $this->inlineServicePass->getInlinedServiceIds();
39+
$inlinedIds = $this->inlineServicePass->getInlinedServiceIdData();
4040
$exceptions = $this->autowirePass->getAutowiringExceptions();
4141

4242
// free up references
4343
$this->autowirePass = null;
4444
$this->inlineServicePass = null;
4545

4646
foreach ($exceptions as $exception) {
47-
if ($container->hasDefinition($exception->getServiceId()) || in_array($exception->getServiceId(), $inlinedIds)) {
47+
if ($this->doesServiceExistInTheContainer($exception->getServiceId(), $container, $inlinedIds)) {
4848
throw $exception;
4949
}
5050
}
5151
}
52+
53+
private function doesServiceExistInTheContainer($serviceId, ContainerBuilder $container, array $inlinedIds)
54+
{
55+
if ($container->hasDefinition($serviceId)) {
56+
return true;
57+
}
58+
59+
// was the service inlined? Of so, does its parent service exist?
60+
if (isset($inlinedIds[$serviceId])) {
61+
return $this->doesServiceExistInTheContainer($inlinedIds[$serviceId], $container, $inlinedIds);
62+
}
63+
64+
return false;
65+
}
5266
}

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,24 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
3737
* Returns an array of all services inlined by this pass.
3838
*
3939
* @return array Service id strings
40+
*
41+
* @deprecated This method is deprecated as of 3.3 and will be removed in 4.0. Use getInlinedServiceIdData() instead.
4042
*/
4143
public function getInlinedServiceIds()
44+
{
45+
@trigger_error(sprintf('%s is deprecated as of 3.3 and will be removed in 4.0. Use getInlinedServiceIdData() instead.', __METHOD__), E_USER_DEPRECATED);
46+
47+
return array_keys($this->inlinedServiceIds);
48+
}
49+
50+
/**
51+
* Returns an array of all services inlined by this pass.
52+
*
53+
* The key is the inlined service id and its value is the service it was inlined into.
54+
*
55+
* @return array
56+
*/
57+
public function getInlinedServiceIdData()
4258
{
4359
return $this->inlinedServiceIds;
4460
}
@@ -57,7 +73,7 @@ protected function processValue($value, $isRoot = false)
5773

5874
if ($this->isInlineableDefinition($id, $definition, $this->container->getCompiler()->getServiceReferenceGraph())) {
5975
$this->container->log($this, sprintf('Inlined service "%s" to "%s".', $id, $this->currentId));
60-
$this->inlinedServiceIds[] = $id;
76+
$this->inlinedServiceIds[$id] = $this->currentId;
6177

6278
if ($definition->isShared()) {
6379
return $definition;

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

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function testThrowsException()
3333
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
3434
->getMock();
3535
$inlinePass->expects($this->any())
36-
->method('getInlinedServiceIds')
36+
->method('getInlinedServiceIdData')
3737
->will($this->returnValue(array()));
3838

3939
$container = new ContainerBuilder();
@@ -54,19 +54,25 @@ public function testThrowExceptionIfServiceInlined()
5454
$autowirePass = $this->getMockBuilder(AutowirePass::class)
5555
->getMock();
5656

57-
$autowireException = new AutowiringFailedException('foo_service_id', 'An autowiring exception message');
57+
$autowireException = new AutowiringFailedException('a_service', 'An autowiring exception message');
5858
$autowirePass->expects($this->any())
5959
->method('getAutowiringExceptions')
6060
->will($this->returnValue(array($autowireException)));
6161

6262
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
6363
->getMock();
6464
$inlinePass->expects($this->any())
65-
->method('getInlinedServiceIds')
66-
->will($this->returnValue(array('foo_service_id')));
65+
->method('getInlinedServiceIdData')
66+
->will($this->returnValue(array(
67+
// a_service inlined into b_service
68+
'a_service' => 'b_service',
69+
// b_service inlined into c_service
70+
'b_service' => 'c_service',
71+
)));
6772

68-
// don't register the foo_service_id service
6973
$container = new ContainerBuilder();
74+
// ONLY register c_service in the final container
75+
$container->register('c_service', 'stdClass');
7076

7177
$pass = new AutowireExceptionPass($autowirePass, $inlinePass);
7278

@@ -78,6 +84,37 @@ public function testThrowExceptionIfServiceInlined()
7884
}
7985
}
8086

87+
public function testDoNotThrowExceptionIfServiceInlinedButRemoved()
88+
{
89+
$autowirePass = $this->getMockBuilder(AutowirePass::class)
90+
->getMock();
91+
92+
$autowireException = new AutowiringFailedException('a_service', 'An autowiring exception message');
93+
$autowirePass->expects($this->any())
94+
->method('getAutowiringExceptions')
95+
->will($this->returnValue(array($autowireException)));
96+
97+
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
98+
->getMock();
99+
$inlinePass->expects($this->any())
100+
->method('getInlinedServiceIdData')
101+
->will($this->returnValue(array(
102+
// a_service inlined into b_service
103+
'a_service' => 'b_service',
104+
// b_service inlined into c_service
105+
'b_service' => 'c_service',
106+
)));
107+
108+
// do NOT register c_service in the container
109+
$container = new ContainerBuilder();
110+
111+
$pass = new AutowireExceptionPass($autowirePass, $inlinePass);
112+
113+
$pass->process($container);
114+
// mark the test as passed
115+
$this->assertTrue(true);
116+
}
117+
81118
public function testNoExceptionIfServiceRemoved()
82119
{
83120
$autowirePass = $this->getMockBuilder(AutowirePass::class)
@@ -91,7 +128,7 @@ public function testNoExceptionIfServiceRemoved()
91128
$inlinePass = $this->getMockBuilder(InlineServiceDefinitionsPass::class)
92129
->getMock();
93130
$inlinePass->expects($this->any())
94-
->method('getInlinedServiceIds')
131+
->method('getInlinedServiceIdData')
95132
->will($this->returnValue(array()));
96133

97134
$container = new ContainerBuilder();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ public function testProcessDoesNotSetLazyArgumentValuesAfterInlining()
252252
$this->assertSame('inline', (string) $values[0]);
253253
}
254254

255-
public function testGetInlinedServiceIds()
255+
public function testGetInlinedServiceIdData()
256256
{
257257
$container = new ContainerBuilder();
258258
$container
@@ -265,15 +265,15 @@ public function testGetInlinedServiceIds()
265265
;
266266

267267
$container
268-
->register('service')
268+
->register('other_service')
269269
->setArguments(array(new Reference('inlinable.service')))
270270
;
271271

272272
$inlinePass = new InlineServiceDefinitionsPass();
273273
$repeatedPass = new RepeatedPass(array(new AnalyzeServiceReferencesPass(), $inlinePass));
274274
$repeatedPass->process($container);
275275

276-
$this->assertEquals(array('inlinable.service'), $inlinePass->getInlinedServiceIds());
276+
$this->assertEquals(array('inlinable.service' => 'other_service'), $inlinePass->getInlinedServiceIdData());
277277
}
278278

279279
protected function process(ContainerBuilder $container)

0 commit comments

Comments
 (0)