[Routing] RouteCollection::get() doesn't find internationalized Routes #27872

spackmat · 2018-07-06T09:29:00Z

Symfony version(s) affected: 4.1.1

Description

When I try to get a route from the RouteCollection with RouteCollection->get($routeName), I get null for internationalized routes, unless I suffix $routeName with the current _locale. When I take a look into my RouteCollection, all my internationalized routes have their suffixes (which is correct). But getting the current route from the request gets me the route name without this locale suffix. So they are incompatible forcing me to do a workaround anytime I want to get a route object from the RouteCollection by its route name.

How to reproduce

/** @var Symfony\Component\HttpFoundation\Request $request */
/** @var Symfony\Component\Routing\RouteCollection $routeCollection */
// gets the base route name without _locale suffix:
$routeName = $request->attributes->get('_route');
// is then always null for internationalized routes:
$routeObject = $routeCollection->get($routeName);
// gets the internationalized route:
$routeObject = $routeCollection->get($routeName . '.' . $request->attributes->get('_locale');

Possible Solution

Maybe, the get() method of the RouteCollection can try to load a route with the given name and the current Request's _locale. This would be consistent with the handling of the generation of internationalized routes, where feeding the full internationalized route name into the generate() method is not recommended.

Or the Request could have a _route_compatible_to_route_collection attribute to feed into RouteCollection::get().

My workaround for that issue looks like this:

// a workaround to find routes that are internationalized, since RouteCollection::get() wants to get the route name with its _locale suffix
if (null === $routeObject = $routeCollection->get($routeName)) {
    if (null === $routeObject = $routeCollection->get($routeName . '.' . $request->attributes->get('_locale', 'en'))) {
        throw new \RuntimeException('No route ' . $routeName . ' found in RouteCollection');
    }
}

Greets,
spackmat

The text was updated successfully, but these errors were encountered:

nicolas-grekas · 2018-07-07T14:18:17Z

A RouteColletion doesn't know anything about the locale, and I'm not sure it should actually.
RouteCollection instances are heavy to create usually, and using Router::getRouteCollection() at runtime is discouraged for this reason. I think the current behavior is correct personally.

spackmat · 2018-07-09T17:46:24Z

I see, then I should build a cached version of the information, I need to get for those routes (in this case the requirements for the route to generate links automatically, if possible). I have a cached map from the route name to controller class and name for another service, so this could be extended.

But besides the performance issue, I think there should be a more consistent handling of localized route names. The route generator handles the localized routes very well using the current locale, so should the whole routing system IMHO.

By the way: in most cases, I need the information only for the current route. Is there another way to get route meta information for the current request's route without waking up the RouteCollection?

stof · 2018-07-09T17:57:13Z

@spackmat no, because these requirements are used only at build time, to generate the code for the matcher and generator.
Generating your own cached storage from the RouteCollection would be the way to go (and this is the place where you could handle the convention for localized routes btw)

spackmat · 2018-07-10T07:46:22Z

Okay, thanks then! I'll extend my cache with the route requirements for that purpose.

Before the issue can be closed: What about adding another attribute to the Request holding the actual route name my_route.en instead of just the common not-localized name my_route. In my point of view, using localized routes changes the actual route names and this change must be handled gracefully in any way. The generator does this and I am with both of you in that the RouteCollection::get() is indeed the wrong place to handle this. So it would help in general, if the Request had a _route_qualified or _fqrn or such thing (in opposition to the _canonical_route within the Route's defaults) to be compatible with contexts, where the routing system is used in a more complex way than using the route generator.

nicolas-grekas · 2018-09-24T10:12:06Z

I'm not sure I understand the use case this would open. The matcher returns _route and _locale, which embed all the info (although not in the exact same format as a _fqrn).

spackmat · 2018-09-24T11:31:41Z

I thought of a value being consistent with the actual internationalized route names. I found it quite surprising, to say the least, that _route doesn't reflect the actual route name as it is used within the routing system. My code now has a adapter/workaround for this removing the locale before caching the route-requirements. But this only works with the assumption, that all local variants of the routes have the same requirements. (I could fix that by implementing more adapter-magic, but such code is a constant source of funny side effects, I prefer not to maintain.)

With "not consistent" I mean that the _route property of a Request now is a sort of a virtual route which has to be resolved before interoperating with the routing system. The generator does this magically, which is nice and covers most usecases, but in my view it would be a great service to developers and much clearer, if the Request would reflect the resolved and actually used route additionally.

nicolas-grekas · 2018-09-24T12:00:26Z

The current behavior is what allows making i18n routes totally seamless: no other parts need to take care of their internal name, as both the generator and the matcher can "talk" together using the common _route and _locale. No changes required. If some code has been written with i18n routes in mind, then one should be able to do deal with them using just these two parameters.
What I mean is that all this is a bit too abstract for me: how does your proposal translate into an actual use case? Let's say "consistency" is an issue (which I don't get why for now), it is not a use case ;)

