[#2242] Correctly support null-identifier and no-event scenarios from Command Handling constructors, Always, and Create-If-Missing creation policies
#2248
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
During the commit phase of an Aggregate, the identifier is expected to be present. Otherwise, no events or state-stored aggregate can be inserted. However, if the identifier is not present at this point that means the command handler decided not to create the aggregate. As such, no commit action is necessary. Validate this behavior with a test case targeted towards a command handling constructor without publishing any events. #2242
If the AggregateNotFoundException is thrown within the test fixture and the identifier is null, that means the identifier hasn't been set by the command handling constructor. #2242
If the PessimisticLockFactory throws a dedicated exception instead of an IllegalArgumentException than the AlwaysCreateAggregateCommandHandler can deal with this scenario by defining the result as null instead of rethrowing the exception. This covers for the scenario when an ALWAYS creation policy decides not to publish any events or sets state. #2242
smcvb
added
Type: Bug
Except new NullLockIdentifierException instead of the previous IllegalArgumentException #2242
abuijze
requested changes
Jun 8, 2022
messaging/src/main/java/org/axonframework/common/lock/PessimisticLockFactory.java
Show resolved
Hide resolved
...ing/src/main/java/org/axonframework/modelling/command/AggregateAnnotationCommandHandler.java
Outdated
Show resolved
Hide resolved
Instead of throwing a specific exception to handled in the case when the ALWAYS creation policy does not set the aggregateIdentifier, we'll instead construct a NoOpLock. This is safe and specific enough, since the AbstractRepository ensure we do not commit whenever the aggregate identifier is null. #2242
abuijze
reviewed
Jun 20, 2022
messaging/src/main/java/org/axonframework/common/lock/PessimisticLockFactory.java
Outdated
Show resolved
Hide resolved
abuijze
approved these changes
Jun 20, 2022
smcvb
added
Status: Resolved
|
Kudos, SonarCloud Quality Gate passed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is perfectly valid for an aggregate creating command handler to decide not to publish any events, nor if it sets the aggregate identifier.
This scenario did not work for command handling constructors, the Create-if-missing, and Always creation policies.
For constructors, this is a bug that sneaked into the framework as part of release 4.5.3.
The constructor predicament is resolved by not entering the prepare-for-commit phase of the
UnitOfWorkwhen the aggregate does not contain an identifier.The create-if-missing policy had an issue in the
AggregateTestFixture, which incorrectly assumed this to be a faulty state.We solved this by checking whether the identifier is null when the aggregate couldn't be found during assertion.
The always policy rethrew the
IllegalStateExceptionof thePessimisticLockFactorythat tried to obtain a lock for anullidentifier.By throwing a dedicated exception (the
NullLockIdentifierException), the always creation policy command handler can react to this accordingly, assuming the scenario the identifier isn't set.Note that issue #2242 only references the Create-If-Missing policy.
Nonetheless, it will resolve #2242 accordingly.