8000 Fixing a bug where a removed service might still cause an autowiring … · symfony/symfony@3375e6b · GitHub
[go: up one dir, main page]
Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3375e6b

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

File tree

4 files changed

+78
-12
lines changed

4 files changed

+78
-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: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,23 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
3737
* Returns an array of all services inlined by this pass.
3838
*
3939
* @return array Service id strings
40+
* @deprecated This method is deprecated as of 3.3 and will be removed in 4.0. Use getInlinedServiceIdData() instead.
4041
*/
4142
public function getInlinedServiceIds()
43+
{
44+
@trigger_error(sprintf('%s is deprecated as of 3.3 and will be removed in 4.0. Use getInlinedServiceIdData() instead.', __METHOD__), E_USER_DEPRECATED);
45+
46+
return array_keys($this->inlinedServiceIds);
47+
}
48+
49+
/**
50+
* Returns an array of all services inlined by this pass.
51+
*
52+
* The key is the inlined service id and its value is the service it was inlined into.
53+
*
54+
* @return array
55+
*/
56+
public function getInlinedServiceIdData()
4257
{
4358
return $this->inlinedServiceIds;
4459
}
@@ -57,7 +72,7 @@ protected function processValue($value, $isRoot = false)
5772

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

6277
if ($definition->isShared()) {
6378
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)
0