spackmat · 2018-09-24T13:08:54Z

You are right, it is indeed a bit abstract. My usecase is described in my first post and also my way to handle the resolve-problem.

But to be more concrete: I have a RouteHelper-service that (amongst other things) generates paths while handling some context-aware route-parameters automatically. To do this, it reads the requirements of the given route name and fills some required parameters with information from the Request and other context information. This is very helpful since the templates just have to ask this service and don't have to hassle with that context stuff. At some point, this abstraction needs to get the requirements and other meta information for the current route.

As you described, i18n routes are totally seamless for the matcher and the generator. That is right. But in fact they are not seamless for any other interoperation with the routing system. So code utilizing the routing system doesn't need to deal with i18n, no changes required, which is cool and should be the goal. But then you say, code utilizing the routing system via other interfaces like the RouteCollection should be able to deal with i18n routes. So what about the goal to make i18n routes seamless? Does it apply to the whole routing system or just a part of it? If it applies, RouteCollection::get() should accept the common route names, so that the actual routes are fully transparent from the outside view, but you comprehensible stated before RouteCollection::get() shouldn't be aware of any locale-stuff. But then if it doesn't apply, then it would be great to expose the real route names to the Request to make it easier to deal with it.

I hope this makes my proposal a bit more clear: Just add a bit of DX-sugar to avoid the funny surprise, when the RouteCollection doesn't contain a route, the Request says it is the current one. ;)

nicolas-grekas · 2018-09-24T13:26:01Z

This relates to #19302 (and linked issues/PRs): RouteCollection is currently meant to populate the generator and the matcher. It's not meant (yet) to do reflection on the already compiled/dumped routes. Could be a part worth improving.

carsonbot · 2020-12-20T13:02:14Z

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

spackmat · 2020-12-20T21:44:16Z

I don't think it is resolved, but I have a more or less solid workaround for it. So it can be closed if I am the only having this problem.

xabbuh · 2020-12-21T11:57:30Z

let's close then

xabbuh added the Routing label Jul 6, 2018

JeroenDeDauw mentioned this issue Jan 29, 2019

Twig path function fails for internationalized routes #30010

Closed

carsonbot added the Stalled label Dec 20, 2020

carsonbot removed the Stalled label Dec 20, 2020

xabbuh closed this as completed Dec 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Routing] RouteCollection::get() doesn't find internationalized Routes #27872

[Routing] RouteCollection::get() doesn't find internationalized Routes #27872

spackmat commented Jul 6, 2018

nicolas-grekas commented Jul 7, 2018 •

edited

Loading

spackmat commented Jul 9, 2018

stof commented Jul 9, 2018

spackmat commented Jul 10, 2018

nicolas-grekas commented Sep 24, 2018

spackmat commented Sep 24, 2018

nicolas-grekas commented Sep 24, 2018 •

edited

Loading

spackmat commented Sep 24, 2018

nicolas-grekas commented Sep 24, 2018 •

edited

Loading

carsonbot commented Dec 20, 2020

spackmat commented Dec 20, 2020

xabbuh commented Dec 21, 2020

[Routing] RouteCollection::get() doesn't find internationalized Routes #27872

[Routing] RouteCollection::get() doesn't find internationalized Routes #27872

Comments

spackmat commented Jul 6, 2018

nicolas-grekas commented Jul 7, 2018 • edited Loading

spackmat commented Jul 9, 2018

stof commented Jul 9, 2018

spackmat commented Jul 10, 2018

nicolas-grekas commented Sep 24, 2018

spackmat commented Sep 24, 2018

nicolas-grekas commented Sep 24, 2018 • edited Loading

spackmat commented Sep 24, 2018

nicolas-grekas commented Sep 24, 2018 • edited Loading

carsonbot commented Dec 20, 2020

spackmat commented Dec 20, 2020

xabbuh commented Dec 21, 2020

nicolas-grekas commented Jul 7, 2018 •

edited

Loading

nicolas-grekas commented Sep 24, 2018 •

edited

Loading

nicolas-grekas commented Sep 24, 2018 •

edited

Loading