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

@EventSourcingHandlers on @AggregateMembers should only receive events matching the @EntityId #388

Closed
tbvh opened this issue Jul 21, 2017 · 3 comments
Assignees
Labels
Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. Type: Feature Use to signal an issue is completely new to the project.
Milestone

Comments

@tbvh
Copy link

tbvh commented Jul 21, 2017

Working with @AggregateMembers allows separation of concerns within one aggregate. And annotating a field with @EntityId results in commands only being routed to the entity that matches that id.
However, for events this routing is not in place. I.e. an EventSourcingHandler on an entity receives the events of all entities as stated here.
This issue plays up when the @AggregateMember annotation is applied on a collection of entities.

This is counter intuitive as it nowhere stated explicitly in the docs (although, granted, it's also nowhere stated that it should work for events either ;) ). The workaround is error prone, since filtering the events needs to be consistently applied for every EventSourcingHandler.

For consistency, the routing of events to EventSourcingHandlers should be in line with the routing to CommandHandlers, and respect the EntityId of AggregateMembers.

@smcvb smcvb added this to the Release 3.1 milestone Jul 21, 2017
@smcvb smcvb added the Type: Enhancement Use to signal an issue enhances an already existing feature of the project. label Jul 21, 2017
@smcvb
Copy link
Member

smcvb commented Jul 21, 2017

We've been discussing this as well, thanks for adding an issue for it.

smcvb added a commit that referenced this issue Oct 16, 2017
Add forwardEntityOriginatingEventsOnly field to indicate whether this
aggregate member should only receive events originating from himself

[#388]
smcvb added a commit that referenced this issue Oct 16, 2017
Adjust all AggregateMemberAnnotatedChildEntity definitions to pull out
the forwardEntityOriginatingEventsOnly field from the annotation and
push it through to the AnnotatedChildEntity class for later inspection.
Additionally reindent according to axon framework code style

[#388]
smcvb added a commit that referenced this issue Oct 16, 2017
Introduce forwardEntityOriginatingEventsOnly check on the
publish(EventMessage<?> msg, P declaringInstance) function to filter out
 any events which aren't originating from the entity itself.
Additionally:
 - Reindent according to axon framework style guide
 - Add javadoc for new forwardEntityOriginatingEventsOnly field
 - Extract spliteratorIsImmutableOrConcurrent() function for clarity

[#388]
smcvb added a commit that referenced this issue Oct 16, 2017
Introduce AbstractChildEntityCollectionDefinition to be extended by the
Collection and Map ChildEntityDefinition implementation to reuse code
among both.

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
-Add javadoc to the AbstractChildEntityCollectionDefinition
-Remove the unused Integer index from the resolveGenericType() function

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
Introduce a ForwardingMode enum to be used in the AggregateMember
annotation to describe how event messages should routed to it.

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
- Drop the 'forwardEntityOriginatingEventsOnly' property in favor of the
 ForwardingMode eventRoutingMode. This allows us to deprecate the
 forwardEvents() property and be more specific on how we want to route
 evens to an Aggregate Member
- Introduce a 'eventRoutingKey' property to be used if the
'eventRoutingMode' is set to 'ForwardingMode.ROUTING_KEY'
- Expand the javadoc and add missing return descriptions where
applicable.

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
Use the ForwardingMode to route events. There is a cut off if
ForwardingMode. == NONE to not forward any events. On ALL the entity
receives all the events within this entity. And lastly if the mode is
ROUTING_KEY, the routing value of the event is matched to the entity
identifier.
The javadoc has ben adjusted correctly to reflect this.

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
Supply the eventRoutingMode and eventRoutingKey fields to the
AnnotatedChildEntity created. Use a determination function to select the
 event routing mode supplied. This is required as long as we're
 servicing the deprecated `forwardEvents` field in the @AggregateMember
 annotation.

[#388]
smcvb added a commit that referenced this issue Nov 13, 2017
Introduce test class for entity event routing separated from the
ModelInspector test class to decrease file size.
Tests a single entity, list entity and map entity aggregate, as
well as an entity with ForwardingMode set to NONE and an entity with a
specific routing key different from the entity id.

[#388]
@smcvb
Copy link
Member

smcvb commented Nov 13, 2017

Opened up a PR to introduce this enhancement (#436).

smcvb added a commit that referenced this issue Nov 14, 2017
Rename all eventRoutingMode occurrences to eventForwardingMode, as that
fits better with the ForwardingMode enum.

[#388]
smcvb added a commit that referenced this issue Nov 14, 2017
Rename routingKeyProperties to commandHandlerRoutingKeys as it clarifies
 the map's intent better.

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Add default description to the eventRoutingKey javadoc

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Remove redundant @return javadoc

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Rename the AbstractChildEntityCollectionDefinition to
AbstractChildEntityDefinition to initiate the reuse in the
AggregateMemberAnnotatedChildEntityDefinition
-Extract the difference from the createChildDefinition() implementations
 in the collection and map definition, and move these as abstract
 function in the AbstractChildEntityDefinition to be implemented by the
 collection and map implementations

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Extend from the AbstractChildEntityDefinition in the
AggregateMemberAnnotatedChildEntityDefinition
-Introduce the extractChildEntityModel()
-Drop the resolveType() and resolveGenericType() in favor of the
extractChildEntityModel()
-Create the commandHandlerRoutingKeys in the
createCommandTargetResolvers() rather then passing it, as it's only a
requirement for the map and iterable impls
-Add dummy javadoc to fill in later

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Some minor reindents and function renames

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Introduce more concise javadoc
-Rename the createResolve...() function to resolve...(), as it's the
actual resolving function rather than the creation of the resolving
function.

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Change event targets from Iterable to Stream

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Correctly create the event target streams
-Remorph stream to a list to solve concurrent modification exceptions

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Move forwarding mode to filter function

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
Move filter logic from the AnnotatedChildEntity to the
AbstractChildEntityDefinition thus to filter the targets at creation,
rather than every time they're called.

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Replace ForwardingMode enum for ForwardingMode interface
-Introduce a ForwardAll ForwardingMode implementation
-Introduce a ForwardNone ForwardingMode implementation
-Introduce a ForwardMatchingInstances ForwardingMode implementation
-Adjust AggregateMember annotation to take in a ForwardingMode class
instead of an enum
-Instantiate the right ForwardingMode in the
AbstractChildEntityDefinition based on the class type in the
AggregateMember
-Filter the event targets based on the eventForwardingMode in the
AbstractChildEntityDefinition implementations
-Fix AggregateMember usages in tests

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Drop the constructors in favor of an initialize() function
-Instead of implementing a boolean filter function, filter a Stream of
candidates directly.
-Drop the useless INSTANCE constants
-Rename the `eventRoutingKey` to `routingKey`
-Update all javadoc accordingly

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Boyscout the javadoc
-Add comment for intermediate '.collect(Collectors.toList())', which
solves a ConcurrentModificationException

[#388]
smcvb added a commit that referenced this issue Nov 23, 2017
-Drop direct if/else checks, in favor of the AggregateMember its
eventForwardingMode property '.newInstance()'
-Call 'initialize()' on the ForwardingMode instance
-Wrap call in try/catch block and throw AxonConfigurationException
-Update the resolveEventTarget its javadoc
-Reorder functions according to when they're used

[#388]
smcvb added a commit that referenced this issue Nov 24, 2017
[#388] Introduce specific Event Routing capabilities for the Entities within an Aggregate
@smcvb
Copy link
Member

smcvb commented Nov 24, 2017

Resolved this issue in #436

@smcvb smcvb closed this as completed Nov 24, 2017
@smcvb smcvb added Type: Feature Use to signal an issue is completely new to the project. Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. and removed Type: Enhancement Use to signal an issue enhances an already existing feature of the project. labels Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2: Should High priority. Ideally, these issues are part of the release they’re assigned to. Status: Resolved Use to signal that work on this issue is done. Type: Feature Use to signal an issue is completely new to the project.
Projects
None yet
Development

No branches or pull requests

2 participants