Skip to content

Add a note about configuring controllers without autowire/autoconfigure #18841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add a note about configuring controllers without autowire/autoconfigure #18841

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions controller/service.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,26 @@ in method parameters:
# controllers are imported separately to make sure services can be injected
# as action arguments even if you don't extend any base controller class
App\Controller\:
resource: '../src/Controller/'
tags: ['controller.service_arguments']
resource: '../src/Controller/'
tags: ['controller.service_arguments']

.. note::

If you don't use either :doc:`autowiring </service_container/autowiring>`
or :ref:`autoconfiguration <services-autoconfigure>`, you'll need to apply
other tags and make some method calls to register your controllers as services:

.. code-block:: yaml

# config/services.yaml

# this extended configuration is only required when not using autowiring/autoconfiguration,
# which is uncommon and not recommended
App\Controller\:
resource: '../src/Controller/'
tags: ['controller.service_arguments', 'container.service_subscriber']
calls:
- setContainer: ['@service_container']
Copy link

@AndreasA AndreasA Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link

@AndreasA AndreasA Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

@AndreasA AndreasA Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😄

Copy link
Member

@alexandre-daubois alexandre-daubois Sep 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link

@AndreasA AndreasA Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


If you prefer, you can use the ``#[AsController]`` PHP attribute to automatically
apply the ``controller.service_arguments`` tag to your controller services::
Expand Down