symfony-cmf / routing-bundle

Symfony bundle to provide the CMF chain router to handle multiple routers, and the dynamic router to load routes from a database or other sources.
160 stars 78 forks source link

ORM: _route attribute of request set to ID rather than to name #478

Closed dbojdo closed 2 years ago

dbojdo commented 2 years ago

Environment

PHP 8.1

Symfony packages

symfony/* 6.0.*

Symfony CMF packages

symfony-cmf/routing        3.0.0 3.0.0 Extends the Symfony routing component for dynamic routes and chaining several routers
symfony-cmf/routing-bundle 3.0.0 3.0.0 Symfony RoutingBundle

Subject

When ORM implementation of the Route entity is used, the "_route" attribute of the Request object is set to the numeric ID of the entity (e.g. "5"). What makes impossible to use this attribute along with the URL generator (cannot generate the URL from the ID, it must be the name).

After investigation I spotted it's due to the fact, that Symfony\Cmf\Bundle\RoutingBundle\Model::getId() always returns string. Before there was no casting to string. The above with the if statement in Symfony\Cmf\Component\Routing\NestedMatcher::getAttributes (is_string($route->getRouteKey())) replaces the route name with it's numeric ID.

Steps to reproduce

Read the "_route" attribute of Request object when ORM used.

Expected results

"_route" attribute should contain the route name

Actual results

"_route" attribute contains the numeric ID of the Route

Proposed solution is to override getRouteKey method of Symfony\Cmf\Bundle\RoutingBundle\Doctrine\Orm\Route to return the name rather than ID.

dbojdo commented 2 years ago

Along with this change we might want to remove unnecessary check in the UrlMatcher (is_string)

dbu commented 2 years ago

Along with this change we might want to remove unnecessary check in the UrlMatcher (is_string)

we should enable phpstan, that would detect such mistakes...