kamilmysliwiec / nest-cqrs-example

The example usage of Nest CQRS module
https://nestjs.com
492 stars 115 forks source link

Is this really CQRS? No separation between Command and Query domains. #28

Open johncmunson opened 1 year ago

johncmunson commented 1 year ago

CQRS (but not necessarily CQS) typically implies that you maintain a completely separate domain model for your Command side and your Query side.

With that in mind, this example seems to be leading people astray and doesn't really communicate the essence of CQRS.

Reason being, the command handlers and the query handlers in this example repo are both using the same domain model.

Shared Domain Model

model

Repo for the Domain Model

repo

Command Handler

command

Query Handler

query

I would love to see this example repo updated to give a more realistic depiction of how to utilize CQRS and DDD patterns. I understand that this was put together primarily for giving a brief conference talk, but I think there's really a missed opportunity for education here.

As it stands now, Nest's attempt to steer people towards CQRS/DDD/reactive patterns seems to just cause confusion and raise more questions than it actually answers. Which is a shame, because Nest really seems primed to be a leader in this space.

brianpooe commented 1 year ago

+1

jcjpsa commented 1 year ago

@johncmunson do you have a reference for a better example like this repo, other than the link you provided in your comment. I am interested in learning the correct pattern.

rrnazario commented 1 year ago

I believe, instead having the previous query handler signature, it should be better to have something like:

export class GetHeroesHandler implements IQueryHandler<GetHeroesQuery, GetHeroesResponse>

Where GetHeroesResponse configures another domain to be proper returned on handler.

kommunicate commented 6 months ago

There seem to be 2 conceptual differences between this package and what is generally recognized as the CQRS pattern. Unfortunately these differences can't be changed in a non-backwards compatible way. In the current implementation, ICommand is completely separate and parallel to IEvent but it should probably extend IEvent instead (at least, I think this is the fastest and easiest way of addressing this issue). The other issue is that Sagas are implemented as projections; not sagas -- they should be listening to the event stream; not the command stream. Canonically, sagas are used to coordinate between multiple aggregate roots. Projections are derived constructs from events.

In DDD the difference between commands and events is the intent of the calling system. An event is a fact, a command is a request to change the state of the aggregate. In the code, the execute method for EventHandlers are correctly defined as pure functions. This means that aggregate state is therefore defined as chained calls to event handlers -- h(h(h({state}))). However, that's only true if commands don't also change aggregate root state. Replaying only the events will have the effect of recreating aggregate state; allowing you to deal with system updates gracefully. State changes therefore get split into 2 parts: the command (the intent -- CreateOrder) and the resulting factual event (OrderCreated). If you were to replay the CreateOrder command it would have the effect of creating a new corresponding OrderCreated event. However, if you only replay the event, the system will gracefully recover. The sample project is so trivial that it side-steps one of the most common issues facing CQRS and event streamed applications -- aggregate mutations are almost always a 2-step process.

I see a lot of code in this package to support management of the AggregateRoot (a lot of autocommit and replay code is there), but it can never work because of the command / event issue. Projections have a similar problem -- they should be created from the event stream; not the command side. That's not to say that listening to commands isn't useful, the current implementation is superb for decoupling validations and provide a convenient place to persist commands. However, the name is different to what is generally understood with this pattern.

johncmunson commented 5 months ago

Yeah, as it stands, there's no way I would ever recommend NestJS if you want to implement these patterns. Perhaps you could use @nestjs/core for the basic app structure, but steer far away from @nestjs/cqrs and roll your own CQRS solution or look for a better library that takes these patterns seriously.