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

Appearance settings

[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

claudusd
Copy link
Contributor

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 in Route instance.

This duplicity doesn't make bug but when we use bin/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
{
    public function __invoke()
    {
    }
}

bin/console debug:router

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

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

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

@Tobion
Copy link
Contributor

Tobion commented Nov 17, 2018

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

@Tobion
Copy link
Contributor

Tobion commented Nov 20, 2018

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

@claudusd
Copy link
Contributor Author

@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
…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       | MIT

This 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 fabpot closed this Nov 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.