Projection implementation dependent on Message interface
camilledejoye opened this issue · 4 comments
Hi,
First of all, thanks for the work done!
To gave a bit of a context: I'm trying to use Prooph, with the PDO implementation and Symfony bundle, to work with my model without changing it.
I don't use Prooph event sourcing component at all.
Since my domain events doesn't implement the Message interface I created a wrapper for them.
So before storing I wrap them, and with a listener on "load" I unwrap them.
So far so good, until... I ran a projection!
That's the guilty part:
Which bring me to ask myself, why coupled the projection with the Message interface ?
At least for this part, didn't check all the projection system yet 😄
My point is that projections and messaging are two different and independent parts.
To go along with this idea, the return type provided by the EventStore interface for the load/loadReverse methods is Iterator
.
So there is no guaranty, no matter what, that we will be provided with a object implementing the Message interface.
And just a little bit further:
We provide the event, just as it is to the handler.
So it feels pretty strange to ask for the type of the event through a method instead of using \get_class($event)
directly.
Because it would mean that we expects to have a situation like that:
$projector->fromStream($streamName)
->when([
AType::class => function ($state, AnotherType $event) {
// Handle the AnotherType event when AType is given
},
]);
Given all that, is there a reason not to start a PR race? 😋
This would be a BC break and cannot be done. EventStore v8 is already in the pipeline though.
So I guess that mean I didn't misunderstand the situation at least.
And I was more thinking about something like:
$messageName = $event instanceof Message ? $event->messageName() : \get_class($event);
To avoid to break anything indeed.
But I will keep an eye on the dev branch for now and use composer VCS repository for my project :)
@elythyr if you can provide a test-case and a patch, that seems to be reasonable to me.
It doesn't worth the time.
I didn't saw any unit test allowing to mock an event store and control the load
method result.
To reproduce what is done in the existing tests it would require to create a new event store that handle domain event which does not extend the Message
interface.
A listener to change the results of the PdoStreamIterator
(because its only able to create objects extending Message
).
It would also require a new Aggregate, with new events, etc.
And I'm not sure what to test anyway, that we provide an object with it's own type and not another one ?
Since you are already working on the next release, I don't really see the point to go through all that :)