Skip to content

Commit

Permalink
bug #35261 [Routing] Fix using a custom matcher & generator dumper cl…
Browse files Browse the repository at this point in the history
…ass (fancyweb)

This PR was merged into the 4.3 branch.

Discussion
----------

[Routing] Fix using a custom matcher & generator dumper class

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

This PR fixes a BC break I encountered while upgrading an existing project from 4.2 to 4.4. In this project I use a custom `generator_dumper_class` that is not a `CompiledUrlGeneratorDumper` (it didn't exist yet). I faced 2 problems:
- The generator is considered "compiled" while it is not. This is because we don't check if the `generator_dumper_class` is effectively a `CompiledUrlGeneratorDumper` to compute the `$compiled` variable. That result in a `\TypeError: Return value of Symfony\Component\Routing\Router::getCompiledRoutes() must be of the type array, int returned`
- My custom dumper is not used at all. This is because of #31964. I altered the condition to fall back only in one way and not the other. The original issue is still fixed (if one uses a classic `UrlGenerator` + a `CompiledUrlGeneratorDumper`, it fall backs on `PhpGeneratorDumper`). However, if one uses a `CompiledUrlGenerator` + a classic `PhpGeneratorDumper` (my case), the classic dumper is still returned. Since `$compiled` is now correctly computed, this case works fine. The Router won't try to get the compiled routes and will use the "old" way.

Commits
-------

3a840a9 [Routing] Fix using a custom matcher & generator dumper class
  • Loading branch information
nicolas-grekas committed Jan 8, 2020
2 parents d638161 + 3a840a9 commit 9b11c36
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/Symfony/Component/Routing/Router.php
Expand Up @@ -291,7 +291,7 @@ public function getMatcher()
return $this->matcher;
}

$compiled = is_a($this->options['matcher_class'], CompiledUrlMatcher::class, true) && (UrlMatcher::class === $this->options['matcher_base_class'] || RedirectableUrlMatcher::class === $this->options['matcher_base_class']);
$compiled = is_a($this->options['matcher_class'], CompiledUrlMatcher::class, true) && (UrlMatcher::class === $this->options['matcher_base_class'] || RedirectableUrlMatcher::class === $this->options['matcher_base_class']) && is_a($this->options['matcher_dumper_class'], CompiledUrlMatcherDumper::class, true);

if (null === $this->options['cache_dir'] || null === $this->options['matcher_cache_class']) {
$routes = $this->getRouteCollection();
Expand Down Expand Up @@ -348,7 +348,7 @@ public function getGenerator()
return $this->generator;
}

$compiled = is_a($this->options['generator_class'], CompiledUrlGenerator::class, true) && UrlGenerator::class === $this->options['generator_base_class'];
$compiled = is_a($this->options['generator_class'], CompiledUrlGenerator::class, true) && UrlGenerator::class === $this->options['generator_base_class'] && is_a($this->options['generator_dumper_class'], CompiledUrlGeneratorDumper::class, true);

if (null === $this->options['cache_dir'] || null === $this->options['generator_cache_class']) {
$routes = $this->getRouteCollection();
Expand Down Expand Up @@ -398,8 +398,8 @@ public function addExpressionLanguageProvider(ExpressionFunctionProviderInterfac
*/
protected function getGeneratorDumperInstance()
{
// For BC, fallback to PhpGeneratorDumper if the UrlGenerator and UrlGeneratorDumper are not consistent with each other
if (is_a($this->options['generator_class'], CompiledUrlGenerator::class, true) !== is_a($this->options['generator_dumper_class'], CompiledUrlGeneratorDumper::class, true)) {
// For BC, fallback to PhpGeneratorDumper (which is the old default value) if the old UrlGenerator is used with the new default CompiledUrlGeneratorDumper
if (!is_a($this->options['generator_class'], CompiledUrlGenerator::class, true) && is_a($this->options['generator_dumper_class'], CompiledUrlGeneratorDumper::class, true)) {
return new PhpGeneratorDumper($this->getRouteCollection());
}

Expand All @@ -411,8 +411,8 @@ protected function getGeneratorDumperInstance()
*/
protected function getMatcherDumperInstance()
{
// For BC, fallback to PhpMatcherDumper if the UrlMatcher and UrlMatcherDumper are not consistent with each other
if (is_a($this->options['matcher_class'], CompiledUrlMatcher::class, true) !== is_a($this->options['matcher_dumper_class'], CompiledUrlMatcherDumper::class, true)) {
// For BC, fallback to PhpMatcherDumper (which is the old default value) if the old UrlMatcher is used with the new default CompiledUrlMatcherDumper
if (!is_a($this->options['matcher_class'], CompiledUrlMatcher::class, true) && is_a($this->options['matcher_dumper_class'], CompiledUrlMatcherDumper::class, true)) {
return new PhpMatcherDumper($this->getRouteCollection());
}

Expand Down

0 comments on commit 9b11c36

Please sign in to comment.