AggregateRoot version never updated

[xdeclercq]

It loos like the AggregateRoot Version field is never updated. It should probably be updated at the end of the LoadsFromHistory method (which should probably be renamed LoadFromHistory), with the version of the last event replayed.

[beachwalker]

This should be incremented when applying any change to the AggregateRoot, so from my point of view the method LoadFromHistory() is not the right point for incrementing the version field. You should do this here instead:

private void ApplyChange(Event @event, bool isNew)
{
    this.AsDynamic().Apply(@event);
    if(isNew) _changes.Add(@event);
    this.Version++
}

(Greg may hit me if I'm wrong)

[Narvalex]

That's correct, @beachwalker! You are not wrong :)

[xdeclercq]

Indeed.

[amolenk]

I'd say that the version should be updated at two different points:

1. Aggregate is rehydrated, Version should reflect the current version number. This could be done in LoadFromHistory method.
2. When the changes are committed (MarkChangesAsCommitted method).

I wouldn't increase the version number directly after an event is applied and the aggregate is still in an uncommitted state, because then we'll lose the original version number and won't be able to do optimistic concurrency control. Consider the following scenario:

1. Aggregate is rehydrated (version = 5)
2. Event is applied (version = 5, # uncommitted events = 1)
3. Another event is applied (version = 5, # uncommitted events = 2)
4. Aggregate is saved in repository (expected version = 5, new version = (5 + 2 =) 7, we can do optimistic concurrency control here. If the expected version differs from the version currently in the repository, throw a ConcurrencyException)
5. MarkChangesAsCommitted is called on aggregate (version = 7, # uncommitted events = 0)
6. Rinse & repeat...

[Narvalex]

We do not lose the original version number at all.

1. Aggregate is rehydrated (version = 5)
2. Event is applied (version = 6, #uncommittedEvents = 1)
3. Another event is applied (version = 7, #uncommittedEvents = 2)
4. Aggregate is saved in repository (expectedVersion = 5, newVersion = 7. Concurrency control is simple: expectedVersion (5) sould be equal to newVersion - #uncomittedEvents `(7 - 2) = 5

[amolenk]

True! That's a nice way to do the check.
I think I still prefer the Version to reflect the last 'committed' version instead of the 'new/dirty' version which may or may not become the next committed version, but you're right that it doesn't matter for concurrency checks.

[Narvalex]

Indeed. There are a lot of ways to do it right. For instance, in my own implementation of event store I do the concurrency check in another way

[darbio]

@Narvalex your link is broken. Can you direct us to your implementation?

[Narvalex]

@darbio : Nowadays Im using EventStore, but back in the day I was inspired by this implementation of a pesimistic concurrency check.