Weird order of event listeners

[enumag]

I use Kdyby/Events dev-master, not stable.

I've an event in one of my components:

class FooFormControl extends Control
{
    /** @var callable[] */
    public $onSuccess;
    //...
}
interface IFooFormFactory
{
    /** @return FooFormControl */
    public function create();
}

In presenter I create the component using a generated factory and bind a callback function:

class FooPresenter extends BasePresenter
{
    public function actionDefault($id)
    {
        //...
        $this->getComponent('form')->onSuccess[] = function () {
            $this->flashMessage('...');
            $this->redirect('Homepage:');
        };
    }
    protected function createComponentForm(IFooFormFactory $factory)
    {
        return $factory->create();
    }
}

I also have a listener:

class FooListener extends Object implements Subscriber
{
    public function getSubscribedEvents()
    {
        return [
            'FooFormControl::onSuccess' => 'fooSuccess',
        ];
    }
    public function fooSuccess()
    {
        //...
    }
}

Turns out that the listener is never called because the callback from presenter is executed first and the redirect shut's down the application. I expected the listener to be executed first because it was added first (by the generated code in generated factory) and the callback from presenter was added later.

When I remove the redirect, the listener is indeed executed, I checked that.

Is this a bug or a feature? Or am I missing something?

Or should I use a different approach? If so can you suggest? :-)

[fprochazka]

It's a feature, but you're not the first one reporting this.

[enumag]

In that case, how should I fix my application? I'm not sure what the intended solution is. Also what is the reason for this behaviour?

[fprochazka]

I know it made sense when I was creating it but I can't remember. I'm not sure we can just change it, don't know how many applications will be broken. But we should definitely have an annotation that could change this. I already started solving this few month ago but never finished it 7abbbd8

[stekycz]

All events should be independent on the order in which they are called. This is not usually a problem however your case is different. I would use two different events for this behavior.

[enumag]

@stekycz I'm aware of that, but I've also read somewhere that this is the recommended way for flash messages and redirects. Two events are of course obvious solution, I just don't like it. :-)

[vysinsky]

Can't you just use priorities of listeners?

[enumag]

@vysinsky What is the priority of the callback added in presenter? :-)

[vysinsky]

Oh.. my bad sorry.

Actually, isn't it 0? So anything with higher priority should be executed first IMO.

[fprochazka]

No, priorities are not relevant when using callbacks.

[enumag]

@vysinsky I tried it but the presenter still goes first.

[enumag]

Guess I'll go with two events for now. 👎

@fprochazka I'd like to see some sort of solution for this but am not confident enough to prepare a PR. I'll leave this issue open, close it yourself if you want. ;-)

[fprochazka]

I think we should come up with a solution :)

[enumag]

Well I'd suggest something like:

/**
 * @var callable[]
 * @lazy
 */
public $onSuccess;

But I might be missing something and am not sure about the annotation name either. Also it needs to be checked compile-time so Kdyby/Annotations are probably out of the question here.

[enumag]

Another thing to consider is adding an option to DI extension to make this behaviour default - in which case we would need an opposite annotation as well.

[fprochazka]

I had the exactly same thought :)

[enumag]

:-) Maybe @globalEventsFirst & @globalEventsLast? Or @globalEventsFirst(true) & @globalEventsFirst(false)?

[enumag]

@fprochazka I looked into 7abbbd8 a bit more and it seems there are some unrelated changes as well - two lines refactored in the Event class (those should be commited regardless) and also some changes in EventManager which I don't understand - can you explain what the EM changes are for? They are unrelated right?

I might use the rest to try preparing a PR when I get bored. :-))

[fprochazka]

@enumag yes, there are unrelated changes, because I started a bit bigger refactoring that should have included some more speed optimizations - but haven't finished it obviousely.

The parts in Event about dispatching should be fine tho. If you add processing in EventsExtension and a test maybe, I'd be happy to merge it.