OnEventStrategy needs to handle custom message names better

[prolic]

Based on discussion at #108:

If message names with " ", and ".", and messages are no objects but string, array, leads to confusing behaviour, as the invoke method uses on + short message name.

Some examples:

• message name is class -> all good
• message name is "foo" -> onFoo -> all good
• message name is "foo-bar" -> onFoo-bar -> all good
• message name is "foo.bar" -> onFoo.bar -> error!
• message name is "foo bar" -> onFoo bar -> error!
• message name is string -> onString(...) - weird!
• message name is array -> onArray(...) - weird!

[codeliner]

I would not call the last two examples weird but an interesting feature :D

[codeliner]

two options:

1. Exception for everything we can not handle
2. replace invalid chars either with "" or even "". The latter would be better for the short event name detection as the dot is a common namespace separator outside of PHP

[prolic]

I usually have an projector class, that handles all projections for a given AR. When I use messages as string or array, I cannot have distinct methods to handle projection updates any more, as every message would like to call onString / onArray. Better would be onEvent() f.e., if there is no way to know the message name.

[prolic]

Perhaps the OnEventStrategy uses the classname only, and if the event is no class or uses special method name that should be used for the OnEventStrategy, too, you should write your own Userland-Strategy. Too many checks and validations will degrade performance and next week we find even more problems like this.

[codeliner]

hhm I need to think about it. Problem is, our message interface explicitly allows custom message names. Your are not forced to use one class per message. When we now limit our invoke strategies to only work with FQCNs why should we allow custom message names then? On the other hand I agree with you that custom names can be handled with custom invoke strategies. The service bus system is flexible enough to handle it.

[prolic]

Let's simply change the OnEventStrategy and also in the AggregateRoot::determineEventHandlerMethodFor and remove all this automation completely.

We can simply work with something like this instead (here example from aggregate root):

public function when(DomainEvent $event)
{
    switch (get_class($event)) {
        case ManagerWasBlocked::class:
            $this->whenManagerWasBlocked($event); // delegate to when...-method
            break;
        case ManagerWasDeleted::class:
            $this->state = ManagerState::DELETED(); // handle immediately
            break;
        default:
            throw new \UnexpectedValueException('Unknown domain event');
    }
}

When people want to have the old behaviour back, they can simply:

• write their own OnEventStrategy
• use their own AggregateRoot abstract class

But this solves all these problems with a single code change.

thoughts @codeliner ?