Neos.EventSourcing icon indicating copy to clipboard operation
Neos.EventSourcing copied to clipboard

EventListener event types mapping discussion points

Open albe opened this issue 7 years ago • 1 comments

EventListeners "listensTo" mapping is currently done via method name whenShortEventName + event class mapping, event class short type name must match the method

Use case 1: Listen to event from other package/BC, without implementing the event class yourself.

namespace Acme\Foo;

class BarListener ... {

    public function whenBarEvent(\Acme\Bar\Domain\BarEvent $event) {
        ...
    }

}

Use case 2: Listen to event from other package/BC, with implementing the event class yourself (no coupling to other package).

namespace Acme\Foo;

class BarEvent implements EventInterface, ProvidesEventTypeInterface {

    public static function getEventType() {
        return 'Acme.Bar:BarEvent';
    }

    ...
}

class BarListener ... {

    public function whenBarEvent(BarEvent $event) {
        ...
    }

}

Use case 3: Listen to event from other package/BC with a conflicting short event type in own BC.

namespace Acme\Foo;

class BarEvent implements EventInterface {
    ...
}

class BarListener ... {

    public function whenAcmeFooBarEvent(\Acme\Foo\BarEvent $event) {
        ...
    }

    public function whenAcmeBarBarEvent(\Acme\Bar\Domain\BarEvent $event) {
        ...
    }

}

This does currently not work, because the handler method name must match the short name of the event class parameter. So effectively, one listener can only ever listen to one short event type name per BC. It could be solved by implementing the event from the other BC in your own BC like this:

namespace Acme\Foo;

class BarEvent implements EventInterface {
    ...
}

class OtherBarEvent implements EventInterface, ProvidesEventTypeInterface {

    public static function getEventType() {
        return 'Acme.Bar:BarEvent';
    }

}

class BarListener ... {

    public function whenBarEvent(BarEvent $event) {
        ...
    }

    public function whenOtherBarEvent(OtherBarEvent $event) {
        ...
    }

}

Use case 4: Listen to an old event version with a new event class (simple event upcasting).

namespace Acme\Foo;

class FooEventV2 implements EventInterface, ProvidesEventTypeInterface {

    public static function getEventType() {
        return 'Acme.Foo:FooEvent';
    }

}

class BarListener ... {

    public function whenFooEvent(FooEventV2 $event) {
        ...
    }

}

This does currently also not work because the handler method name must match the event class short type. One would have to also change the handler method name, but it would be nice to at least leave the version name from the handler method name.

To discuss: This leads to different schemas being stored for the "Acme.Foo:FooEvent" type. Normally new such events should be stored with the new event type "Acme.Foo:FooEventV2" in order to prevent listeners that do not yet support the new version from breaking (unless they implement downcasting). So the V2 event class should deserialize from both "Acme.Foo:FooEvent" and "Acme.Foo:FooEventV2". Another approach to upcasting is implementing an actual upcaster middleware for the EventStore, that transforms the old events after deserialization.

class FooEventV2Upcaster implements EventUpcasterInterface {

   public function upcast(FooEvent $oldEvent): FooEventV2 {
     return new FooEventV2($oldEvent->prop1, $oldEvent->prop2 + $oldEvent->prop3);
   }

}

Maybe, this should even happen before deserialization, so that the old event classes do no longer need to be kept alive in the domain code base. In that case the upcaster would accept the payload (as array of simple types) and the event type and return a new payload and event type. Maybe also receive the metadata and return new metadata. This could be enveloped in a StoredEvent class then.

Use case 5: Listen to an old event version with one or multiple completely different event classes.

namespace Acme\Foo;

class PartialFooEvent1 implements EventInterface, ProvidesEventTypeInterface {

    public static function getEventType() {
        return 'Acme.Foo:FooEvent';
    }
    ...
}

class PartialFooEvent2 implements EventInterface, ProvidesEventTypeInterface {

    public static function getEventType() {
        return 'Acme.Foo:FooEvent';
    }
    ...
}

class BarListener ... {

    public function whenPartialFooEvent1(PartialFooEvent1 $event) {
        ...
    }

    public function whenPartialFooEvent2(PartialFooEvent2 $event) {
        ...
    }

}

This is currently impossible, due to the EventTypeResolver holding a 1:1 map of fully qualified event type <=> event class name. It could be solved with upcasters from above, if they support returning more than one resulting event.

...
public function upcast(FooEvent $oldEvent): MappedEvents

where MappedEvents is just an ordered collection of EventInterfaces, in this case PartialFooEvent1 and PartialFooEvent2.

Suggestion: Map via event class type resolving only and ignore method name (or at least make it less strict, e.g. only do an "contains short event type" check) in order to allow case 3+4. It is actually a rather arbitrary constraint by some convention and not a technical necessity. Alternatively, we should respect the ProvidesEventTypeInterface inside the EventListenerLocator and compare the method name against the (shortened and/or stripped) event type from the getEventType() call, e.g.

// Allow "whenAcmeBarBarEvent" method name
$expectedFullyQualifiedMethodName = 'when' . preg_replace('/[^A-Za-z0-9]/', '', $eventClass::getEventType());
// Allow "whenBarEvent" method name
$expectedMethodName = 'when' . end(implode(':', $eventClass::getEventType()));

See https://github.com/neos/Neos.EventSourcing/blob/master/Classes/EventListener/EventListenerLocator.php#L218

albe avatar Oct 27 '18 12:10 albe

Hey there!

I'm currently upcasting an event or two and I came across this topic.

I do like the idea of having something in place that allows to consume one event and creates a couple of others on runtime.

I'd imagine an interface like this:

interface Upcaster extends Iterator {
    public function __construct(RawEvent $rawEvent);
    public function current(): RawEvent;
}

Here's what Prooph does in this case: http://docs.getprooph.org/event-store/upcasting.html

I quickly discussed this with @bwaidelich where he was immediately concerned about creating multiple runtime-only events with the same sequence number.

Currently there's a "TODO" mark in the EventNormalizer::denormalize() method.

I'm unsure if that's the right place. I'd rather make that part of the EventStream, just wrap it around the EventStream::$streamIterator, something like this:

    public function __construct(StreamName $streamName, EventStreamIteratorInterface $streamIterator)
    {
        $this->streamName = $streamName;
        $this->streamIterator = new \NoRewindIterator((function() use ($streamIterator) {
            foreach ($streamIterator as $event) {
                yield from [$event]; // This is the part where upcasts happen
            }
        })());
    }

That's basically the "use case 4" and "use case 5" scenarios here I'm voting for.

As for the mapping from event class names to when methods: I'm all in for dropping the constraint of "listener method name must match the event short name" thingy. Since the EventListenerLocator::getListenersForEventClassName returns just callables anyway (which is really a good thing), I don't see a point in keeping that constraint, besides enforcing a certain naming convention.

Don't get me wrong: I do like the naming convention. But it feels mandatory to me to just not enforce that.

Regards, Stephan.

stephanschuler avatar Jul 09 '19 16:07 stephanschuler