Add ability to use controller as service

[maxromanovsky]

It would be great to add ability to use "Controller as service" feature to ClassResourceInterface descendants handled by SensionExtraBundle: http://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/routing.html#controller-as-service

It helps to use best practices of DI container usage (i.e. no direct usage of the container).

[lsmith77]

PRs very welcome :)

[maxromanovsky]

@lsmith77 How do you think it should be implemented?

• via existing Sensio\Bundle\FrameworkExtraBundle\Configuration\Route annotation
• introducing somw new annotation in FOSRestBundle namespace?

[lsmith77]

is there any reason not do leverage the SensioFrameworkExtraBundle config there?

[maxromanovsky]

I haven't dig into it yet. It was just a question about you vision from the architectural perspective.

[lsmith77]

haven't dug into it myself yet and I have faith you will come up with a good answer .. but when in doubt i think reusing the annotation likely makes sense.

[xabbuh]

Is someone of you working on an implementation?

[maxromanovsky]

Not yet. Currently there's a lot of tasks on the project

[xabbuh]

Alright. Let me know when you start working on this. Otherwise I can pick this after Christmas as I could need this as well.

[maxromanovsky]

I'll notify you if I am able to start an implmentation.

[mbedna]

PR: https://github.com/FriendsOfSymfony/FOSRestBundle/pull/646

[xabbuh]

:+1:

@lsmith77 is it already possible to configure a service as a rest controller in XML or YAML? Didn't find out how.

[lsmith77]

certainly possible with YAML

[xabbuh]

Ah, figured out how to do it. Should we document this? For me it was not that clear that your controller class needs to implement the ClassResourceInterface.

By the way: The generate routes names look rather odd to me. For example, if I have a getUsersAction() method in a UserController class, the generated route name is get_user_users. As you can see the service name is not part of the route name. Is this intended?

[lsmith77]

more docs is usually good. :)

as for the route names. i don't think that we can change anything there now. could cause a lot of breakage.

[xabbuh]

Alright, let's see if I can spend some time on this. :)

I thought of something like prepending the route name with the service name. Don't know if this can be achieved easily. Nonetheless, that would make it much easier since you have all your routes from the service next to each other.

[merk]

@xabbuh we could always have a second naming strategy that can be swapped out (just like the JMSSerializerBundle)

[xabbuh]

@merk I think that would require some big refactoring, wouldn't it? Not sure if it's worth to do that.

[merk]

Probably yeah - though I dont use automatic route generation because I like finer control over what the route name will be.

[gitnik]

Any info on this? I tried to get CaS working but I couldn't. Might as well be because there is nothing in the docs and I approached the problem the wrong way

[deegital]

+1

[xabbuh]

Basically, configuring a controller as service isn't that hard. Can you show how you tried to do it?

[dvcrn]

Isn't https://github.com/liip/LiipHelloBundle/blob/master/Controller/HelloController.php a typical CoS using FOSRest?

[lsmith77]

would be nice if someone could make a fork of the symfony standard edition .. or the rest edition to illustrate their problem with this feature.

[mnapoli]

I can't get controller as service to work too. Here is what I do:

notification:
    type:     rest
    resource: Missive\ApiBundle\Controller\NotificationController

class NotificationController extends FOSRestController
{
    public function __construct(LoggerInterface $logger) {
    }

    /**
     * @Get("notifications")
     */
    public function getNotificationsAction() {
    }
}

Exception thrown: Catchable Fatal Error: Argument 1 passed to Missive\ApiBundle\Controller\NotificationController::__construct() must implement interface Psr\Log\LoggerInterface, none given

What am I doing wrong?

[lsmith77]

@mnapoli this is quite simple. you need to define the controller as a service and then specify the service name as the resource. in that service definition you of course also need to add that logger argument.

[mnapoli]

Damn OK I get what's wrong thanks.

Sorry I skipped something: the controller service definition is == the class name (I'm using PHP-DI in Symfony).

So "Missive\ApiBundle\Controller\NotificationController" is actually the service name. And FOSRestBundle seems to interpret it as "it's not a container entry, so load it without using the container".

"Normal" controllers and routing in the Framework Bundle works differently: if it's the short controller notation (e.g. MyBundle:MyController) then the controller is loaded normally (not controller as service), if it's a class name then the controller is loaded through the container…

[lsmith77]

I guess the fix for that would be to move up the $this->container->has($controller) check: https://github.com/FriendsOfSymfony/FOSRestBundle/blob/master/Routing/Loader/RestRouteLoader.php#L134

I guess it should not really cause any issues, since I doubt anyone would name a service like a class name if they would prefer the class to be loaded directly but not through the container.

[mnapoli]

Right! I'm on it thanks for the tip.