neos / Neos.EventSourcing

A library for Event Sourcing and CQRS for Flow projects.
MIT License
45 stars 30 forks source link

Consider re-introducing Command Bus #118

Closed robertlemke closed 4 years ago

robertlemke commented 7 years ago

A central entry point for commands (think: central command controller) and a dispatcher / router which knows which command handler is responsible for handling a specific command would leak less of that knowledge and spread it across all the application (it's a cross-cutting concern after all).

Also it would allow us to centralize authorization and make testing easier.

@dfeyer @bwaidelich

bwaidelich commented 7 years ago

For the record: I was in favor of removing the Command Bus (#3) but have been convinced otherwise in the meantime. But IMO we should keep it simple and evolve it iteratively

albe commented 7 years ago

:+1: And make it optional. It is quite useful in quite some cases, but one can also get along without.

bwaidelich commented 5 years ago

As just discussed with @skurfuerst and @kitsunet we should also introduce some kind of simple "middleware" mechanism, so that one can register a command handler that calls the previously registered one when it's done (chain).

albe commented 5 years ago

one can register a command handler that calls the previously registered one when it's done (chain).

Huh? Care to explain the use-case and why that can't be implemented in userland in edge cases, e.g. by just invoking the specific command handler method? I somehow feel this adds too much complexity inside the command handling pipeline, where you expect one command to be handled at one place. The outcome can potentially fan out via process managers reacting to the emitted events and emitting new commands (that invoke the "old" handler explicitly). Having a chain of command handling sounds like a hidden business logic/process to me.

bwaidelich commented 5 years ago

Care to explain the use-case [...]

Good call.. We thought about some kind of "middleware" mechanism in order to solve the "Deleted Nodes" scenario for the ES Neos Content-Repository (see https://github.com/neos/contentrepository-development-collection/issues/100).

why that can't be implemented in userland in edge cases, e.g. by just invoking the specific command handler method?

Yes, I think you're right.. We should probably just re-route the affected command(s) and invoke the original handler from our custom implementation if required.

bwaidelich commented 5 years ago

FYI: I had a quick look at league/tactician and that makes a pretty good impression at first sight. I'll try to implement this in the ES Content-Repository for a test

skurfuerst commented 5 years ago

looks useful :)

bwaidelich commented 5 years ago

After playing around with this for the Content Repository I think that a CommandBus should not be part of the core ES package, because:

  1. It's really easy just to use an existing one (like league/tactician) or even create a custom one
  2. If we agree to a simple default implementation (CommandBus::handle(object $command): void) we can't implement blocking commands. If we want to support blocking command handling, we'll have to introduce a (most of the time) over-sophisticated and -opinionated implementation
albe commented 5 years ago

we can't implement blocking commands

Care to explain what you mean by that in context of PHP? AFAIS a command is always blocking until it is either accepted or rejected ("there is no fire-and-forget/one-way command"). In PHP this means the underlying consistency boundary has handled it or the command was malformed. Everything after that happens inside the projections, which are always async by default now.

And PS:

  1. A CommandBus has nothing really to do with an Event-Sourcing package ;)
bwaidelich commented 5 years ago

Care to explain what you mean by that in context of PHP?

Sure, for the Event-Sourced Content Repository we currently block some commands until the read model is up to date¹.. I know, right? But we currently don't see another viable way to keep some of the blocking behavior we currently require for Neos.

A CommandBus has nothing really to do with an Event-Sourcing package

Right, that's why I also suggest to keep it out. I do think that we could use an EventBus though, maybe with #212


¹ See:

https://github.com/neos/contentrepository-development-collection/blob/5efdf871aeb29b37f7125967463ad52d920655c7/Neos.EventSourcedContentRepository/Classes/Domain/ValueObject/CommandResult.php#L79

Example usage:

https://github.com/neos/contentrepository-development-collection/blob/5efdf871aeb29b37f7125967463ad52d920655c7/Neos.EventSourcedNeosAdjustments/Classes/Ui/Controller/BackendServiceController.php#L208

albe commented 5 years ago

A thanks. I'll have to read into that code a bit :)

bwaidelich commented 4 years ago

Btw, re

Right, that's why I also suggest to keep it out. I do think that we could use an EventBus though, maybe with #212

We did that now with #256 - It's just not called EventBus but EventPublisher

bwaidelich commented 4 years ago

I think it's time to close this one. Just like @albe wrote

A CommandBus has nothing really to do with an Event-Sourcing package

It's something that can be easily implemented (or re-used from existing packages) for the usual case and for the more advanced requirements we would buy in a whole lot of additional complexity (middleware, ...) if we wanted to support those