Skip to content
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 our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter fallback routes if the accepted languages do not match #435

Merged
merged 3 commits into from Apr 18, 2019

Conversation

Projects
None yet
2 participants
@leofeyer
Copy link
Member

commented Apr 12, 2019

This PR fixed #430 by filtering fallback routes if the accepted languages do not match.

  • Unit tests

I will add the unit tests as soon as we have agreed on the implementation. 馃槈

@leofeyer leofeyer added the defect label Apr 12, 2019

@leofeyer leofeyer added this to the 4.7.5 milestone Apr 12, 2019

@leofeyer leofeyer requested a review from aschempp Apr 12, 2019

@leofeyer leofeyer self-assigned this Apr 12, 2019

@leofeyer leofeyer force-pushed the hotfix/language-filter branch from a479121 to 549abae Apr 12, 2019

if (!$route->getHost()) {
$host = $route->getHost();
if (!$host || $host !== $httpHost) {

This comment has been minimized.

Copy link
@leofeyer

leofeyer Apr 12, 2019

Author Member

@aschempp I am not 100% sure if this is right, but filtering routes which have a domain that does not match the host (instead of just the ones that have no domain at all) does decrease the number of potential matches.

This comment has been minimized.

Copy link
@aschempp

aschempp Apr 15, 2019

Contributor

Yeah, I think this makes sense. I just didn't add it because they would not match anyway. But this should theoretically result in a smaller number of route match tests.

@aschempp
Copy link
Contributor

left a comment

However鈥 should we not remove the expression language?

@aschempp
Copy link
Contributor

left a comment

O well鈥 you already did 馃槀

@leofeyer leofeyer merged commit 85d8290 into 4.7 Apr 18, 2019

3 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
coverage/coveralls Coverage increased (+0.8%) to 88.846%
Details

@leofeyer leofeyer deleted the hotfix/language-filter branch Apr 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.