Closed chris-k-k closed 2 years ago
thanks for the detailed error report. i don't have time right now to dig through the code to see what exactly happens. but looking at your report, i see that you seem run the application in a sub-path of your server /test-project/public/de/my-content
. if things are set up correctly and work when you do not use a locale, it might be that we have a bug when the locale is not the first piece of the path (afaik in the beginning we messed up with assuming the application always runs at /
. we fixed things about that but maybe we missed something).
could you please check if this is the reason? do things work if the application runs at the root path? (it should be possible to test that using symfony serve
if you use the symfony binary, or php -S localhost:8888
(in the public folder) otherwise.)
Thank you for your quick reply! :) Unfortunately, running the application at the root path doesn't fix the issue.
Symfony\Component\HttpKernel\Exception\NotFoundHttpException:
No route found for "GET http://127.0.0.1:8000/en/testpath"
DEBUG08:58:58 | app | Router Symfony\Cmf\Bundle\RoutingBundle\Routing\DynamicRouter was not able to match, message "No routes found for "/en/testpath"."
The $pathinfo variable that is passed to the matchCollection method of class UrlMatcher only contains the relative path (e.g. '/testpath' or '/de/testpath'). Removing the leading locale part from $pathinfo within this method makes it work: it tries to find a match by comparing $trimmedpathinfo to the static prefix stored in the route first:
// check the static prefix of the URL first. Only use the more expensive preg_match when it matches
if ('' !== $staticPrefix && !str_starts_with($trimmedPathinfo, $staticPrefix)) {
continue;
}
... and then comparing the route's regular expression with $pathinfo second:
if (!preg_match($regex, $pathinfo, $matches)) {
continue;
}
When 'sanitizing' the url string/removing the leading locale from $pathinfo, everything works as expected. Crude brute-force example just for testing purposes:
protected function matchCollection(string $pathinfo, RouteCollection $routes): array
{
$pathinfo = str_replace('/de/', '/', $pathinfo);
$pathinfo = str_replace('/en/', '/', $pathinfo);
...
}
Log messages:
1) Matched route "test-10".
request Hide context
[
"route" => "test-10"
"route_parameters" => [
"_content_id" => "App\Entity\TestEntity:36"
"_route" => "test-10"
"_controller" => "App\Controller\TestEntityController::index"
]
"request_uri" => "http://127.0.0.1:8000/testpath"
"method" => "GET"
]
2) Matched route "test-10".
[
"route" => "test-10"
"route_parameters" => [
"_content_id" => "App\Entity\TestEntity:36"
"_route" => "test-10"
"_controller" => "App\Controller\TestEntityController::index"
]
"request_uri" => "http://127.0.0.1:8000/de/testpath"
"method" => "GET"
]
3) Matched route "test-10".
[
"route" => "test-10"
"route_parameters" => [
"_content_id" => "App\Entity\TestEntity:36"
"_route" => "test-10"
"_controller" => "App\Controller\TestEntityController::index"
]
"request_uri" => "http://127.0.0.1:8000/en/testpath"
"method" => "GET"
]
Hey there! :) I think I found a more elegant fix/workaround for this issue I ran into. With 2 small changes I get the desired result:
\symfony-cmf\routing\src\Candidates\Candidates.php
//////////
/**
* Get the locales that are supported by this strategy.
*
* @param string[] $locales The locales that are supported
*/
public function getLocales(): array
{
return $this->locales;
}
//////////
Adding a getter for $locales to Candidates class makes it possible to retrieve the locales array in the RouteProvider class:
\symfony-cmf\routing-bundle\src\Doctrine\Orm\RouteProvider.php
public function getRouteCollectionForRequest(Request $request): RouteCollection
{
$collection = new RouteCollection();
$candidates = $this->candidatesStrategy->getCandidates($request);
if (0 === \count($candidates)) {
return $collection;
}
$routes = $this->getRouteRepository()->findByStaticPrefix($candidates, ['position' => 'ASC']);
/** @var $route Route */
//////////
$locales = $this->candidatesStrategy->getLocales(); // <-- get the strategy's locales
foreach ($routes as $route) {
$collection->add($route->getName(), $route); // <-- add the 'untouched' route to the route collection
if (0 !== count($locales)) { // <-- check if locales are set
foreach ($locales as $locale) {
$multilangRoute = clone $route; // <-- clone the route
$multilangRoute->setName($locale . '-' . $route->getName()); // <-- modify the route's name
$multilangRoute->setStaticPrefix('/' . $locale . $route->getStaticPrefix()); // <-- modify the route's static prefix
$collection->add($multilangRoute->getName(), $multilangRoute); // <-- add the cloned route to the collection
}
}
}
//////////
return $collection;
}
This essentially adds a new route for every locale set in the configuration. After that, the matchCollection method of Class UrlMatcher iterates over all routes in the collection and returns a final match (correctly incorporating the locale prefix).
These 2 changes have the added benefit of being able to generate the url with the locale prefix in a template:
<!-- always returns a path to the unmodified route i.e. without any locale prefix -->
<!-- request uri: '/en/testpath' -> generated path: '/testpath' -->
<!-- myRoute = $contentDocument from the controller -->
<a href="{{ path('cmf_routing_object', {_route_object: myRoute}) }}">Read on</a>
<!-- the route document contains the modified static prefix i.e. the locale will be added to the path -->
<!-- request uri: '/en/testpath' -> generated path: '/en/testpatch' -->
<!-- myRouteDocument = $routeDocument from the controller -->
<a href="{{ path('cmf_routing_object', {_route_object: myRouteDocument}) }}">Read on</a>
This workaround accomplishes what I'm looking for w/o the need to change any core files of Symfony, only 2 files in the Cmf Routing Bundle. Unfortunately, I can't tell what repercussions this will have considering the Cmf Routing Bundle as a whole - that's way over my head ;) But if this issue indeed is to be considered a bug, perhaps my workaround will give some pointers where to look for a solid solution. Cheers!
EDIT: Obviously, you could prevent yourself from running into this issue completely by simply storing one route for every language your app is supporting in the database - perhaps in the end, this is even preferable. But I wanted to have the option of only storing one route for all languages with the controller or the template handling the translation...
sorry, for the delay, finally found time to dig into this. looking at the code, i suspect that you create your routes without the add_locale_pattern
flag. when you want the url schema to be /{locale}/path/to/content
you need to create the route with that flag. that will make it expect the locale as first path element. then the matching should work without changing the code. does that work?
i think i should convert the "Locales" block in https://symfony.com/bundles/CMFRoutingBundle/current/routing-bundle/dynamic.html to explain things better.
EDIT: sry, bad internet connection, hence the double/triple/w.e. post
thank you very much for the reply! no worries about the delay :)
i did as you asked and set this option when creating a route:
#[Route('/test/{routename}', name: 'app_test')]
public function index(EntityManagerInterface $manager, ContentRepository $contentRepository, string $routename = 'testroute'): Response
{
$post = new TestEntity();
$post->setName('My Content');
$manager->persist($post);
$manager->flush(); // flush to be able to use the generated id
$route = new CmfRoute();
$route->setName($routename);
$route->setStaticPrefix('/' . $routename);
+ $route->setOption('add_locale_pattern', true);
$route->setDefault(RouteObjectInterface::CONTENT_ID, $contentRepository->getContentId($post));
$route->setContent($post);
$post->addRoute($route); // Create the backlink from content to route
$manager->persist($post);
$manager->flush();
return $this->render('test/index.html.twig', [
'controller_name' => 'TestController',
]);
}
i was completely unaware of that option/flag, sorry. it does produce the intended result! with the minor drawback that the url scheme with locale (/{locale}/pathtocontent) is now enforced ie a url with missing locale prefix gets a 404 response. would be nice to have an option that makes urls w/o locale still find a match as long as the static prefix matches - so one could decide at a later point to either offer the content in the default language or let the 404 response stand - something like [match_implicit_locale] but in reverse if that makes sense... unless this option exists and i'm - again - unaware of it ;)
please consider this issue resolved since the code works as intended. Tyvm again for your help! :)
the documentation was lacking in that part. i added a bunch of documentation in https://github.com/symfony-cmf/routing-docs/pull/4
for supporting no locale: i checked a bit and found some old stackoverflow discussions. you can not have a default before required parts, so setting a default value for _locale
on your route entity won't help i think.
however, you normally would not want the same content available under /en/test
and /test
. my approach to this would be to write an event listener that listens to exceptions, checks if the exception was a NotFoundHttpException and in that case check if the requested url did not start with one of your locales and change the response to a redirect to the requested path with the default locale prepended.
Environment
PC - Windows 10 Xampp Apache/2.4.53 (Win64) OpenSSL/1.1.1n PHP/8.1.6 10.4.24-MariaDB database
Symfony packages
Symfony CMF packages
Subject
It looks like the multi-language support when using the doctrine ORM variant isn't working correctly. When setting the 'locales' parameter in 'cmf_routing.yaml' the matcher is unable to find a match when the url contains the locale (e.g. '/de/my-content').
I was able to pin down the source of this issue - at least, what I think the issue may be: The getCandidates method of class Candidates correctly removes the the locale from the URL string and adds it to the list of possible candidates for a match. The getRouteCollectionForRequest method of class RouteProvider is able to retrieve the route from the database (w/o the locale).
But this all seems to be ignored in the finalMatch method of class UrlMatcher: the RouteCollection derived from the candidates list is injected, but the method tries to find a match with $request->getPathInfo() which still contains the url including the locale prefix. (E.g. instead of trying to match with the static prefix '/test-page' w/o locale, it tries to match with '/de/test-page'.
A quick hack to demonstrate a work-around (it works but it's kinda ugly - I'm not yet familiar enough with Symfony to find an elegant solution ;) ):
Another solution might be to remove the locale from the context path info and pass $context->getPathInfo() to the match method:
Or it might be I'm wrong and just missing something here - perhaps a faulty config setting...
Steps to reproduce
set the locales parameter in the configuration:
request a page with a url containing the locale prefix
Expected results
Requests which contain a locale prefix ('de, 'en') ought to match with the static prefix of a dynamic route (_match_implicitlocale set to default/true): /de/my-content -> /my-content /en/my-content -> /my-content /my-content -> /my-content
(with the controller/template handling the translation)
Actual results
Adding a locale prefix to the url leads to a ResourceNotFound exception even though the route with its static prefix is present in the database.