ASOS / SimpleEventStore

SimpleEventStore
MIT License
81 stars 24 forks source link

Revision / Expected Version should be an internal concern? #18

Open OnamChilwan opened 7 years ago

OnamChilwan commented 7 years ago

Noticed that the expected revision needs to be supplied in when persisting a stream and thought, shouldn't this be an internal concern?

When consuming this for the first time I was a little confused what this was and arguably should it be the consumers responsibility to manage this?

mikegore1000 commented 7 years ago

Hi Onam,

The AppendToStream operation is intentionally designed like this. The value drives the optimistic concurrency checks when writing to the event store.

So if you want a new stream, pass in 0. If you got a concurrency exception from that you know the stream exists. When appending to an existing stream, you know if another process has got in first if you get a concurrency exception assuming you've supplied the correct value (you can get this value when reading the stream).

Do you think some examples in the docs would help here, or do you think a change in API would make sense? Just wary of adding additional operations when the existing one covers quite a few use cases. It's very similar to how Event Store's client API works (though I believe they have some constants you can pass in, but it's still an int).

DaveAurionix commented 7 years ago

We're using SES wrapped in a simple event sourcing library too, containing three classes. It exposes the concept of an EventStream (wrapping an EventStore and simplifying this revision management question) and also an AggregateBase class that can repopulate an aggregate from an EventStream and append events to the stream, managing the revision. The only dependency of this tiny 3-class library is SES.

There was a lot of discussion over approaches for the Aggregate - the base class is not ideal, but when we looked at it from the consuming component's point of view, this seemed to result in the simplest possible API to handle aggregates and event streams.

Is it stretching the responsibility boundary to add EventStream and/or Aggregate concepts into SES? At the moment it is an Event Store. This would add support for Event Sourcing. A small argument in favour might be if the majority of use cases for SES involve the same approach of projecting a stream via an Aggregate. SES would then support those use cases with a simpler API (not replacing the existing pure event-store API). Would need to revisit the existing Simple Event Store packages (non-ASOS, different project, same name) to check we're not going down the same road. The ASOS approach is simpler, less feature-rich (in a good way) and that point of difference between the two might be important.

mikegore1000 commented 7 years ago

The current idea is to not add an aggregate abstraction into SES. The reason for this is that event sourcing can be done with or without DDD. For example you could have a simple transaction script that handles this (guessing this is where you'd use your EventStream abstraction).

DaveAurionix commented 7 years ago

True, thanks.

OnamChilwan commented 7 years ago

@mikegore1000 I guess the documentation could provide a little more detail about this and its usage. Perhaps an example would help? The current example is very basic so perhaps some guidelines (best practice) to managing the revision number I would personally find very useful.

Also, the revision number I get the impression this needs to be persisted somewhere? Reason I ask this is because if I were to read the stream from somewhere other than zero, I can't rely on the revision number being the number of events processed. This is why I guess it doesn't feel correct for the client to manage this and arguably where would you store this.. on the event or somewhere else entirely?

stevewgh commented 7 years ago

Shameless plug: I've put a .NET Standard library together called TinyAggregate which might help with this. It doesn't have any dependencies on SES but they do play nice together.

As the name suggests, TinyAggregate is focussed on the DDD side of things. Within the Aggregate base class there's a LoadedAtVersion property which makes it easier to detect concurrency issues when you do come to save the events.

YMMV but it works well with my projects that use SES.

NuGet: TinyAggregate https://github.com/stevewgh/TinyAggregate