Currently Drupal supports to set a fully qualified class name as a controller which implements the __invoke method however this is very limited since the controller is instantiated from it's string without having the option to pass services to the construct. This means that the invokable controller can't use any external services using dependency injection. I looked into how this is supported in Symfony Framework and it was easy to add this functionality to Drupal. As mentioned in the Symfony Framework documentation this is popular with the Action-Domain-Response (ADR) pattern.
Will work like this:
services.yml
services:
controller.invoke:
class: Drupal\invoke\Controller\InvokeController
public: truerouting.yml
acme_invoke:
path: /invoke
defaults:
_controller: controller.invoke
requirements:
_access: 'TRUE'
Of course in Drupal 8.5 (and higher) it will also work like this:
services.yml
services:
Drupal\invoke\Controller\InvokeController
public: truerouting.yml
acme_invoke:
path: /invoke
defaults:
_controller: Drupal\invoke\Controller\InvokeController
requirements:
_access: 'TRUE'
The controller can in both cases be:
- A service with __invoke method registered in the container without having any arguments in the __construct
- A service with __invoke method not registered in the container with or without having any arguments in the __construct
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | core-invokable_controller-2988152-12.patch | 1.82 KB | slootjes |
| #9 | core-invokable_controller-2988152-9.patch | 1.61 KB | slootjes |
| #4 | core-invokable_controller-2988152-4.patch | 1.64 KB | slootjes |
| #2 | core-invokable_controller-2988152-1.patch | 3.29 KB | slootjes |
Comments
Comment #2
slootjes commentedHere's the patch for supporting the described functionality, test included
Comment #3
slootjes commentedComment #4
slootjes commentedThere was a UTF-8 BOM present in the patch in #1 which is removed in this patch.
Comment #5
borisson_Setting to Needs review to have the testbot take a look at the patch in #4.
Comment #6
slootjes commentedUpdate example some more.
Comment #9
slootjes commentedAssuming it failed on the Windows line endings here is a new patch with Unix line endings.
Comment #10
slootjes commentedComment #11
tim.plunkettWe already support
__invoke()for class-based controllers, might as well do so for services too.One nit on the test:
Let's make it a test case of testGetControllerFromDefinition() by adding a line to providerTestGetControllerFromDefinition().
The test method itself can set up the service definition similar to how testCreateController() does it now.
Comment #12
slootjes commentedRemoved the stand alone test and added it to providerTestGetControllerFromDefinition() instead, code remains the same.
Comment #13
tim.plunkettGreat, thanks!
Comment #14
slootjes commentedComment #15
alexpottDo we need any documentation updates in core to detail that this is supported? I think that we should document this in core/lib/Drupal/Core/Routing/routing.api.php no?
Also I think we need a change record http://drupal.org/list-changes/drupal for Drupal 8.7.x (the branch this change will likely make it into).
Comment #16
slootjes commented@alexpott I can look into that; not sure if https://www.drupal.org/docs/8/api/routing-system/structure-of-routes might be a better place, or both. Let me know :)
Comment #17
alexpott@slootjes we still need a draft change record for this change. I agree that changing https://www.drupal.org/docs/8/api/routing-system/structure-of-routes once this issue lands makes sense. There is a tension though about when to do that since 8.7 won't be released the moment this patch is committed. I agree that there is no obvious place to update docs in the code although I feel that most of https://www.drupal.org/docs/8/api/routing-system/structure-of-routes belongs in core/lib/Drupal/Core/Routing/routing.api.php.
Comment #18
slootjes commented@alexpott Change record created https://www.drupal.org/node/2997122
I agree on that it would be nice to have the documentation in the routing.api.php file but the risk of that is to have 2 different sources for the same content which will be out of sync.
Comment #19
borisson_We now have the required Change Record, that does not seem to be missing anything. Adding documentation to the routing.api.php file doesn't seem like it should be done in this issue. Keeping the needs documentation updates tag.
Comment #20
alexpottCommitted aca89d4 and pushed to 8.7.x. Thanks!