Skip to content

[DependencyInjection] autoconfigure behavior describing tags on decorators #38999

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.

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

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Nov 5, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

@xabbuh xabbuh added this to the 4.4 milestone Nov 5, 2020
@xabbuh xabbuh changed the title [DepenndencyInjection] autoconfigure the container.service_subscriber tag on decorators [DependencyInjection] autoconfigure the container.service_subscriber tag on decorators Nov 5, 2020
@nicolas-grekas
Copy link
Member

For reference, disabling autoconfig for decorating services has been proposed by @stof in #30391 (comment)

The drawback was also described:

If you have a memoizing decorator, it might make perfect sense to tag the decorator service as resettable (to clear its memoization), because that's a behavior that belongs to it.

As pointed out, all tags that describe behavior that belongs to the service itself should be preserved.
container.service_subscriber is one of them, but we could add kernel.reset, kernel.event_subscriber and others I suppose.

Now, I don't know how to make this generic...

@xabbuh xabbuh force-pushed the service-subscriber-tagged-decorator branch from 6bd1db1 to a12aca6 Compare November 13, 2020 09:08
@xabbuh xabbuh changed the title [DependencyInjection] autoconfigure the container.service_subscriber tag on decorators [DependencyInjection] autoconfigure behavior describing tags on decorators Nov 13, 2020
@xabbuh
Copy link
Member Author

xabbuh commented Nov 13, 2020

I have reworked the patch to account for some more tags.

@xabbuh xabbuh force-pushed the service-subscriber-tagged-decorator branch from a12aca6 to 73a3b83 Compare November 13, 2020 09:09
@xabbuh xabbuh merged commit 3834d76 into symfony:4.4 Nov 13, 2020
@xabbuh xabbuh deleted the service-subscriber-tagged-decorator branch November 13, 2020 09:30
@fabpot fabpot mentioned this pull request Nov 21, 2020
This was referenced Nov 29, 2020
fabpot added a commit that referenced this pull request Feb 4, 2022
…ecorators (LANGERGabrielle)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Fix AsEventListener not working on decorators

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

When using the `AsEventListener` on a decorator, it is ignored instead of registering the decorator as an event listener. This is because autoconfigured tags are not applied to decorators since #30417.

`EventSubscriberInterface` works as expected since #38999, when the `kernel.event_subscriber'` tag was added to a list of "behavior describing tags" which can be autoconfigured onto decorators.

This PR adds `kernel.event_listener` to this default list of tags, making `AsEventListener` work as expected on decorators.

Commits
-------

5fbb217 Added `kernel.event_listener` to the default list of behavior describing tags, fixing AsEventListener attribute not working on decorators.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants