CQRS / ES - Meeting, 2016-09-06

Meeting between Basti (@bwaidelich) and Robert (@robert) regarding the Neos CQRS / ES packages.

Basti went through some slides of his presentation from the Neos Devcamp which outline the minimal flow of commands and events.

Question: do we actually need to support / implement a Command Bus?

The reason for having a Command Bus is supposedly only for deferring execution of commands to remote applications or redirecting commands to other channels. Some people (Greg Young …) consider execution of remote commands as a wrong design decision, so if we restrict ourselves to only execute commands locally and interact with remote applications through events, we might have an easier time and a more simple code base.

Question: Aggregates usually replay all events in order to reconstitute their (PHP-) object. Can that be optimized?

Many operations of an aggregate don’t even need the aggregate’s state. And events don’t need to be replayed, if we implement a clever snapshot-mechanism, basically working like a projection. We’ll need the version information in order to install some optimistic locking.

-> Basti says: letting repo collect events and then publish them may be overhead, rather let’s Aggregate methods return an event directly

-> Robert: but what about creating new Aggregates?

$user = User::signUp($firstName, $lastName);

-> Robert: since Repositories need to have an Identity Map anyway, they can as well have a Unit of Work which collects events, or we let the Aggregate publish events directly (synchronously):

UserAggregate {
  protected $name;
  public function rename($newName) {
	$this->recordThat(UserRenamed::occur($this->identifier, [‘oldName’ => $this->name, ‘name’ => $newName ]);
//  -> calls $this->eventBus->publishEvent($userRenamed)
 // -> calls $this->onRenamed()
  }

  protected function onRenamed($even) {
    $this->name = $event->newName;
  }

-> Basti: the Event should only contain the domain-relevant payload, but it must be possible to add additional meta data (enrich the event) which adds information about the user who triggered the event, her IP address and so on.
We agree on that a user (developer) should be made aware of that he’s dealing with events, and should have control over when events are published. A dev shouldn’t get the impression that this is just a variation of Flow’s current persistence mechanism.

Regarding the current Neos CQRS / ES packages (on Github):

We all (Basti, Dominique, Robert) need an ES / CQRS solution urgently, so one option is to work on our individual code bases in parallel. But it’s also a waste of resources, so let’s try to get closer to a common solution next Friday.

Basti and Robert: the current implementation (by Dominique) already has too many extension points (interfaces …) and focusses a lot on Event Store internals. Let’s try to rather create a basic but working ES / CQRS solution first and avoid bells and whistles wherever we can. For that we need, of course, to identify the essential API so we don’t need to refactor it later on.

Robert: will go through Dominique’s code this week, probably fork and adjust it, and try to refactor some commands / events of his Beach application to get a feeling for it.

1 Like

Really sad I couldn’t attend it, would have loved to join the discussion, so thanks for the extensive notes!

→ Basti says: letting repo collect events and then publish them may be overhead, rather let’s Aggregate methods return an event directly

That’s also something I already considered a lot, because of the pureness of the implementation. It’s basically the functional approach to ES.

→ Robert: but what about creating new Aggregates?

$user = User::signUp($firstName, $lastName);

That’s a drawback of that pattern - the constructor/factory method (in OO) is supposed to return the instance of the aggregate, but therefore can’t easily also return the Event(s). Ocramius suggested to return a DTO in that case with the instance and the Events on the DDDinPHP group. My take would be to still just return the Events for the following reasoning:

The construction of an aggregate is in possibly 99% of cases a singular method invocation like any other aggregate method inside a CommandHandler. In that case you just want to persist the outcoming Events and not do anything else with the Aggregate itself. Otherwise the identity map of the store comes to the rescue.

The only real concern I have with that implementation is that it couples all users of the Aggregate to the persistence infrastructure, because they become responsible for persisting the events. It possibly leaks technicality into the domain.

Question: Aggregates usually replay all events in order to reconstitute their (PHP-) object. Can that be optimized?

Many operations of an aggregate don’t even need the aggregate’s state. And events don’t need to be replayed, if we implement a clever snapshot-mechanism, basically working like a projection. We’ll need the version information in order to install some optimistic locking.

I would really like to see the aggregate that contains so many events that its slower to replay the events than hydrating a fullblown ORM Model instance first before deciding on such optimizations. But in any case, it just boils down to a state-cache tagged with the aggregate version, so the aggregate needs to implement a serializeable interface. That cache then needs to be flushed whenever the Event structure or Aggregate changes (i.e. on deployment).

Thanks for writing together the notes!

Just to recap on a couple of topics:

We should probably first resolve why we need a ```CommandHandlerto begin with. AFAIK the reason is that there should only be a single entry point for mutations to your model. A controller action is only one possible entry point and it should only forward any incoming data (aka command). The CommandHandler receives a command, reconstitute the correspondingAggregate(usually by using some specialRepository ), calls some method on the Aggregate and persists it again. So it ensures the transaction ofCommand=>Event(s)=>AggregateState`

The responsibility of the CommandBus is to forward a command to exactly one handler (no matching handlers => error, more than one matching handlers => error) and I struggled to see the sense in this.
One argument (apart from the IMO obsolete “remote command” one) is that you could decorate the handler in order to validate or log any commands going into the system.
But I think that’s nothing we would need. Actual validation needs to take place in the Aggregate anyways (and I would consider authorization a special form of that) and for the syntactic validation we already have nice tools.
And re logging: That’s IMO one of the rare use case for AOP:

within(CommandControllerInterface) && method(.*->handle.*())

My thinking was more like: Before adding more complexity by implementing a snapshot-mechanism we could also just persist the Aggregate’s state (if it has any) when changed.

Think of our main usecase for this whole effort: The Content Repository.
Replaying all events that happened to a node only to change one property could be very expensive. Hydrating in that case would be quite lightweight in comparison (don’t think of "fullblown ORM Model instances, it’s just one lookup that could even contain the node’s state as JSON).
Obviously we’d need a way to “reset” the Aggregate’s state, for example if it gets new properties.

Anyways, I agree that this is more of an optimization. But IMO we should definitely look into it before implementing some kind of snapshot mechanism.

My main concern with the current architecture is that the Aggregate collects events that occur during it’s lifecycle because IMO the lifecycle should not be longer than one command was accepted (otherwise you might have published events that are not yet in the EventStore o.O).

I guess that both of these characteristics originate in the shared state nature of offline software application where you would reconstitute an Aggregate from its events, then apply some commands to it and only persist it back at a later point. With PHP we don’t have that “later point” so we could save ourselves a lot of complexity (i.e. the two tables we currently need for the Event Store).

Just for the record: I got this idea from @akii at the sprint and I just consider it as one possible alternative. Another one could be, that the Aggregate can publish events directly to the EventBus.

Sounds interesting, could you share a link?

Sure, here is the Thread: https://groups.google.com/forum/#!msg/dddinphp/qn9ZBYoDbYE/znaV6IcuAgAJ

Thanks, I already found it in the meantime. Interesting discussion!

I think if the aggregate return event … we leak complexity outside of the Aggregate, to other parts of the application need to take care of the events dispatching. I’m in favor to let the Aggregate handle the event bus.

Agree on most other proposition, and ready to build a plan to make this happen :wink:

About the current extension point, I think nothing is market API currently, we can keep them, and see if we really need them based on more feedback from CQRS project outside of the CR. Having replaceable parts can make sense. Or we can decide the opposite, remove all entry point, right now, and add them later, but I’m in favor of challenging them before removing them.

Good point. If we’d go this route we would need to provide simple means of processing those and that might not make it any simpler.

I think it was more about the architectural style.

I’d argue the other way around: Find at least two valid scenarios before introducing an abstract concept. IMO loads of the Interfaces and traits in Flow are a false assumption (in that they aren’t actually meant to be replaceable, see PackageManagerInterface for example) and/or introduce undesired complexity that could mostly be solved much cleaner using composition.
I liked Marcos talk in that regard: https://www.youtube.com/watch?v=ML-lqk_Rbew

1 Like

Fine for me to make as simple as possible, and extendable only on “real” needs :wink: make sense. Thinking about interface is hard, so let’s focus on make this things working and next step make it flexible if really needed

1 Like