Skip to content

[EventDispatcher] Added possibility for subscribers to subscribe several times for same event #2148

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
Sep 28, 2011
Merged

Conversation

jalliot
Copy link
Contributor

@jalliot jalliot commented Sep 10, 2011

[EventDispatcher] Added possibility for subscribers to subscribe several times for same event

closes #2146

And it is of course fully BC :)

@jalliot
Copy link
Contributor Author

jalliot commented Sep 10, 2011

If merged, #2021 will have to reflect the change too

peetersdiet added a commit to peetersdiet/symfony that referenced this pull request Sep 22, 2011
fabpot added a commit that referenced this pull request Sep 28, 2011
Commits
-------

5146a1f [EventDispatcher] Added possibility for subscribers to subscribe several times for same event

Discussion
----------

[EventDispatcher] Added possibility for subscribers to subscribe several times for same event

[EventDispatcher] Added possibility for subscribers to subscribe several times for same event

closes #2146

And it is of course fully BC :)

---------------------------------------------------------------------------

by jalliot at 2011/09/09 17:34:07 -0700

If merged, #2021 will have to reflect the change too
@fabpot fabpot merged commit 5146a1f into symfony:master Sep 28, 2011
fabpot added a commit that referenced this pull request Sep 29, 2011
Commits
-------

3223c5a Removed now useless test
21cf0ac Backported new behaviour from PR #2148 and removed check for interface at run-time
8b240d4 Implementation of kernel.event_subscriber tag for services.

Discussion
----------

Added missing kernel.event_subscriber tag (closes #2012)

This PR adds a ``kernel.event_subscriber`` tag which allows to register services as event subscribers in the same way ``kernel.event_listener`` allows to register them as event listeners.
The service is still lazy loaded and the DIC does not need to be recompiled for every modification in the service's code.

There is one important thing to remember:
If the service is created by a factory, the class parameter **MUST** reflect the real class of the service, although it is not needed at the moment for the DIC. For that issue, we could either forbid services created by factories or add a note to the documentation.

This PR closes #2012.

---------------------------------------------------------------------------

by jalliot at 2011/08/24 06:42:18 -0700

I'm not sure the test is good enough so feel free to add some more.

---------------------------------------------------------------------------

by jalliot at 2011/08/25 03:46:20 -0700

I re-implemented the check for EventSubscriberInterface in ContainerAwareEventDispatcher because I think the overhead is minimum and it allows to use this method even without the tag (at run-time).
I also added some tests for RegisterKernelListenersPass.

---------------------------------------------------------------------------

by stof at 2011/09/04 02:42:00 -0700

@jalliot Your branch conflicts with the current master. could you rebase it ?

---------------------------------------------------------------------------

by jalliot at 2011/09/04 02:57:03 -0700

Rebased

---------------------------------------------------------------------------

by jalliot at 2011/09/13 02:19:46 -0700

@fabpot What do you think about this PR? At the moment, the subscribers are not really usable in Symfony2 because of the lack of this tag.

---------------------------------------------------------------------------

by fabpot at 2011/09/13 04:17:46 -0700

I don't like subscribers. There are other PRs on adding more support for them, but the reality is that they are complex for no added benefit. I'm wondering if it wouldn't be better to just remove them altogether.

---------------------------------------------------------------------------

by jalliot at 2011/09/13 04:38:20 -0700

@fabpot Well I prefer listeners too but I think that if Symfony2 does support subscribers (which it does at the moment), it should do it properly and completely, thus allowing to register subscriber services like here or to register several methods for one same event like in #2148.
But I guess that if you merged those 2 PRs (well actually this one would have to be modified first if #2148 is merged but I'll do it then), many use cases would be covered and people should stop asking for more support :) (except maybe for removing the static modifier but this would be wrong IMO and prevent entirely this PR).

---------------------------------------------------------------------------

by fabpot at 2011/09/28 11:47:10 -0700

@jalliot: #2148 has been merged. Can you update this PR accordingly? thanks.

---------------------------------------------------------------------------

by jalliot at 2011/09/28 12:00:44 -0700

Sure thing. Will do it as well as removing the check for the interface tonight or tomorrow :)

---------------------------------------------------------------------------

by jalliot at 2011/09/29 08:53:17 -0700

@fabpot Check for interface removed and #2148 merged. Also rebased on latest master.

---------------------------------------------------------------------------

by fabpot at 2011/09/29 09:09:11 -0700

Tests do not pass.

---------------------------------------------------------------------------

by jalliot at 2011/09/29 09:18:48 -0700

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

A subscriber cannot be registered several times for an event
2 participants