[FrameworkBundle] Simplify "invokable" controllers as services #11193
Conversation
|
Cannot be merged into 2.3 |
|
Ok, I wondered about that - I guess it is a new feature. Do I need to re-submit? |
| @@ -64,6 +64,12 @@ protected function createController($controller) | |||
|
|
|||
| return array($this->container->get($service), $method); | |||
| } else { | |||
| if ($this->container->has($controller) && | |||
There was a problem hiding this comment.
please use elseif instead of adding another if inside the else.
|
This should indeed go into master. |
|
Ok, is it difficult for a merger to merge this into master or should I open a new PR? |
|
I think it can be moved automatically without conflict. This can has not changed in later releases IIRC |
|
👍 |
|
Please add a test for this, then I'm 👍 for merge into master |
| @@ -63,6 +63,10 @@ protected function createController($controller) | |||
| list($service, $method) = explode(':', $controller, 2); | |||
|
|
|||
| return array($this->container->get($service), $method); | |||
| } elseif ($this->container->has($controller) && | |||
There was a problem hiding this comment.
Probably previous condition also must check whether container has such service.
There was a problem hiding this comment.
nope. The previous condition is explicitly a service syntax. Not checking if the container has a service means you get an exception telling you the service does not exist in case you made a typo.
|
OK. Will add. It looks like there are no tests for this class. Or am I missing something? |
|
I added tests for |
| use Symfony\Component\HttpKernel\Tests\Controller\ControllerResolverTest as BaseControllerResolverTest; | ||
|
|
||
| /** | ||
| * @author Kevin Bond <kevinbond@gmail.com> |
There was a problem hiding this comment.
authors are not added to tests
|
👍 for a merge in master |
|
Thank you @kbond. |
…ler services (kbond) This PR was merged into the master branch. Discussion ---------- [Cookbook][Controller] Add note about invokable controller services | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes (symfony/symfony#11193) | Applies to | 2.6+ | Fixed tickets | - Commits ------- 9879ee6 Add note about invokable controller services
…verterListener (jameshalsall) This PR was squashed before being merged into the 3.0.x-dev branch (closes #333). Discussion ---------- Adding support for invokable controllers to the ParamConverterListener Invokable controller support was added to Symfony in symfony/symfony#11193. The problem was that the `ParamConverterListener` did not support passing an object as the controller, this PR adds that support. Commits ------- 1b83587 Adding support for invokable controllers to the ParamConverterListener
… in the RequestDataCollector (jameshalsall) This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes #12443). Discussion ---------- [HttpKernel][2.6] Adding support for invokable controllers in the RequestDataCollector | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A As part of #11193 support for controllers using `__invoke()` was added. The `RequestDataCollector` did not support controllers that were defined in the routing as... ```php route_name: path: /{id} defaults: { _controller: acme_app.page.controller.page } requirements: id: \d+ ``` Where the controller was defined as... ```php class PageController { public function __invoke() { // } } ``` This PR adds that support. Tests have been updated. Commits ------- f1d043a [HttpKernel][2.6] Adding support for invokable controllers in the RequestDataCollector
Simplifies the configuration when defining controllers as services that implement
__invoke.Old:
New: