Add a note about configuring controllers without autowire/autoconfigure

[javiereguiluz]

Fixes #16579.

[AndreasA]

are you sure service_container is correct here as one needs only the service locator instance of the service subscriber and not the full container or is the service ID replaced here in case the container.service_subscriber is used?

Though I think from the compiler pass it should be ContainerInterface or serviceproviderinterface

[javiereguiluz]

With the config shown, it worked for me when disabling autowire/autoconfigure. Other configs didn't work for me.

If you can, please tell me the exact config to try and I'll report the results here. Thanks!

[AndreasA]

hi @javiereguiluz it is possible that it is correct but e.g. in other bundles like the user bundle https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Resources/config/registration.xml#L26 this was used for the call.

[AndreasA]

@javiereguiluz I have now tested it and with Symfoyn 5.4 at least, if I use

calls:
    - setContainer: [ '@Psr\Container\ContainerInterface' ]

then I get Symfony\Component\DependencyInjection\Argument\ServiceLocator as class of the container.

If I use

calls:
    - setContainer: ['@service_container']

I get ContainerIjSHgNp\App_KernelDevDebugContainer as class of the container.

The first one seems to be correct as the second one is the full service container which might actually not have all relevant services.

[AndreasA]

and if I use $this->container->get('parameter_bag') with the first it works the second I get service or alias has been removed or inlined when the container was compiled.

[AndreasA]

btw. I think the config should only be done for a specific controller class as example, as controllers that do not extends AbstractController might fail otherwise (they might not use setcontainer).

sorry for all those comments 😄

[alexandre-daubois]

I may have been a bit fast on approval 😄 I also tried with a minimalistic example:

class TestController extends AbstractController implements ServiceSubscriberInterface
{
    #[Route('/')]
    public function index(): Response
    {
        return new Response();
    }
}

Config:

services:
    # ...

    App\Controller\:
        resource: '../src/Controller'
        public: true
        autowire: false
        autoconfigure: false
        tags: ['controller.service_arguments', 'container.service_subscriber']
        calls:
            - setContainer: ['@service_container']

And I also have the The "parameter_bag" service or alias has been removed or inlined when the container was compiled error 🤔

'@Psr\Container\ContainerInterface' works also well for me, tested on 6.4. I guess we should document this with this service.

I wonder if it isn't related to this known bug actually. I'm not sure, but it really looks like it.

[javiereguiluz]

I think that at this point, we should ask @nicolas-grekas for advice about what to do here. Thanks!

[nicolas-grekas]

There is never a reason to inject the service container anywhere, so this is wrong. It will fail when any private services are accessed. It happens that AbstractController consumes services that are declared public (the router?) But this works by chance. Instead one should declare a scoped locator. There's a way to define one using tags, I don't remember how exactly and I'm AFK. Anyone does?

[AndreasA]

The locator should be defined by the container.servicer_subscriber tag in this case as the Abstractconroller provides the corresponding subscribedservices method., at least I think so?
otherwise it is even more configuration to get all correct arguments into the abstractcontroller (which might even change in the future).
not sure what the ID of that one is to inject it. I think however, ContainerInterface should work because that is what autowiring binds the service for - otherwise it would not work correctly with autowiring but not sure if this is the preferred service ID.

might also be something to add to the service subscriber/locator documentation because I don't think it is mentioned there either.

[alexandre-daubois]

Great!

[alexandre-daubois]

Do you need help to finish this one @javiereguiluz? 🙂 Or maybe it's ready?

[javiereguiluz]

@alexandre-daubois could you please take over this PR? Because of the long discussion, this is now more confusing to me than at the beginning. I'll close here and if you have some time, you can take part or all of the changes here in your PR. Thanks a lot!

[alexandre-daubois]

@javiereguiluz of course! I'll keep you updated