AxonFramework / AxonFramework

Framework for Evolutionary Message-Driven Microservices on the JVM
https://axoniq.io/
Apache License 2.0
3.31k stars 789 forks source link

Reject or warn about wrong usage of `@EventHandler` and `@EventSourcedHandler` annotations #1755

Open azzazzel opened 3 years ago

azzazzel commented 3 years ago

Enhancement Description

It would be good if the framework can prevent developers from (or at least warn them about) using @EventHandler and @EventSourcedHandler annotations in the wrong contexts.

Current Behaviour

In Aggregate - a method annotated with @EventHandler behaves exactly like if it was annotated with @EventSourcedHandler

In non-Aggregate (event handling component) - a method annotated with @EventSourcedHandler is silently ignored.

Wanted Behaviour

In Aggregate - a method annotated with @EventHandler should result in error or warning

In non-Aggregate (event handling component) - a method annotated with @EventSourcedHandler should result in error or warning

sandjelkovic commented 2 years ago

The issue with this issue is that there is no deterministic way to distinguish these 2 annotations based on the context. Some of the reasons for this are:

In addition to all this, erroring and aborting the app startup would be a breaking change, the only reasonable solution would be to log the issue as a warning.

Resolving this will require remodelling or extending some of the framework's internal classes. While possible, it is not a trivial effort and will be better grouped with some other features that will have to redesign the handler inspectors or annotations in the future.

smcvb commented 2 years ago

Thanks for the clarification, @sandjelkovic. Based on what you've shared, I think it's fair we change the priority to a 4 right now and keep the on-hold status effective.

Blackdread commented 2 years ago

And would a java agent (at compilation time) could do that? So instead it would fail the compilation.

CodeDrivenMitch commented 2 years ago

This could be a feature request for the IntelliJ plugin, showing an error if it has the wrong annotation. https://github.com/AxonFramework/IdeaPlugin

Blackdread commented 2 years ago

Most devs uses Intellij IDEA but would be great if could also work on a Maven plugin (maybe also gradle but I prefer maven). And with a plugin, there could be checks done in CI, etc.

smcvb commented 2 years ago

Removed the milestone for now, as we will not be able to resolve this issue within release 4.6.0. We will adjust this ticket accordingly once we pick it up again.

smcvb commented 1 month ago

Added this issue to release 5.0.0 as we intend to validate the feasibility of this in the new APIs.