fix: Use new state in snapshotWhen #32462
Conversation
| @@ -574,6 +572,8 @@ private[akka] object Running { | |||
| } | |||
|
|
|||
| currentState = currentState.applyEvent(setup, event) | |||
| if (shouldSnapshotAfterPersist == NoSnapshot) | |||
| shouldSnapshotAfterPersist = setup.shouldSnapshot(currentState.state, event, _currentSequenceNumber) | |||
There was a problem hiding this comment.
it's correct for persist of one event, but not for this persistAll case
There was a problem hiding this comment.
If that is the case (and I do believe you), are we missing a test case?
There was a problem hiding this comment.
So, in case of persistAll, we were saying an out-of-date state? 😱
There was a problem hiding this comment.
I think, only the shouldSnapshot predicate did get passed in the old state, before the event was applied.
There was a problem hiding this comment.
If that is the case (and I do believe you), are we missing a test case?
Probably not much testing on that detail. I'm afraid I don't have time to add any before my vacation. I'm confident that this is correct and can be merged, but feel free to follow up with more tests.
| @@ -574,6 +572,8 @@ private[akka] object Running { | |||
| } | |||
|
|
|||
| currentState = currentState.applyEvent(setup, event) | |||
| if (shouldSnapshotAfterPersist == NoSnapshot) | |||
| shouldSnapshotAfterPersist = setup.shouldSnapshot(currentState.state, event, _currentSequenceNumber) | |||
There was a problem hiding this comment.
So, in case of persistAll, we were saying an out-of-date state? 😱
No description provided.