diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #34192 [Routing] Fix URL generator instantiation (X-Coder264, HypeMC)

Browse Source 
This PR was merged into the 4.3 branch.

Discussion
----------

[Routing] Fix URL generator instantiation

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

After merging my fix (PR https://github.com/symfony/symfony/pull/31023) from the 4.2 branch to 4.3 (which was master at the time) one code path which instantiates the url generator was missed. This new code path was added via https://github.com/symfony/symfony/pull/28865 and was missed during the merge because there was no merge conflict there as that code path didn't exist on 4.2.

So the current state on the 4.3 branch is:

- https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L356 (default locale is properly passed to the constructor)

- https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L372 (default locale is **NOT** properly passed to the constructor)

- https://github.com/symfony/symfony/blob/v4.3.5/src/Symfony/Component/Routing/Router.php#L378 (default locale is properly passed to the constructor)

I noticed this in my app because tests started to fail on the branch where we are upgrading our app from SF 4.2 to 4.3 due to this. This PR fixes the inconsistency (now the default locale is passed on all three places) which fixes the bug.

Commits
-------

9aa66e2f59 Add tests to ensure defaultLocale is properly passed to the URL generator
16c9bafee4 Fix URL generator instantiation
This commit is contained in:
Nicolas Grekas 2019-11-04 21:23:21 +01:00
commit a56ac78269
2 changed files with 81 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Component/Routing/Router.php
View File

@ -369,7 +369,7 @@ class Router implements RouterInterface, RequestMatcherInterface
);
if ($compiled) {
$this->generator = new $this->options['generator_class'](require $cache->getPath(), $this->context, $this->logger);
$this->generator = new $this->options['generator_class'](require $cache->getPath(), $this->context, $this->logger, $this->defaultLocale);
} else {
if (!class_exists($this->options['generator_cache_class'], false)) {
require_once $cache->getPath();

80
src/Symfony/Component/Routing/Tests/RouterTest.php
View File

@ -22,10 +22,26 @@ class RouterTest extends TestCase
private $loader = null;
private $cacheDir;
protected function setUp(): void
{
$this->loader = $this->getMockBuilder('Symfony\Component\Config\Loader\LoaderInterface')->getMock();
$this->router = new Router($this->loader, 'routing.yml');
$this->cacheDir = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('router_', true);
}
protected function tearDown(): void
{
if (is_dir($this->cacheDir)) {
array_map('unlink', glob($this->cacheDir.\DIRECTORY_SEPARATOR.'*'));
rmdir($this->cacheDir);
}
$this->loader = null;
$this->router = null;
$this->cacheDir = null;
}
public function testSetOptionsWithSupportedOptions()
@ -132,4 +148,68 @@ class RouterTest extends TestCase
$this->router->matchRequest(Request::create('/'));
}
public function testDefaultLocaleIsPassedToGeneratorClass()
{
$this->loader->expects($this->once())
->method('load')->with('routing.yml', null)
->willReturn(new RouteCollection());
$router = new Router($this->loader, 'routing.yml', [
'cache_dir' => null,
], null, null, 'hr');
$generator = $router->getGenerator();
$this->assertInstanceOf('Symfony\Component\Routing\Generator\UrlGeneratorInterface', $generator);
$p = new \ReflectionProperty($generator, 'defaultLocale');
$p->setAccessible(true);
$this->assertSame('hr', $p->getValue($generator));
}
public function testDefaultLocaleIsPassedToCompiledGeneratorCacheClass()
{
$this->loader->expects($this->once())
->method('load')->with('routing.yml', null)
->willReturn(new RouteCollection());
$router = new Router($this->loader, 'routing.yml', [
'cache_dir' => $this->cacheDir,
], null, null, 'hr');
$generator = $router->getGenerator();
$this->assertInstanceOf('Symfony\Component\Routing\Generator\UrlGeneratorInterface', $generator);
$p = new \ReflectionProperty($generator, 'defaultLocale');
$p->setAccessible(true);
$this->assertSame('hr', $p->getValue($generator));
}
/**
* @group legacy
*/
public function testDefaultLocaleIsPassedToNotCompiledGeneratorCacheClass()
{
$this->loader->expects($this->once())
->method('load')->with('routing.yml', null)
->willReturn(new RouteCollection());
$router = new Router($this->loader, 'routing.yml', [
'cache_dir' => $this->cacheDir,
'generator_class' => 'Symfony\Component\Routing\Generator\UrlGenerator',
], null, null, 'hr');
$generator = $router->getGenerator();
$this->assertInstanceOf('Symfony\Component\Routing\Generator\UrlGeneratorInterface', $generator);
$p = new \ReflectionProperty($generator, 'defaultLocale');
$p->setAccessible(true);
$this->assertSame('hr', $p->getValue($generator));
}
}