[FrameworkBundle] Return the invokable service if its name is the class name #18289
Conversation
15ebcff to
0a7ea2c
Compare
| $controller = $this->container->get($class); | ||
| } else { | ||
| $controller = parent::instantiateController($class); | ||
| } |
There was a problem hiding this comment.
I think we should make that check in createController() where we also retrieve the controller from the container when the service_name:method_name notation is used: https://github.com/dunglas/symfony/blob/invokable_service/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerResolver.php#L56-L71
There was a problem hiding this comment.
This is not possible without a larger refactoring because createController is never called in this case: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php#L70
An alternative is to refactor the whole getController method but it's risky and intrusive. It's why I've chosen this simple fix.
There was a problem hiding this comment.
Hm, yeah, it's probably not worth to make such big changes.
|
👍 (with @stof's comment applied) |
| @@ -78,7 +78,11 @@ protected function createController($controller) | |||
| */ | |||
| protected function instantiateController($class) | |||
| { | |||
| $controller = parent::instantiateController($class); | |||
| if ($this->container->has($class)) { | |||
| $controller = $this->container->get($class); | |||
There was a problem hiding this comment.
the simple way to fix my comment about ContainerAwareInterface is to return directly here
|
Status: needs work |
|
@stof fixed Status: needs review |
|
Thank you @dunglas. |
|
I've unintentionally opened this PR against master, but it should be merged in 2.7 (as described in the PR header). @fabpot is it possible to merge it back in other branches? |
… name is the class name (dunglas) This PR was merged into the 2.7 branch. Discussion ---------- [FrameworkBundle][2.7] Return the invokable service if its name is the class name | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/A | License | MIT | Doc PR | n/a Backport #18289 to 2.7 as this is a bug fix. Commits ------- 5c87d76 [FrameworkBundle] Return the invokable service if its name is the class name
if a service is invokable and has the same name than its class name, the controller resolver of FrameworkBundle doesn't retrieve the service and tries to construct a new instance of the class instead.
This is a very rare edge case, but this fix is useful for dunglas/DunglasActionBundle#36: referencing auto-registered controllers following the ADR style in YAML and XML routing files will be more intuitive.
Currently:
defaults: { _controller: 'Your\Action\FQN:__invoke' }, after this fix:defaults: { _controller: 'Your\Action\FQN' }.This PR also fix a currently useless test.