[HttpKernel] Add a ContainerControllerResolver (psr-11) #21768
Conversation
| } | ||
| } | ||
|
|
||
| return parent::createController($controller); |
There was a problem hiding this comment.
This method could be rewritten to use guard clauses, making it a lot easier to read
There was a problem hiding this comment.
I made some changes. Is it better?
|
|
||
| class ControllerResolverTest extends BaseControllerResolverTest | ||
| class ControllerResolverTest extends Psr11ControllerResolverTest |
There was a problem hiding this comment.
tests in this class could stay to verify it doesn't change behavior, if this happens, it would break a lot.
There was a problem hiding this comment.
The tests cases were simply moved in the Psr11ControllerResolverTest class and are inherited, so I think we're safe, aren't we? :)
| @@ -35,39 +34,19 @@ class ControllerResolver extends BaseControllerResolver | |||
| */ | |||
| public function __construct(ContainerInterface $container, ControllerNameParser $parser, LoggerInterface $logger = null) | |||
There was a problem hiding this comment.
Why not just changing the type hint and make the parser optional?
There was a problem hiding this comment.
Because the new class is available in the HttpKernel component, whereas this one is in the framework bundle, as well as the ControllerNameParser.
I'm fine with this class as is in the framework. :) I just think having a base ControllerResolver usable with psr-11 shipped with the component makes sense.
|
I'm not fan of the Psr11 prefix. What about ServiceLocatorControllerResolver? ContainerControllerResolver? ServiceControllerResolver? |
|
Why not. What is your own preference? What do others think? I like |
|
I prefer ContainerControllerResolver actually. |
|
Renamed to |
| /** | ||
| * @param ContainerInterface $container A psr-11 container instance | ||
| * @param LoggerInterface|null $logger | ||
| */ |
|
Thank you @ogizanagi. |
…) (ogizanagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [HttpKernel] Add a ContainerControllerResolver (psr-11) | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A Extracts the controller as service resolution from the framework bundle controller resolver in a `Symfony/Component/HttpKernel/Controller/Psr11ControllerResolver`, allowing you to use `HttpKernel` with your own psr-11 container. Commits ------- 7bbae41 [HttpKernel] Add a ContainerControllerResolver (psr-11)
Extracts the controller as service resolution from the framework bundle controller resolver in a
Symfony/Component/HttpKernel/Controller/Psr11ControllerResolver, allowing you to useHttpKernelwith your own psr-11 container.