Skip to content
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

Jump to bottom

[DependencyInjection] Services no longer injected into __invoke controllers method #27208

Closed
MatTheCat opened this issue May 9, 2018 · 6 comments
Closed

[DependencyInjection] Services no longer injected into __invoke controllers method #27208

MatTheCat opened this issue May 9, 2018 · 6 comments
Labels
Bug Status: Needs Review Status: Reviewed

Comments

@MatTheCat
Copy link
Contributor

Symfony version(s) affected: 4.1.0-BETA1

Description
After upgrading to 4.0.8 to 4.1.0-BETA1 some controllers stopped working, displaying

Controller "Controller" requires that you provide a value for the "$argument" argument. Either the argument is nullable and no null value has been provided, no default value has been provided or because there is a non optional argument after this one.

After some debugging it appears ServiceValueResolver's container doesn't contain the controller so its supports method returns false.

How to reproduce
Typehint a controller's __invoke method argument with a service class and then navigate to a route calling this method.
@xabbuh
Copy link
Member

xabbuh commented May 9, 2018

Would you mind creating a small example application that allows to reproduce?

@MatTheCat
Copy link
Contributor Author

I can't reproduce it yet so my explanation must be wrong. Still investigating.

@stof
Copy link
Member

stof commented May 9, 2018

is your service tagged with controller.service_arguments ?

@MatTheCat
Copy link
Contributor Author

MatTheCat commented May 9, 2018 •
edited

@stof not my service but the controller. I started from the default services.yaml so I have

SendinBlue\Controller\:
    resource: '../src/Controller'
    tags: ['controller.service_arguments']

@MatTheCat
Copy link
Contributor Author

The issue really comes from __invoke; if I rename it services get injected correctly. But the diff between the two containers doesn't show anything probant...

@ogizanagi ogizanagi mentioned this issue May 9, 2018
Merged
@ogizanagi
Copy link
Member

Should be fixed by #27214

fabpot added a commit that referenced this issue May 14, 2018
@fabpot
bug #27214 [HttpKernel] Fix services are no longer injected into __in…
5f0e2d6 
…voke controllers method (ogizanagi)

This PR was merged into the 4.1 branch.

Discussion
----------

[HttpKernel] Fix services are no longer injected into __invoke controllers method

| Q             | A
| ------------- | ---
| Branch?       | 4.1 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #27208   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

_TL;DR:_ The `RemoveEmptyControllerArgumentLocatorsPass` is the one adding the `Controller::_invoke` => `Controller` shortcut missing from the service locator. It isn't properly executed on some cases. This fixes it.

Since #26833, the resolvers are decorated by a `TraceableValueResolver`, which usually isn't much an issue to deal within passes. But the `RemoveEmptyControllerArgumentLocatorsPass` happens late (`TYPE_BEFORE_REMOVING`), when decoration inheritance is already resolved, so accessing `$controllerLocator = $container->getDefinition((string) $serviceResolver->getArgument(0));` isn't accessing the controller locator, but the decorated service instead.

Commits
-------

ee44903 [HttpKernel] Fix services are no longer injected into __invoke controllers method
@fabpot fabpot closed this as completed May 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @stof @MatTheCat @xabbuh @ogizanagi @carsonbot