Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Right now, Drupal only handles class::method
notation to define controllers. As the controller is created by the controller resolver, a controller cannot its own constructor. This is by itself not a problem as you can get the container by implementing ContainerAwareInterface. But sometimes, you might want to control own your controllers are created, and using services for controllers allow you to do that.
Proposed resolution
This patch adds support for services as controllers with the same notation as Symfony itself. Use service_name:method
instead of class::method
.
Comment | File | Size | Author |
---|---|---|---|
#19 | controller_resolver_19.patch | 2.83 KB | fabpot |
#17 | controller_resolver_17.patch | 2.74 KB | fabpot |
#13 | controller_resolver_13.patch | 2.75 KB | fabpot |
#10 | controller_resolver.patch | 2.76 KB | fabpot |
#8 | controller_resolver.patch | 2.65 KB | fabpot |
Comments
Comment #2
fabpot CreditAttribution: fabpot commentedResubmitting the patch
Comment #3
fabpot CreditAttribution: fabpot commentedComment #5
fabpot CreditAttribution: fabpot commentedTests are failing randomly as the same patch does not end with the same failing tests.
Comment #6
fabpot CreditAttribution: fabpot commentedComment #7
aspilicious CreditAttribution: aspilicious commentedYour comments exceed the 80 chars limit.
http://drupal.org/node/1354#general
Read code twice and I have no idea what it does :) So I'm leaving this for others.
Comment #8
fabpot CreditAttribution: fabpot commentedHopefully, the coding standards are fixed now.
Let me explain what this patch does with a simple example. Let say that you have a
SomeModule
module enabled.In a routing configuration file, you associate a route pattern with a controller (the code that is executed when the route pattern matches). Here is a simple example:
With the above configuration (stored in
SomeModule.routing.yml
), whenever you enter a URL that matches/hello/{name}
(/hello/Bram
for instance), the route matches and so the associated controller is executed. But the_controller
value in the routing configuration file can only be a string, and the one used here is not a valid PHP callable.So, the controller resolver is responsible for the creation of the PHP callable that should be executed (
HttpKernel
will then execute this PHP callable). Here, it will create an instance ofDrupal\SomeModule\HelloController
and return an array where the first element is the object and the second one thehelloAction
string.Moreover, if the controller class implements
ContainerAwareInterface
, the container instance is injected by calling thesetContainer()
method (this is done by the controller resolver as well). This makes it possible for the controller to get access to any service from the container easily.But sometimes, you want to control how the controller is created. In this case, instead of defining a controller like above (
class::method
), you can (thanks to this patch), define a method on a service (by using theservice:method
convention):Here, the controller resolver will get the controller from the container (the service name being
some_module.hello_controller
) instead of creating it directly. So, the controller will now be created by the container thanks to the definition you have defined in yourSomeModuleBundle
class.Comment #9
aspilicious CreditAttribution: aspilicious commentedWoow, this explanation helps a lot. You should create lots of issues ;).
1) I love the Exception handling.
2) In http://drupal.org/node/1354#classes there is a small example of exception documentation. Maybe you could adapt that standard in this patch.
3) Overall this makes sense.
I'm not a fan of these string tricks. But I guess there isn't another way to check this. (I can't think of any)
Comment #10
fabpot CreditAttribution: fabpot commentedI've added a phpdoc about the exception
Comment #11
damiankloip CreditAttribution: damiankloip commentedFirstly, I think this would be an awesome patch to get in. Having the ability to just swap out these in the container will be great!
A couple of things:
Nitpick alert: I think we use '-' instead of '*' characters here.
I think usually we do these conditionals the other way around. I.e.
(strpos(..) !== FALSE)
.Same as above and could we just lose the
$count =
part and just dosubstr_count($controller, ':') == 1
instead?Comment #12
damiankloip CreditAttribution: damiankloip commentedComment #13
fabpot CreditAttribution: fabpot commentedI've fixed the CS.
Comment #14
Crell CreditAttribution: Crell commentedLukas would be proud of you. :-)
I'm going to RTBC this, but since it's a feature request it probably won't get committed for a little bit as we're over thresholds. I don't know if any existing work that would break it, though, so it's probably safe to sit for a bit.
Thanks, fabpot!
Comment #15
damiankloip CreditAttribution: damiankloip commentedAgreed, this looks great now!
Comment #16
catchThis looks great but I have minor nitpicks with the comments.
service:method here.
But service_name:method here.
Should pick one :)
Also we usually shy away from one-word comments like this, but I think in this case it's explanatory enough.
Moving to a task as well.
Comment #17
fabpot CreditAttribution: fabpot commentedI've renamed everything to
service:method
Should be ready now.
Comment #18
aspilicious CreditAttribution: aspilicious commentedI missed this one. We should throw this to in the docs ;)
Shouldn't block a possible commit
Comment #19
fabpot CreditAttribution: fabpot commentedadded a missing phpdoc
Comment #20
Crell CreditAttribution: Crell commentedNitpicks appear to have been addressed.
Comment #21
webchickCommitted and pushed to 8.x. Thanks!
Comment #23
dawehnerI'm wondering whether there should be a changerecord for that issue?