Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Jan 8, 2020

Conversation

@fancyweb
Copy link
Contributor

fancyweb commented Jan 8, 2020

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.
@fancyweb

This comment has been minimized.

Copy link
Contributor Author

fancyweb commented Jan 8, 2020

I applied the same patch logic on the matcher btw because looking at the code, I guess it suffers the same problem.

@yceruto yceruto added this to the 4.3 milestone Jan 8, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 8, 2020

Thank you @fancyweb.

nicolas-grekas added a commit that referenced this pull request Jan 8, 2020
…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
@nicolas-grekas nicolas-grekas merged commit 3a840a9 into symfony:4.3 Jan 8, 2020
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@fancyweb fancyweb deleted the fancyweb:routing-compiled-fix branch Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.