NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Routing] Keeping routes priorities after add a name prefix to the collection#37176

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:5.1fromyceruto:route_priority_with_prefix

Jun 10, 2020

Merged

[Routing] Keeping routes priorities after add a name prefix to the collection#37176

fabpot merged 1 commit intosymfony:5.1fromyceruto:route_priority_with_prefix

Jun 10, 2020

Conversation

Copy link

Member

yceruto commentedJun 9, 2020

Q	A
Branch?	5.1
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix#37089
License	MIT
Doc PR	-

carsonbot added Status: Needs Review Routing Bug labels

Jun 9, 2020

yceruto mentioned this pull request

Jun 9, 2020

Routing Priority#37089

Closed

yceruto commented

Jun 9, 2020

View reviewed changes

src/Symfony/Component/Routing/RouteCollection.php

		$prefixedRoutes[$prefix.$name] =$route;
		if (null !==$name =$route->getDefault('_canonical_route')) {
		$route->setDefault('_canonical_route',$prefix.$name);
		if (null !==$canonicalName =$route->getDefault('_canonical_route')) {

Copy link

MemberAuthor

ycerutoJun 9, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Fixed name collision

nicolas-grekas approved these changes

Jun 9, 2020

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Cool thanks for having a look

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jun 9, 2020

Copy link

MemberAuthor

yceruto commentedJun 9, 2020•
edited
Loading

This is called here for instance:

symfony/src/Symfony/Component/Routing/Loader/YamlFileLoader.php

Lines 217 to 219 in6d6d989

	if (null !==$namePrefix) {
	$subCollection->addNamePrefix($namePrefix);
	}

even if the name prefix is not defined, due to:

symfony/src/Symfony/Component/Routing/Loader/YamlFileLoader.php

Line 174 in6d6d989

$namePrefix =$config['name_prefix'] ??'';

I wonder if we should compare with empty string instead or change to$config['name_prefix'] ?? null and save some ticks.

xabbuh approved these changes

Jun 9, 2020

View reviewed changes

Copy link

Member

nicolas-grekas commentedJun 9, 2020

@yceruto that'd make sense yes

yceruto commented

Jun 9, 2020

View reviewed changes

src/Symfony/Component/Routing/Loader/YamlFileLoader.php

		$methods =isset($config['methods']) ?$config['methods'] :null;
		$trailingSlashOnRoot =$config['trailing_slash_on_root'] ??true;
		$namePrefix =$config['name_prefix'] ??'';
		$namePrefix =$config['name_prefix'] ??null;