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.

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

Issue when manually removing subscriber and have multiple subscribers on the same event #80

Closed
kytart opened this issue Sep 30, 2015 · 2 comments · Fixed by #87
Closed

Comments

@kytart
Copy link
Contributor

kytart commented Sep 30, 2015

I have some functional tests and need to disable a few subscribers that are not important for the test environment. The only way that I figured out was to manually remove them during the test setup.

It looks something like this

$container = $this->getContainer();
$evm = $this->getEventManager();

$subscribers = [
    BlameableSubscriber::class,
    NotificationSubscriber::class,
];

foreach ($subscribers as $class) {
    /** @var Subscriber $subscriber */
    $subscriber = $container->getByType($class);
    $evm->removeEventListener($subscriber->getSubscribedEvents(), $subscriber);
}

I found a bug tough.

  • Let's say I have event onSave
  • I also have 2 subscribers, that subscribe to this event - NotificationSubscriber and AnotherSubscriber

I remove NotificationSubscriber from the manager as shown above, which means, that only AnotherSubscriber remains.

Problem is, when removing NotificationSubscriber, there's this code in Kdyby\Events\EventManager on line 222

foreach ((array) $unsubscribe as $eventName) {
    foreach ($this->listeners[$eventName] as $priority => $listeners) {
        if (($key = array_search($subscriber, $listeners, TRUE)) === FALSE) {
            continue;
        }

        unset($this->listeners[$eventName][$priority][$key]);
        if (empty($this->listeners[$eventName][$priority])) {
            unset($this->listeners[$eventName][$priority]);
        }
        if (empty($this->listeners[$eventName])) {
            unset($this->listeners[$eventName]);
        }

        // there are no listeners for this specific event, so no reason to call sort on next dispatch
        $this->sorted[$eventName] = array();
    }
}

Particularly, the issue is with this line

// there are no listeners for this specific event, so no reason to call sort on next dispatch
$this->sorted[$eventName] = array();

This effectively causes that $this->sorted['onSave'] is set to an empty array, even tough it should still contain AnotherSubscriber. Later, when the event is dispatched, the listeners are fetched from this array, which results in no subscribers being called, event tough AnotherSubscriber is still registered in Kdyby\Events\EventManager::listeners.

@kytart
Copy link
Contributor Author

kytart commented Sep 30, 2015

I created a pull request that fixes this issue. Take a look at it, please.

@fprochazka
Copy link
Member

FYI, you should be able to do

foreach ($subscribers as $class) {
    /** @var Subscriber $subscriber */
    $subscriber = $container->getByType($class);
    $evm->removeEventListener($subscriber);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants