Skip to content

Commit abb9132

Browse files
fabpotfabpot
committed
bug symfony#25756 [TwigBundle] Register TwigBridge extensions first (fancyweb)
This PR was squashed before being merged into the 3.4 branch (closes symfony#25756). Discussion ---------- [TwigBundle] Register TwigBridge extensions first | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#25610 | License | MIT | Doc PR | - The only extension that is really needed to display the current exception page is the `CodeExtension` so we could only prepend this one. However, prepending all of them seems safer to me in the long term. Also I deeply looked into why this problem only appeared in 3.4 and found the reason. Before 3.4 it actually never reaches the `ExceptionController` for this kind of error because it cannot be resolved because it needs a twig instance in its constructor. This instance is directly taken from the container. Before 3.4 when an exception is thrown when you try to get a service from the container, the instance stored in the `$services` array is unset which is not the case in further versions. So in 3.4+, the `ExceptionController` can be resolved because the instance of twig is still in the container even after the initial exception. It also means these kind of exceptions are displayed with bugs on all versions before 3.4 I guess. Actually it shows the message 2 times : one for the initial exception and the other one when it tries to resolve the `ExceptionController`. Maybe another solution might be to use a dedicated twig instance with the right settings just for the exception page ? Commits ------- c8465ed [TwigBundle] Register TwigBridge extensions first
2 parents b1bf41a + c8465ed commit abb9132

File tree

2 files changed

+104
-4
lines changed

2 files changed

+104
-4
lines changed

‎src/Symfony/Bundle/TwigBundle/DependencyInjection/Compiler/TwigEnvironmentPass.php

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,22 @@ public function process(ContainerBuilder $container)
3434
// For instance, global variable definitions must be registered
3535
// afterward. If not, the globals from the extensions will never
3636
// be registered.
37-
$calls = $definition->getMethodCalls();
38-
$definition->setMethodCalls(array());
37+
$currentMethodCalls = $definition->getMethodCalls();
38+
$twigBridgeExtensionsMethodCalls = array();
39+
$othersExtensionsMethodCalls = array();
3940
foreach ($container->findTaggedServiceIds('twig.extension', true) as $id => $attributes) {
40-
$definition->addMethodCall('addExtension', array(new Reference($id)));
41+
$methodCall = array('addExtension', array(new Reference($id)));
42+
$extensionClass = $container->getDefinition($id)->getClass();
43+
44+
if (is_string($extensionClass) && 0 === strpos($extensionClass, 'Symfony\Bridge\Twig\Extension')) {
45+
$twigBridgeExtensionsMethodCalls[] = $methodCall;
46+
} else {
47+
$othersExtensionsMethodCalls[] = $methodCall;
48+
}
49+
}
50+
51+
if (!empty($twigBridgeExtensionsMethodCalls) || !empty($othersExtensionsMethodCalls)) {
52+
$definition->setMethodCalls(array_merge($twigBridgeExtensionsMethodCalls, $othersExtensionsMethodCalls, $currentMethodCalls));
4153
}
42-
$definition->setMethodCalls(array_merge($definition->getMethodCalls(), $calls));
4354
}
4455
}

‎src/Symfony/Bundle/TwigBundle/Tests/DependencyInjection/Compiler/TwigEnvironmentPassTest.php

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\TwigBundle\Tests\DependencyInjection\Compiler;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bundle\TwigBundle\DependencyInjection\Compiler\TwigEnvironmentPass;
16+
use Symfony\Component\DependencyInjection\ContainerBuilder;
17+
use Symfony\Component\DependencyInjection\Definition;
18+
use Symfony\Component\DependencyInjection\Reference;
19+
20+
class TwigEnvironmentPassTest extends TestCase
21+
{
22+
public function testTwigBridgeExtensionsAreRegisteredFirst()
23+
{
24+
$twigDefinition = new Definition('twig');
25+
26+
$containerBuilderMock = $this->getMockBuilder(ContainerBuilder::class)
27+
->setMethods(array('hasDefinition', 'get', 'findTaggedServiceIds', 'getDefinition'))
28+
->getMock();
29+
$containerBuilderMock
30+
->expects($this->once())
31+
->method('hasDefinition')
32+
->with('twig')
33+
->will($this->returnValue(true));
34+
$containerBuilderMock
35+
->expects($this->once())
36+
->method('findTaggedServiceIds')
37+
->with('twig.extension')
38+
->will($this->returnValue(array(
39+
'other_extension' => array(
40+
array()
41+
),
42+
'twig_bridge_extension' => array(
43+
array()
44+
)
45+
)));
46+
47+
$otherExtensionDefinitionMock = $this->getMockBuilder(Definition::class)
48+
->setMethods(array('getClass'))
49+
->getMock();
50+
$otherExtensionDefinitionMock
51+
->expects($this->once())
52+
->method('getClass')
53+
->will($this->returnValue('Foo\\Bar'));
54+
55+
$twigExtensionDefinitionMock = $this->getMockBuilder(Definition::class)
56+
->setMethods(array('getClass'))
57+
->getMock();
58+
$twigExtensionDefinitionMock
59+
->expects($this->once())
60+
->method('getClass')
61+
->will($this->returnValue('Symfony\\Bridge\\Twig\\Extension\\Foo'));
62+
63+
$containerBuilderMock
64+
->expects($this->exactly(3))
65+
->method('getDefinition')
66+
->withConsecutive(array('twig'), array('other_extension'), array('twig_bridge_extension'))
67+
->willReturnOnConsecutiveCalls(
68+
$this->returnValue($twigDefinition),
69+
$this->returnValue($otherExtensionDefinitionMock),
70+
$this->returnValue($twigExtensionDefinitionMock)
71+
);
72+
73+
$twigEnvironmentPass = new TwigEnvironmentPass();
74+
$twigEnvironmentPass->process($containerBuilderMock);
75+
76+
$methodCalls = $twigDefinition->getMethodCalls();
77+
$this->assertCount(2, $methodCalls);
78+
79+
$twigBridgeExtensionReference = $methodCalls[0][1][0];
80+
$this->assertInstanceOf(Reference::class, $twigBridgeExtensionReference);
81+
/* @var Reference $twigBridgeExtensionReference */
82+
$this->assertEquals('twig_bridge_extension', $twigBridgeExtensionReference->__toString());
83+
84+
$otherExtensionReference = $methodCalls[1][1][0];
85+
$this->assertInstanceOf(Reference::class, $otherExtensionReference);
86+
/* @var Reference $otherExtensionReference */
87+
$this->assertEquals('other_extension', $otherExtensionReference->__toString());
88+
}
89+
}

0 commit comments

Comments
 (0)