Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.x] Prohibit calling Aggregate's state() from @Apply-er methods #1501

Merged
merged 7 commits into from
Feb 6, 2023

Conversation

armiol
Copy link
Contributor

@armiol armiol commented Feb 2, 2023

This changeset addresses #1499 by disallowing to call state() method from aggregate appliers.

Previously, end-users could rely on Aggregate.state() from @Apply-ers. However, in some cases (such as the one described in #1499) state() does not reflect the last state of an aggregate. This is so because appliers are invoked in scope of a single transaction — be it when loading an aggregate instance from storage, or when applying the just-emitted events during the command dispatching. Until such a transaction is completed, state() does not reflect the model updates corresponding to the respective domain events. Using the returned state value most likely leads to bugs.

The designed way has always been to use builder() instead of state(), because it is kept up-to-date throughout the active aggregate transaction.

Therefore, once this PR is merged, any calls to state() from @Apply-annotated method will lead to an IllegalStateException.

@armiol armiol self-assigned this Feb 2, 2023
@armiol armiol changed the base branch from master to 1.x-dev February 2, 2023 12:26
@armiol armiol changed the title [1.x] Make Aggregates state()` properly available when loading after snapshot was made [1.x] Make Aggregate's state() properly available when loading after snapshot was made Feb 2, 2023
@armiol armiol changed the title [1.x] Make Aggregate's state() properly available when loading after snapshot was made [1.x] Prohibit calling Aggregate's state() from @Apply-er methods Feb 3, 2023
@armiol armiol marked this pull request as ready for review February 3, 2023 15:23
Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@armiol armiol merged commit a417208 into 1.x-dev Feb 6, 2023
@armiol armiol deleted the 1.x-snapshot-aggregate-state-improvements branch February 6, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants