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

[Routing] Remove duplicate schemes and methods for invokable controllers#29225

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

Closed

claudusd wants to merge1 commit intosymfony:4.1fromclaudusd:duplicate_methods_schemes

Closed

[Routing] Remove duplicate schemes and methods for invokable controllers#29225

claudusd wants to merge1 commit intosymfony:4.1fromclaudusd:duplicate_methods_schemes

Conversation

Copy link

Contributor

claudusd commentedNov 14, 2018

Q	A
Branch?	4.1
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT

Hi,

When we use annotation for invokable controller the http's methods and schemes are duplicate inRoute instance.

This duplicity doesn't make bug but when we usebin/console debug:router the path's method and scheme are print twice.

To reproduce :

/** * @Route("/here", name="lol", methods={"GET", "POST"}, schemes={"https"}) */class InvokableController{publicfunction__invoke()    {    }}

bin/console debug:router

----------- ----------------------------- ------------------------------ --  Name   Method           Scheme         Host   Path                                   ---------- ----------------------------- --------------- ------- ---------  lol   GET|POST|GET|POST https|https    ANY  /here                                     ------------------------ ------------ -------- ------ ----------------------

carsonbot added Status: Needs Review Routing Bug labels

Nov 14, 2018

Tobion suggested changes

Nov 15, 2018

View reviewed changes

src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php OutdatedShow resolvedHide resolved

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

Nov 15, 2018

nicolas-grekas added this to the4.1 milestone

Nov 15, 2018

Tobion reviewed

Nov 17, 2018

View reviewed changes

src/Symfony/Component/Routing/Loader/AnnotationClassLoader.php Outdated

		$globals['path'] ='';
		$globals['name'] ='';
		$globals['localized_paths'] =array();
		$globals =$this->resetGlobals();

Copy link

Contributor

TobionNov 17, 2018

Choose a reason for hiding this comment

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

please move this call outside the foreach as it does not need to be done foreach annotation

Copy link

Contributor

Tobion commentedNov 17, 2018

Could you open this against the 3.4 branch as the bug is also in there.

[Routing] Remove duplicate schemes and methods for invokable controllers

59a8c3b

Copy link

Contributor

Tobion commentedNov 20, 2018

As I said, please open a new PR against 3.4 or rebase this one. In 3.4 it's slightly different aslocalized_paths does not exist.

claudusd mentioned this pull request

Nov 21, 2018

[Routing] Remove duplicate schemes and methods for invokable controllers#29274

Merged

Copy link

ContributorAuthor

claudusd commentedNov 21, 2018

@Tobion I open the PR#29274 to backport on 3.4.
Sorry for the delay.

fabpot added a commit that referenced this pull request

Nov 24, 2018

bug#29274[Routing] Remove duplicate schemes and methods for invokab…

812a878

…le controllers (claudusd)This PR was merged into the 3.4 branch.Discussion----------[Routing] Remove duplicate schemes and methods for invokable controllers| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#29225| License       | MITThis PR backport for 3.4 branch the same issue than the PR#29225.I add a test to check the fix when annotation are on the class and rename another one when the route annotation is on the invoke method.Commits-------640ccdf [Routing] Remove duplicate schemes and methods for invokable controllers

fabpot closed this

Nov 24, 2018