-
Notifications
You must be signed in to change notification settings - Fork 21
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
Take class hierarchy into account when resolving #290
Conversation
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/[email protected]
with:
upload-result: true Contact Qodana teamContact us at [email protected]
|
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 👍
@@ -29,7 +29,7 @@ import org.axonframework.intellij.ide.plugin.util.creatorResolver | |||
* Provides a gutter icon on all generic handler methods | |||
*/ | |||
class CommonHandlerMethodLineMarkerProvider : AbstractHandlerLineMarkerProvider() { | |||
private val blacklistedTypes = listOf(MessageHandlerType.DEADLINE, MessageHandlerType.COMMAND, MessageHandlerType.COMMAND_INTERCEPTOR) | |||
private val blacklistedTypes = listOf(MessageHandlerType.DEADLINE) |
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.
Not sure that I comprehend why only the DEADLINE
is left for black listing. Care to explain?
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 class marks any @MessageHander
annotated method, unless that handler type is in the list.
Before this PR, a command handler had a specific one, but with the interceptors gone it's not needed. Aditionally, in the future, the interceptors will be generic for any type of message and implemented in this provider.
The reason deadline is still here is because this is a specific one (DeadlineHandlerMethodLineMarkerProvider
) because it uses the DeadlineManagerReferenceResolver
instead of the MessageHandlerResolver
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.
I left it a list, because maybe we need different MarkerProviders later after all
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.
Thanks for the explanation; it makes sense to me!
Both when resolving handlers and creators, the hierarchy of classes was not interpreted correctly. This has been resolved. In addition, I discovered that the current interceptor implementation stood in the way of this fix, and that it should be implemented in another way. Interceptors are not handlers, after all, but this mechanism was abused for that. As such, the CommandHandlerInterceptor support has been removed. This may be re-added in the future.
# Conflicts: # CHANGELOG.md # src/main/kotlin/org/axonframework/intellij/ide/plugin/resolving/handlers/searchers/CommandHandlerInterceptorSearcher.kt # src/main/kotlin/org/axonframework/intellij/ide/plugin/resolving/handlers/types/CommandHandlerInterceptor.kt
41ef75f
to
0425dbc
Compare
Both when resolving handlers and creators, the hierarchy of classes was not interpreted correctly. This has been resolved.
In addition, I discovered that the current interceptor implementation stood in the way of this fix, and that it should be implemented in another way. Interceptors are not handlers, after all, but this mechanism was abused for that. As such, the CommandHandlerInterceptor support has been removed. This may be re-added in the future.