Calling `$this->repository->get` for the same AR twice will not find recorded events on the first result.

[basz]

Use case:

I'm trying to use the user AR's as an identity for my permission checks. By coincidence I started to add permissions to the UserAR. For other AR's then Users this problem wouldn't surface.

https://gist.github.com/basz/86942655f534c34f984b5d43f0d79f66#file-gistfile1-txt-L65-L73

Digging into this I believe it happens because calling repo->get will always rebuild a new object and return that. Loosing events recorded on the first.

https://github.com/prooph/event-store/blob/master/src/Aggregate/AggregateRepository.php#L162

Is there a reason the repository doesn't return the cached version if it is available? Would it be bad if we add that?

[basz]

I just confirmed that my gist works as expected when I add the following to the AggregateRepository Class at the appropriate place. But, again I cannot oversee if this might bring unexpected consequences.

    public function getAggregateRoot($aggregateId)
    {
        Assertion::string($aggregateId, 'AggregateId needs to be string');

        if (isset($this->identityMap[$aggregateId])) {
            return $this->identityMap[$aggregateId];
        }

        if ($this->snapshotStore) {
            $eventSourcedAggregateRoot = $this->loadFromSnapshotStore($aggregateId);

[codeliner]

@basz yes, that would be a problem:

from the docs https://github.com/prooph/event-store/blob/master/docs/repositories.md#wiring-it-together

But the identity map is cleared after each transaction commit. You may noticed the assertNotSame checks in the test. The repository keeps an aggregate only in memory as long as the transaction is active. Otherwise multiple long-running processes dealing with the same aggregate would run into concurrency issues very fast.

[basz]

No sure I understand. I get that it the identity map gets cleared after each commit. This behaviour occurs within the same transaction.

$AthingWithAnId = repoitory->get('an_id');
$AthingWithAnId->doSomethingThatRecordEvents();
$BthingWithAnId = repoitory->get('an_id'); // same AR
$BthingWithAnId->doSomethingThatRecordEvents();
$AthingWithAnId !== $BthingWithAnId

events in $AthingWithAnId are lost as only $BthingWithAnId now gets queries for events

[codeliner]

//command, begin transaction
$AthingWithAnId = repoitory->get('an_id');
$AthingWithAnId->doSomethingThatRecordEvents();
//commit transaction, clear identity map

//... whatever you do here, you'll get a fresh instance

Imagine this:

//Process1 -> command, begin transaction
$AthingWithAnId = repoitory->get('an_id');
$AthingWithAnId->doSomethingThatRecordEvents();
//commit transaction, clear identity map

// nearly parallel Process2 -> command, begin transaction
$AthingWithAnId = repoitory->get('an_id');
$AthingWithAnId->doSomethingThatRecordEvents();
//commit transaction, clear identity map

//back in Process1
$BthingWithAnId = repoitory->get('an_id'); // same AR
$BthingWithAnId->doSomethingThatRecordEvents(); //no problem, because you got a fresh version tha already have the events applied recored in the parallel process2

[codeliner]

seems like you are dealing with state in your scenario. Use a read model for that! And don't do too much in one request. You're running PHP not Scala
If you have a long-running process avoid state and pull things from a centralized storage, can also be a Redis cache or something similar but don't keep state in your PHP application and rely on it!

[codeliner]

and if you really, really, really say:parallel processes will not happen, well then use your own repository with an identity map. (this hint destroys itself within 10 seconds ... 💥 )

[basz]

:-)

Isn’t 'your 'back in Process1' after Process1 did a commit? In that case yes an fresh copy should be returned. I would argue that within the same transaction multiple repo->gets calls should always return the same instance, not a reconstructed one every time?

I'm don't believe I'm doing that much here. Reconstructing two AR's in one handler shouldn't be considered a lot of work...

[codeliner]

When happens the second recording or can you describe your use case in more details.

I only see one state changing action in your handler at the end: https://gist.github.com/basz/86942655f534c34f984b5d43f0d79f66#file-gistfile1-txt-L75

where is the problem?

[basz]

There is no second state change. The problem is; when the thing is finally commited the identityMap doesn't hold those changes because that instance was overwritten by the seconds call.

I admit this feels a little weird as both AR's are the same (The authorised user ($identity a UserAR) is trying to close it's own account which also happens to be a UserAR). In most other situations - say the authorised user want to pay an order - this problem would not occur as the OrderAR in the identityMap would not be overwritten (if it's uuid is different).

I understand the 'use a read model' idea in principal. but I also think it isn't bad perse to use the ES model for simple get statements (I do not attempt to mutate those). (I know the exact uuid, (no searching) and it is the most up-to-date). Read models are only projections to facilitate complex querying, so if your use case allows it you could create an application without any read models at all.

[prolic]

@basz you should have a read model with the current state of your AR for reading. CQRS means you SEPERATE reads from writes. Not using the same database helps a lot to achieve this :)