search

gautema / CQRSlite

A lightweight framework to help creating CQRS and Eventsourcing applications in C#
Other
1.1k stars 266 forks source link

Optional expected version? #67

Closed binarymash closed 6 years ago

binarymash commented 6 years ago

I'm a bit confused about the implementation of the expected version of an aggregate. You've said in https://github.com/gautema/CQRSlite/issues/40 that expected version is optional on a command. I can see that ISession.Get<T>(...) defaults to a null expected version if no value is supplied; this will typically be called from my implementation of ICommandHandler when a ICommand is handled. However, I also see that ICommand does not allow a nullable expected version.

My particular use case is that I want to create a process manager to manage the interactions between two aggregates, AggregateA and AggregateB. The process manager subscribes to an event from AggregateA, and then sends a command to AggregateB. However, it cannot know what the expected version of AggregateB is, so it cannot use ICommand.

I'm not sure if my confusion is because I don't understand how process managers handle concurrency, or if I'm not understanding the code correctly, or if there is a bug in the code.

Any help is greatly appreciated!

gautema commented 6 years ago

I must admit, I can't remember why ExpectedVersion is required there. I think it started together with a few other fields as required, and most of them have been removed since other users have needed versions without knowing Id and so on. I'll look a bit further into this tomorrow and see if the property is ready for being retired.

binarymash commented 6 years ago

Well there are many use cases where it does makes sense to pass an expected version on the command, but as per your comment on #40 I think it shouldn't be a requirement. So I suppose the question is, should it be a nullable int on ICommand, or should it be up to the user to add it to their commands themselves if they need it? Looks like the code compiles if I comment out the property on ICommand, but as it's a common use case it might make sense to keep it on there but make it nullable.

Keep in mind that I don't really know what I'm doing with CQRS/ES so I might be talking rubbish :)

gautema commented 6 years ago

I agree that it shouldn't be required here since it's not required later in the chain. I think removing it will be the best, so existing users don't have to start casting or change their code in other ways.

binarymash commented 6 years ago

Yeah, I think that's the best approach. I'll create a pull request.

binarymash commented 6 years ago

Looks good in 0.21.0, thanks.