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
Fix duplicate command handler resolver not being used for annotated aggregates #2207
Fix duplicate command handler resolver not being used for annotated aggregates #2207
Conversation
446834a
to
4b8f007
Compare
Kudos, SonarCloud Quality Gate passed! |
verify(commandBus).subscribe(eq(UpdateCommandWithAnnotatedMethod.class.getName()), | ||
// Is subscribed two times because of the duplicate handler. This is good and indicates usage of the | ||
// DuplicateCommandHandlerResolver | ||
verify(commandBus, times(2)).subscribe(eq(UpdateCommandWithAnnotatedMethod.class.getName()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should be obsolete with the change in the AnnotatedHandlerInspector
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by obsolete? the times(2)
change is valid, see the comment. I have added an additional handler, with a different signature. The change in AnnotatedHandlerInspector
is to prevent abstract methods from being registered twice (for the same implementation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aaah thanks for clarifying, I wasn't thinking straight enough when I wrote that I guess 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
When the AggregateAnnotationCommandHandler subscribes to the commandBus, it does so once for every payload type. This is independent of how many handlers are present for that type, since it does so using a set. In addition, it passes itself as the handler, making the DuplicateCommandHandlerResolver useless since comparing handlers is impossible (both handler arguments are the AggregateAnnotationCommandHandler itself, instead of the conflicting handlers).
In this PR this has been changed so the AggregateAnnotationCommandHandler subscribes the child command handlers to the CommandBus, instead of itself. This bypasses its handle and canHandle methods (since it is never really subscribed), but then uses these of the handlers in its list.
In addition, this PR solves an issue with the
AnnotatedHandlerInspector
. Even when methods were abstract in a superclass, they were considered a member. This leads to double registrations on the command bus.Closes #1756