Add support for Symfony localized routes for multisite#1819
Add support for Symfony localized routes for multisite#1819haivala wants to merge 14 commits intosonata-project:4.xfrom
Conversation
Introduces SiteAwareRouter for automatic route partitioning by locale,
enabling multisite applications with locale-specific URL structures.
Key features:
- Automatic route partitioning by .{locale} suffix
- Structural 404s for wrong-locale URLs
- Per-request caching of allowed locales
- 24 unit tests with 100% coverage
- Comprehensive documentation
Breaking changes:
- Routes with dots treated as neutral unless matching {base}.{locale} format
- SiteRequestContext::getBaseUrl() includes site relativePath
Fixes sonata-project#1819
|
Actually, should I target 4.x as this is mainly adding functionality? |
|
Hey @haivala would it be possible siplit those featues in small PRs? it will be easier to review. |
|
I have been using this now for weeks in production and it works really well. I should mention that I'm using This is basically bug fix for the locale routing as it forces users to define locale routes for everything (as it is best practice). Before this all Symfony routes worked with all Sites without locale/prefix definition. For example: if you defined I do not have time to make smaller PRs from this, at least for now. |
|
I'm not sure if your changes are related with this new feature added on symfony 6.1 But it looks promising for this bundle: https://symfony.com/blog/new-in-symfony-6-1-locale-switcher |
|
One more example where PageBundle breaks stuff. You install LiipImageBundle and try to use it. The Site adds locale prefix to all media requests and because of that they do not work. I solved this by symbolic linking the cache images for the prefix folder. |
|
I think this should this be merged to 4.x? Just remove the BCHelper? |
|
This pr need to be split into small ones, it is too big to be merged :/ |
How? |
|
how about just rebasing this to 4.x as most of the changes are there already? like the BCHelper |
|
I have now time to do this. What should I do? |
|
Hi @haivala Well I guess you are the best one to figure out which changes could be merged in others pr. I do not know if it possible. But would be possible to create prs for thoses fix.
For example this double locale prefix, isn't it possible provide a fix just for this? Maybe start writing a test, it will be easier to visualise the root issue. WDYT? |
|
no. they are all tied together. I think it would be best to merge this to 4.x. This only effects on multisite with locale projects and fixes them. Main fix is the route partitioner. Looks like everything else on this MR has been already merged to 4.x. Only problem I see is that if someone uses multisite setup: after this they need to actually configure the i8n routing in Symfony. Now they have to ignore i8n routing setup for Symfony which in itself is a bug. |
|
Would be nice to have answer soon because I have now time to do this. |
|
What I can say is, try to figure out what can be merge separated in small PR, if it isn't possible. then I do not have any advise. and PRs with more then 10 files is a no go to merge. I can't guarantee that I can support with this issue/feature. But try to write some functional/unit test that cover what you want to archive with the current code. then it will be easier to visualize what you want solve. |
|
I can see that you did not even check what there is as you did not even notice that there is tests. This is bug fix and at the same time as it is a feature. Little support would be nice |
|
There is code, documentation and tests. Sorry that it is more than 10 files |
|
Could you write a functional test, reproducing the issue? |
|
I do not have time. Here is controller I used to test this |
|
please review this? |
| // Register the locale routing listener only if the feature is enabled. | ||
| // ContainerConfigurator does not allow branching on parameter values at compile time, | ||
| // so we always define the service, but we add the event listener tag only when the | ||
| // boolean parameter is true via a small runtime guard service (the listener itself | ||
| // short-circuits when disabled). To avoid even registering the tag when disabled, | ||
| // we rely on a compiler pass (optional) — if not present, the early-return in the | ||
| // listener keeps overhead negligible. |
There was a problem hiding this comment.
I cannot understand why comment was added and 0 real code here.
| // Short-circuit: if this is not a page route (not an alias/slug), delegate directly | ||
| // to the inner router to avoid double prefixing or unintended CMS decoration. | ||
| // This includes both Symfony localized routes (with .<locale> suffix) and regular routes. | ||
| if (\is_string($name) && !$this->isPageAlias($name) && !$this->isPageSlug($name)) { | ||
| return $this->router->generate($name, $parameters, $referenceType); | ||
| } |
There was a problem hiding this comment.
This is not a small change.
If i understand before this, the method was throwing a notFound exception while now it delegates the generation.
Can't this change be in a single PR ?
| // Normalize path for CMS page lookup without altering the original request path | ||
| // so Symfony's own localized route matching (with prefixes) remains intact. | ||
| $lookupPath = $pathinfo; | ||
| $relativePath = $site->getRelativePath(); | ||
|
|
||
| // Map bare site prefix (e.g. "/en") directly to the root CMS page ("/") for that site. | ||
| // This allows the English homepage to resolve at "/en" while the stored page URL remains "/". | ||
| if (null !== $relativePath && '' !== $relativePath && '/' !== $relativePath && $lookupPath === rtrim($relativePath, '/')) { | ||
| $lookupPath = '/'; | ||
| } | ||
|
|
||
| if (null !== $relativePath && '' !== $relativePath && '/' !== $relativePath && str_starts_with($lookupPath, $relativePath.'/')) { | ||
| $lookupPath = substr($lookupPath, \strlen($relativePath)); | ||
| if ('' === $lookupPath) { | ||
| $lookupPath = '/'; | ||
| } | ||
| } |
There was a problem hiding this comment.
You're changing the match behavior which might either be considered as a bugfix or a BC break and the benefit of this change is unclear to me.
Can't it be in a single PR ?
| // Detect a locale suffix in the route name (e.g. ".en", ".fi"). | ||
| // If present and it does not match the current site's locale, skip creating/updating | ||
| // a hybrid page entry for this site. This prevents duplicating localized hybrid pages | ||
| // across all sites and ensures wrong-locale routes 404 instead of falling back. | ||
| if (1 === preg_match('/\\.([A-Za-z_]+)$/', $name, $lm)) { | ||
| $routeLocale = $lm[1]; | ||
| $siteLocale = $site->getLocale(); | ||
| if (\is_string($siteLocale) && '' !== $siteLocale && $routeLocale !== $siteLocale) { | ||
| continue; | ||
| } | ||
| } |
There was a problem hiding this comment.
Same here, this will change the behavior of route name with a . isn't it ?
This is BC break then
Should it be in a single pr with an option ?
| if ($this->denyCrossLocaleGenerate && null !== $siteLocale && '' !== $siteLocale && $hasSuffix && $routeLocale !== $siteLocale) { | ||
| // At this point, mismatch means user forced a different locale via _locale; allow it. | ||
| // To deny even forced cross-locale generation, uncomment the exception below. | ||
| // throw new RouteNotFoundException("Cross-locale generation denied for route '$name' on site locale '$siteLocale'"); |
There was a problem hiding this comment.
It's unclear why we have an empty if here ; either you forgot to uncomment the exception or your can delete the whole if.
| // Do not rewrite the request pathInfo here. | ||
| // Keeping the original prefixed path allows Symfony's localized routing | ||
| // to differentiate locale variants and prevents cross-locale matches. | ||
| // (Previously: $request->setPathInfo($pathInfo ?? '/');) | ||
| if (null !== $this->site) { | ||
| $request->setPathInfo($pathInfo ?? '/'); | ||
| // Intentionally left blank. | ||
| } |
There was a problem hiding this comment.
This change is unclear to me:
- I dont understand why it's needed
- If we end up with an empty if, we just need to remove it
- Can't it be considered as a BC break and could it be in a separate pr ?
There was a problem hiding this comment.
What does the HostPathByLocaleSiteSelector change actually do?
HostPathByLocaleSiteSelector::handleKernelRequest() is part of the site selection mechanism. In a “host+path” multisite setup, the selector typically:
looks at the incoming request path (e.g. /en/products)
determines which Site matches (e.g. site with relativePath="/en")
and then (this is the important part) it rewrites the Request pathInfo to remove the site prefix, so the rest of the app routes as if the site were at /.
Before (current bundle behavior)
If the site’s relativePath is /en and you request:
Incoming URL: /en/products
The selector finds the English site and then rewrites the request:
Request::getPathInfo() becomes /products (prefix stripped)
This helps CMS page lookup and “site-relative” matching, because Sonata pages are usually stored as /products, not /en/products.
After (your change in PR #1819)
You removed the rewrite:
PHP
// Previously: $request->setPathInfo($pathInfo ?? '/');
So the request path remains:
Request::getPathInfo() stays /en/products
Why that matters for Symfony localized routes
Symfony localized routes often differ by prefix:
English route path: /en/products
Finnish route path: /tuotteet (or /products without /en)
If the selector strips /en and turns the request into /products, then Symfony’s router will try to match /products and it may:
match the wrong locale route, or
match a neutral route, or
fall through in unexpected ways
Keeping /en/products intact is what allows “structural 404 for wrong locale” to work reliably: the router can see the locale prefix and refuse other locale variants.
Side effects / why maintainers will care
This is a behavior change for anyone using HostPathByLocaleSiteSelector, because other parts of the request lifecycle may have relied on the prefix-stripped pathInfo. For example:
CMS page router might now receive /en/... and fail unless it does its own normalization (which you added to CmsPageRouter::match()).
Any custom listener/controller logic reading Request::getPathInfo() may change behavior.
So effectively:
Old behavior: “pretend site is mounted at / by rewriting request”
New behavior: “keep the real URL; make routers handle site prefixes correctly”
Recommendation
If maintainers want a config flag, then this selector change should be enabled only when sonata_page.localized_routing.enabled is true (or moved together with the router changes), because on its own it can break existing setups.
Summary
This PR adds native support for Symfony's localized routing with per-locale URL prefixes, enabling multisite applications with strict locale isolation and structural 404s for wrong-locale URLs.
Key benefits:
app_home.fi,app_home.en)Example setup:
Implementation
Core Components
SiteAwareRouter(new)router.defaultto partition routes by locale suffixapp_home→app_home.fi)_localeparameterRoutePartitioner(new).{locale}suffixadmin.dashboardtreated as neutral, not localedashboard)strrpos()andsubstr()CmsPageRouter(modified)SiteRequestContext::getBaseUrl()/en) to root page (/) for English siteRoutePageGenerator(modified)*.enhybrid pages on Finnish sitesRequest Flow
Matching (incoming requests):
SiteAwareRouterbuilds locale-specific matcher (lazy)CmsPageRouterif no route matchGeneration (URL creation):
Breaking Changes
{base}.{locale}format:admin.dashboard→ neutral route (not localedashboard)api.v2→ neutral route (not localev2)admin_dashboard.en,api_v2.fiMigration Guide
If NOT using localized routing:
If WANT to use localized routing:
relativePath:See docs/reference/localized_routing.rst for full documentation.
I am targeting this branch because this adds a new feature with minimal breaking changes (only affects routes with dots in names).
Closes #1744
Changelog
To do