Extract aggregate root into traits to make it easier to avoid domain extending infrastructure #62
Conversation
| { | ||
| $this->repository = new AggregateRepository( | ||
| $this->eventStore, | ||
| AggregateType::fromAggregateRootClass('ProophTest\EventSourcing\Mock\User'), |
There was a problem hiding this comment.
- 'ProophTest\EventSourcing\Mock\User'
+ \ProophTest\EventSourcing\Mock\User::class
This helps with IDEs that will goto definition.
src/AggregateRoot.php
Outdated
| @@ -51,50 +29,4 @@ protected function __construct() | |||
| } | |||
|
|
|||
| abstract protected function aggregateId(): string; | |||
There was a problem hiding this comment.
why is that not part of the event sourced trait?
There was a problem hiding this comment.
I guess because a trait cannot contain abstract methods to force the using class to implement it
@Xerkus Any ideas how to improve this?
There was a problem hiding this comment.
oh was late yesterday. Need to revert my comment. A trait can contain abstract methods ...
|
@codeliner can you take care of it and see whether it makes sense for us? |
|
@Xerkus Have you a valid use case to use two traits instead of one trait? |
There was a problem hiding this comment.
@Xerkus I'm hot against traits and I do like the idea to provide both ways in the same package.
Hopefully we can solve the problem with the abstract aggregateId method missing in the traits...
And I'm also interested in the answer to @sandrokeil 's question ;)
Anyway, I'd like to explain why we have this abstract AggregateRoot:
We always say and it is also in the docs prooph/event-sourcing is a demonstration of how your own event sourcing implementation could look like.It is a very good default. It is battle tested and also DDD experts say: Don't fight your framework (DDD is not about implementation anyway)
BUT if you don't like it for whatever reason you can copy & paste code, you can use your own ideas just make sure that at the end the AggregateTranslator provides correct information for the repository and the event-store.
The task of prooph is to provide production ready, stable and battle tested infrastructure glue code. Don't write your own event store, because it is just waste of time and you can do many mistakes that we already did and have corrected in prooph while running it in production.
Don't write your own buses. prooph/common with its message contract and prooph/service-bus with the flexible and very powerful plugin system saves you again a lot of time compared to a home grown solution hacked together at night.
But the event-sourcing part is the critical one. It leaks into your domain. It couples your domain to the framework. You can choose the hard way and implement the event-sourcing part alone.
You can even go one step further and work with your own messages without extending from prooph/common. Even in those cases prooph can help you a lot with the infrastructure parts, but yeah the hard way requires a lot more work on your end and misses the point of reusable components somehow.
The abstract base class has some advantages:
- we can use the decorator pattern to access protected methods of any aggregate without using reflection
- we can enforce that the domain aggregate implements a protected method (not possible with interfaces) and therefor we avoid that event sourcing internals are part of the public API of your aggregate roots. Which is much more important than the framework coupling IMO.
- we mark domain objects as AggregateRoot (ignore the imported prooph namespace, just read
User extends AggregateRoot). This is good for educational situations. At the moment I give a 3 days CQRS /ES workshop with prooph. The team never heard about event sourcing before except the name itself. Such a simple abstract class is much easier to understand than the traits.
Bonus: the IDE supports it, too. PHPStorm marks your empty User red and suggests the implementation ofaggregateIdandapply, see my workshop material: https://github.com/proophsoftware/es-workshop/blob/master/src/Model/User.php#L17
A perfect starting point to explain some basic concepts and each time you add a new AR in your system you will get reminded of what you have to do.
Traits are good to easily copy & paste code into a class. They are mixins but your domain entities are still coupled (use instead of extends). It is the same discussion as with doctrine annotations in entities. Do annotations couple my domain entity with doctrine or not? Depending on who you ask, you will get a different answer. Same for traits vs. extend ...
Having said this, I'd not change the default to using traits BUT I'm fine with providing traits as an alternative. We can add to docs that prooph/event-sourcing ships with two ways and depending on personal flavor you can choose one of the options or still implement your own. I guess a lot of people would favor the traits, but for examples and in workshops, talks, etc. my suggestion is still to use the abstract AggregateRoot just for the sake of easier understanding the idea behind.
src/AggregateRoot.php
Outdated
| @@ -51,50 +29,4 @@ protected function __construct() | |||
| } | |||
|
|
|||
| abstract protected function aggregateId(): string; | |||
There was a problem hiding this comment.
I guess because a trait cannot contain abstract methods to force the using class to implement it
@Xerkus Any ideas how to improve this?
| { | ||
| if (null === $this->aggregateIdExtractor) { | ||
| $this->aggregateIdExtractor = function (): string { | ||
| return $this->aggregateId(); |
There was a problem hiding this comment.
Here is the problem from above.
It is no longer guaranteed and the user gets no hint that such an aggregateId method is required by the translator.
The least thing we could do is add a method exists check and throw a meaningful exception instead of fatal but still not the best experience for the user.
|
moved As for he reason of two traits - not everything would be event sourced in my domain but pretty much everything would be producing domain events. ES most likely would be limited to a core domain, on as needed basis. Event producer part of event sourcing is a different story though, much like TDD promotes better OOP practices, event sourcing applied internally promotes better domain design. Programmer is forced to think not only what the state should be, but why it should be that. Not to mention that internal event sourcing is enforcing events consistency with the entity state, audit trail will never be missing a thing. |
|
@Xerkus I'm fine with the changes and the two traits. Some docs would be nice, too. I can imagine that a lot of people will ask us why there are two traits and when to use which. Also can you elaborate on this statement:
We experience the exact opposite. But I have to admit that we work on a "chaotic" domain with different systems involved. Event sourcing helps a lot to keep everything well organized. We started with a small part last year and now have a complex beast. Based on my own experience I'd say that without CQRS / ES we would need more developers and much more time to control complexity. It is not the core domain but CQRS / ES is still worth the effort. So my personal belief is that CQRS / ES can be used for any problem space except very very simple CRUD stuff. But who really works on CRUD-only domains these days? You always have different systems / services involved and so on. |
|
@Xerkus can you add some docs? |
|
cool, thx @Xerkus |
I strongly oppose the idea of my entities extending anything from the infrastructure.
All I need is to implement
AggregateTranslator. While it is easy to implement in my own codebase, i believe it would be better placed directly in theevent-sourcingpackage.This PR extracts
AggregateRootcode into two traits, EventProducerTrait and EventSourcedTrait, and introduces closure translator that can work both withAggregateRootand with entities implementing same "interface".