Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Routing] Deal with hosts per locale#36187

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromodolbeau:array-of-hosts
Apr 20, 2020

Conversation

odolbeau
Copy link
Contributor

@odolbeauodolbeau commentedMar 24, 2020
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#30617
LicenseMIT
Doc PRTODO

Allow to define a different host for each locale in routing.

It's now possible to define this kind of configuration:

controllers:resource:../../src/Controller/type:annotationhost:fr:www.example.fren:www.example.com

It's still possible to define an unique host (host: wwww.example.com) and if a host is defined for a given route directly, it's not overridden.

To be done:

  • YamlLoader
  • XmlLoader
  • PhpLoader?
  • Documentation
  • Changelog

alexislefebvre, gmponos, welcoMattic, pmaxs, damienalexandre, lyrixx, noniagriconomie, and cbastienbaron reacted with thumbs up emojiwelcoMattic, MatTheCat, gmponos, pmaxs, damienalexandre, lyrixx, martinbmnt, HeahDude, and cbastienbaron reacted with heart emoji
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedMar 31, 2020
edited
Loading

What happens when the route defines both localized paths and localized hosts?

It's still possible to define an unique host (host: wwww.example.com) and if a host is defined for a given route directly, it's not overridden.

I'm not sure this is correct: other attributes (defaults, condition, etc) give precedence to the importing side, isn't it?

@odolbeau
Copy link
ContributorAuthor

odolbeau commentedMar 31, 2020
edited
Loading

What happens when the route defines both localized paths and localized hosts?

Both will be used. I didn't add any test for this case cause TBH I don't know what's the expected behavior.

I'm not sure this is correct: other attributes (defaults, condition, etc) give precedence to the importing side, isn't it?

You tell me. :) I can remove this behavior (here) if you think it makes sense.
I added it cause you may want to define a totally different domain for a given route (and only for this one).
Anyway, it's doable by adding another rule in configuration so giving precedence to the importing side is not a problem.

@nicolas-grekas
Copy link
Member

I didn't add any test for this case cause TBH I don't know what's the expected behavior.

can you please add one? the behavior should be that we synchronize both path and host to create only one route for the locale, that matches hostand path, both at the same time.

I can remove this behavior (here) if you think it makes sense.

I confirm, seethis line that already has the correct precedence. It would make no sense to have precedence vary by type of declaration I think.

@odolbeau

This comment has been minimized.

@odolbeauodolbeauforce-pushed thearray-of-hosts branch 4 times, most recently frome71c8b9 to79942b2CompareApril 4, 2020 14:42
@odolbeau
Copy link
ContributorAuthor

Looks like all implementations (xml, yaml & php) are working now. :)
For tests, I use a route file container 3 routes: 1 without host nor path, 1 with a single host, 1 with 2 hosts & 2 paths.
This file is imported 4 times: without any host, with only 1 host, with several hosts & with both hosts & locales (prefixes)
I tried to be as exhaustive as possible for those tests, tell me if I forget something.

Now waiting for your reviews! :)

@welcoMattic
Copy link
Member

Thanks@odolbeau for your work!
I tested ithere with a replication of a real-life case based on a client project, and it works like a charm 👌

odolbeau reacted with hooray emoji

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks nice. Here are some questions + suggestions.

foreach ($host as $locale => $localeHost) {
$localizedRoute = clone $route;
$localizedRoute->setDefault('_locale', $locale);
$localizedRoute->setRequirement('_locale', $locale);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

missingpreg_quote()

btw, don't we miss this line inPrefixTrait? if confirmed, the fix should be for 4.4

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

sweet thanks

@nicolas-grekas
Copy link
Member

Just one thing: can you please rebase and update the changelog of the component?

@odolbeau
Copy link
ContributorAuthor

Just one thing: can you please rebase and update the changelog of the component?

Done! 👍

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM, made a minor comment on the CHANGELOG entry.

@odolbeau
Copy link
ContributorAuthor

Documentation available here:symfony/symfony-docs#13571

@fabpot
Copy link
Member

Thank you@odolbeau.

@fabpotfabpot merged commite464954 intosymfony:masterApr 20, 2020
@odolbeauodolbeau deleted the array-of-hosts branchApril 20, 2020 08:56
@nicolas-grekasnicolas-grekas modified the milestones:next,5.1May 4, 2020
@fabpotfabpot mentioned this pull requestMay 5, 2020
@seb-jean
Copy link
Contributor

Hello@odolbeau ,

Could it work with the subdomain system?

With the files below, will it be possible to have

Thanks :)

Here are my different files:

# config/routes/annotations.yamlcontrollers:     resource:../../src/Controller/     type:annotation     host:         fr:www.example.fr         en:www.example.com
// src/Controller/BlogController.phpnamespaceApp\Controller;useSymfony\Bundle\FrameworkBundle\Controller\AbstractController;useSymfony\Component\Routing\Annotation\Route;class BlogControllerextends AbstractController{/**     * @Route("/blog", name="blog_list")     */publicfunctionlist()    {// ...    }}
// src/Controller/Admin/BlogController.phpnamespaceApp\Controller\Admin;useSymfony\Bundle\FrameworkBundle\Controller\AbstractController;useSymfony\Component\Routing\Annotation\Route;class BlogControllerextends AbstractController{/**     * @Route("/", name="admin_index", host="admin")     */publicfunctionindex()    {// ...    }}

javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestApr 21, 2021
This PR was squashed before being merged into the 5.2 branch.Discussion----------[Routing] Explain host per locale<!--If your pull request fixes a BUG, use the oldest maintained branch that containsthe bug (seehttps://symfony.com/releases for the list of maintained branches).If your pull request documents a NEW FEATURE, use the same Symfony branch wherethe feature was introduced (and `5.x` for features of unreleased versions).-->Reopens#13571 initially opened by `@odolbeau` but staled for a while then closed when removing master branch.This is just a copy/paste from the inital PR.Documents feature:symfony/symfony#36187Closes#13572Commits-------ebb266c [Routing] Explain host per locale
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@welcoMatticwelcoMatticwelcoMattic approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

[Routing] Implement host per locale
6 participants
@odolbeau@nicolas-grekas@welcoMattic@fabpot@seb-jean@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp