[#4362] Allow for commands without an Entity to succeed#4388
Conversation
…utionException If the EntityCommandHandlingComponent is unable to find an entity-id when handling a command, we may be dealing with a creational command handler. Hence, we should invoke EntityMetamodel#handleCreate. If that's successful, our guess was correct. If this call returns a NoHandlerForCommandException, we were not dealing with a creational command handler at all. In that case, we should return the original EntityIdResolutionException #4362
When the EventStoreTransaction#appendEvent is invoked and we have tags without the existence of an AppendCondition, that signals a scenario where a creational command handler is invoked to append an event without any preceding source invocation. Thus, we should be able to assume we are dealing with the first event in this case. Making an ORIGIN marker with the found tags is restrictive, but should suffice in this case. #4362
Adjust tests to not have a resolvable identifier for creation. Although this means a shift, there's arguably more value in testing the suggested approach (to not have identifiers to load an entity when the command handler is static) then the other way around. This shift has shown the test cases to inconsistently set creational command handlers. Furthermore, the custom EntityIdResolver does not align. Lastly, the validation changes to an AppendEventsTransactionRejectedException for duplication, as the appendCondition is violated in these cases #4362
- Resolve nullability of getting a resource - Clarify updating a resource may mean you need to deal with null - Fix ugly indentation as a result of non-null/nullability settings - Clarify the ManagedEntity may not find an entity to load #4362
smcvb
added
the
Type: Bug
smcvb
added
the
Priority 1: Must
hatzlj
left a comment
hatzlj
left a comment
There was a problem hiding this comment.
looks good to me, no blockers
smcvb
added
the
Status: Under Discussion
|
The failing test tests something important: publishing two events during up front. Granted, it does this in our Test Fixtures. Never the less, I think we should have a test validating that a command handler that does not source an entity can also append several tagged events without issue, as this will be a common scenario. |
# Conflicts: # eventsourcing/src/main/java/org/axonframework/eventsourcing/eventstore/DefaultEventStoreTransaction.java
Changes to merge with axon-5.1.x #4362
smcvb
requested review from
hjohn and
laura-devriendt-lemon
and removed request for
a team
Fix assetion to correclty validate the exception #4362
smcvb
removed
the
Status: Under Discussion
Having retested this, I believe this assumption was incorrect (luckily). So, moving this issue to review again. |
Use computeResourceIfAbsent #4362
Add the EntityMetamodel#CREATE_WITHOUT_LOAD resource key of type boolean. This resource should be set whenever the EntityCommandHandlingComponent decided to invoked EntityMetamodel#handleCreate **without** trying to load an entity first. This is important for the DefaultEventStoreTransaction to derive whether it should default the AppendCondition. This fine-tunes the support of the DefaultEventStoreTransaction that defaulted the AppendCondition for any set of tags, to only do this whenever a creational-command-handler is triggered. #4362
…Condition is defaulted Set EntityMetamodel#CREATE_WITHOUT_LOAD in test validating the AppendCondition is defaulted #4362
hatzlj
left a comment
hatzlj
left a comment
There was a problem hiding this comment.
Minor javadoc findings, and two questions, looks good otherwise.
There was a problem hiding this comment.
I think we should solve this differently, and more cleanly. As it is, we now have an EntityMetaModel import in the eventsourcing layer. The processing context is now being used to trigger an alternative path from several layers up deep in the EventStore machinery. I think there is a real need to also offer this for more direct use cases, and by doing so we could also solve this case.
How about this:
We add/extend EventStoreTransaction API to express that we wish to create a new stream (and so no prior events should exist), either:
- Add a flag to
appendEvent - Add a new method
appendCreationEvent
This new path then unions tags (via updateResource/orCriteria) at ORIGIN (if used more directly, users could call #appendEvent multiple times in a single transaction to create multiple streams, and that would work then as well).
That's the new API.
Then internally, we do one of these:
- We keep the processing context flag, but resolve it MUCH earlier (still in the same layer) and use it to choose between the two
appendEventflavors - Or: we put a different
EventAppender(decorator?) in theProcessingContextthat forwards calls toappendCreationEvent-- I think since it is created lazily, we could put it in for this case before it is otherwise created...
I fully agree with the hack-part; I wasn't overly pleased with my solution either, but couldn't figure out another path. However, both your suggestions won't fly either. Not in their current form, at least. So, working with that limitation, the switch between |
Co-authored-by: Jakob Hatzl <hatzlj@users.noreply.github.com>
…entstore/DefaultEventStoreTransaction.java Co-authored-by: Jakob Hatzl <hatzlj@users.noreply.github.com>
|
| eventQueue.add(taggedEvent); | ||
| Set<Tag> tags = taggedEvent.tags(); | ||
| if (appendingWithoutSourcing() && !tags.isEmpty()) { | ||
| // No AppendCondition is present, but the event contains tags. |
There was a problem hiding this comment.
Wait… what if you want to append unconditionally (like you append MondeyDeposited to account - you don't have max limit on account, you don't need to source anything - you just always allow to do that)?
I don’t understand why we always need an AppendCondition with tags.
There was a problem hiding this comment.
That's exactly what I tried to explain in the description 🙃
Let me rehash it here:
Step 2 - Make an AppendCondition when none is present on EventStoreTransaction#appendEvent
Providing the fix in step 1 clarified another predicament.
There currently is no means to consciously set the AppendCondition when appending events from within an Event-Sourced Entity (e.g. the aggregate-centric approach).
Axon Framework resolves this by constructing an AppendCondition based on an EventStoreTransaction#source invocation.
This works fine, if we have something to source.
In step 1, we effectively remove any sourcing invocation by forcefully taking the EntityMetamodel#handleCreate route.
Hence, we need to have a fallback for this scenario.
Luckily, the DefaultEventStoreTransaction invokes the event-tagger lambda to construct TaggedEventMessage instances out of the given EventMessage to EventStoreTransaction#appendEvent.
If the TaggedEventMessage contains Tags, we can thus assume the user intends to append events that belong to a certain consistency boundary.
A clear example are creational events that have been published as result of creational commands that do not have a @TargetEntityId (or equivalent) annotated field/value.
However, this AppendCondition defaulting should only happen whenever we're dealing with a creational command handler within an entity. Thus, whenever the EntityMetamodel#handleCreate method is invoked without loading an entity first.
To that end, the EntityMetamodel#CREATE_WITHOUT_LOAD ResourceKey has been added.
To comply to that scenario, the DefaultEventStoreTransaction checks (1) if the the EventMessage has tags, (2) if the ProcessingContext contains the EntityMetamodel#CREATE_WITHOUT_LOAD set to true, and (3) if the current ProcessingContext does not contain an AppendCondition already.
When true we can construct a origin-based AppendCondition with the given tags.
We cannot assume any other consistency marker than origin, as the only means to get a concrete consistency marker is by sourcing.
4 participants
This pull request aims to resolve #4362, which is done in two steps.
Step 1 - Ensure the
EntityCommandHandlingComponentdeals withEntityIdResolutionExceptionThe original issue stems from the
EntityIdResolutionExceptionthrown by the automatically setEntityIdResolverthat looks for the@TargetEntityIdannoted field/value.However, commands creating an entity are not inclined to hold this identifier at all. An entity could very well invoke a service to create the identifier for the entity in question.
Furthermore, the predicament triggered for a
staticcommand handler INSIDE an entity. When thestaticcommand handler is placed in a regular component, the issue did not trigger (because theEntityIdResolverwasn't invoked.The combat this scenario, the
EntityCommandHandlingComponentnow has a try-catch around theEntityIdResolverinvocation.When an
EntityIdResolutionExceptionis thrown, theEntityCommandHandlingComponentwill "guess" that the command in question is a creational command handler.As such, it invokes the
EntityMetamodel#handleCreateright away.When the
EntityMetamodelinvocation fails with aNoHandlerForCommandException, we can be certain we were not dealing with a creational command handler.Hence, we fallback to throw the original
EntityIdResolutionExceptionfrom theEntityCommandHandlingComponent.However, when no
NoHandlerForCommandException, the guess was correct.Step 2 - Make an
AppendConditionwhen none is present onEventStoreTransaction#appendEventProviding the fix in step 1 clarified another predicament.
There currently is no means to consciously set the
AppendConditionwhen appending events.Axon Framework resolves this by constructing an
AppendConditionbased on anEventStoreTransaction#sourceinvocation.This works fine, if we have something to source.
In step 1, we effectively remove any sourcing invocation by forcefully taking the
EntityMetamodel#handleCreateroute.Hence, we need to have a fallback for this scenario.
Luckily, the
DefaultEventStoreTransactioninvokes the event-tagger lambda to constructTaggedEventMessageinstances out of the givenEventMessagetoEventStoreTransaction#appendEvent.If the
TaggedEventMessagecontainsTags, we can thus assume the user intends to append events that belong to a certain consistency boundary.A clear example are creational events that have been published as result of creational commands that do not have a
@TargetEntityId(or equivalent) annotated field/value.However, this
AppendConditiondefaulting should only happen whenever we're dealing with a creational command handler within an entity. Thus, whenever theEntityMetamodel#handleCreatemethod is invoked without loading an entity first.To that end, the
EntityMetamodel#CREATE_WITHOUT_LOADResourceKeyhas been added.To comply to that scenario, the
DefaultEventStoreTransactionchecks (1) if the theEventMessagehas tags, (2) if theProcessingContextcontains theEntityMetamodel#CREATE_WITHOUT_LOADset totrue, and (3) if the currentProcessingContextdoes not contain anAppendConditionalready.When
truewe can construct a origin-basedAppendConditionwith the given tags.We cannot assume any other consistency marker than origin, as the only means to get a concrete consistency marker is by sourcing.
Round-up
To test the above behavior, I have adjusted one of our integration tests by changing the creation commands to no longer hold an
@TargetEntityId.Although this means we do not test a create-or-update scenario with this command, I wagered the consequence to be neglibible. Especially given the other tests we have for create-if-missing.
Furthermore, I added some indenting and annotation changes to the touched files while working on this.
By doing the above, this PR resolves #4362