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

AnnotationEventHandlerAdapter returns prematurely #1058

Open
pepperbob opened this issue Apr 18, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@pepperbob
Copy link
Contributor

commented Apr 18, 2019

The AnnotationEventHandlerAdapter which is responsible for delegating method calls to the individual event-handler methods will return prematurely if there is more than one @EventHandler method that can handle the event.

Looking at the code in AnnotationEventHandlerAdapter:81 the loop is stopped immidiately after the call to the first event handler.

While it is certainly reasonable to return a value from other MessageHandlers, e.g. CommandHandlers it does not make much sense in the EventHandling context where there are many by-design and return values are generally ignored anyways.

The issue can easily be fixed by removing the return statement from line 81.

@pepperbob

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

Apparently, there are more variations of filtering of EventHandlers:

This still looks like some bug or at least an unnecessary restriction imposed by implemenation details, but maybe there is a specific reason for this?

Use case: we'd like to enrich semantics on our events by adding interfaces to them. That means two events can share the same abstraction/s.

@abuijze

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

Hi @pepperbob,

this is actually by design. The semantics of the handler invocation is that the most specific handler matching the incoming message is invoked on an instance. Since the handlers are sorted by how specific they are, it can stop after the first invocation.

See documentation: https://docs.axoniq.io/reference-guide/implementing-domain-logic/event-handling/handling-events

In all circumstances, at most one event handler method is invoked per listener instance. Axon will search for the most specific method to invoke, using the following rules:

  1. On the actual instance level of the class hierarchy (as returned by this.getClass()), all annotated methods are evaluated
  2. If one or more methods are found of which all parameters can be resolved to a value, the method with the most specific type is chosen and invoked
  3. If no methods are found on this level of the class hierarchy, the supertype is evaluated the same way
  4. When the top level of the hierarchy is reached, and no suitable event handler is found, the event is ignored.
@pepperbob

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

@abuijze, thanks for your explanation.

Still I really can't get my head around how this behavior is helpful in the case of event-handlers:
conceptually there are many of the same type and we simply register a bunch of @EventHandler-annotated methods - so why should the framework matter if I spread these across classes or keep them within a single place?

I think the current behavior even makes some situations more or less unpredictable, e.g. in this example it's not obvious which method will be called and why:

interface ConceptA
interface ConceptB

class EventA implements ConceptA, ConceptB

...
@EventHandler
void on(ConceptB e) {
}

@EventHandler
void on(ConceptA e) { 
}
...

When working with interfaces to describe shared aspects of events, it gives us a hard time dealing with these efficiently because we have to be really careful that the framework won't silently ignore registered @EventHandlers - especially when existing concepts are amended.

Unfortunately the described behavior is currently hard-coded for the most part (Sagas and Aggregates) and can only be overwritten for ordinary EventHandlers (by using a custom MessageHandler), rendering event handling inconsistent.

Do you think this has a chance to be generalized and made configurable for all event handlers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.