-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: create new annotation for classes that allows you to have multiple channels within #76
base: dev
Are you sure you want to change the base?
Conversation
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.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
Hi @azizabah, thanks for the PR. To support this feature you would need to refactor the annotation scanner / processor first. The logic lives in the context module here. In short, we currently only check classes for the |
@lorenzsimon Thanks for the pointer. I will take a look there. |
@lorenzsimon - Updated with a unit test showing it can handle both now. There was a breaking change introduced in Spring Boot 3.4.0 related to mockMVC so that is why I version locked it to 3.3.5 as part of this PR. I would suggest we tackle that as a separate PR. |
...ntext/src/main/kotlin/org/openfolder/kotlinasyncapi/context/annotation/AnnotationProvider.kt
Outdated
Show resolved
Hide resolved
...ntext/src/main/kotlin/org/openfolder/kotlinasyncapi/context/annotation/AnnotationProvider.kt
Outdated
Show resolved
Hide resolved
… the correct classes.
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.
Almost there 🤏
...annotation/src/main/kotlin/org/openfolder/kotlinasyncapi/annotation/AsyncApiDocumentation.kt
Outdated
Show resolved
Hide resolved
...annotation/src/main/kotlin/org/openfolder/kotlinasyncapi/annotation/AsyncApiDocumentation.kt
Outdated
Show resolved
Hide resolved
@@ -81,6 +83,7 @@ class AnnotationProvider( | |||
componentToChannelMapping[clazz.java.simpleName] = | |||
annotation.value.takeIf { it.isNotEmpty() } ?: clazz.java.simpleName | |||
} | |||
is AsyncApiComponent -> asyncApiComponentProcessor.process(annotation, clazz) |
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.
sorry, there is actually still something missing: you see that componentToChannelMapping above? you have to add the the channels you find in the class to this map. because it is used to bind the channel components to the actual channel top level object. not sure if that makes sense. but currently you would only add the channels under components.channels but you also want them in asyncapi.channels.
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.
@lorenzsimon - Ok. I wasn't sure what the purpose of that map was. I will add it.
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.
Updated.
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.
AsyncApiComponent
annotation does not need the value property. You need to map the value of the channel annotation itself. In your example, some/{parameter}/channel
should be the name of the channel. And this is what the bind method is doing. It checks what the value property is, uses this as the name of the channel and then references the channel component based on the class name (or the function name in your case).
{
"channels": {
"some/{parameter}/channel": ...
}
}
I think we should add it to the spring integration test:
Line 177 in df0aa85
val expected = TestUtils.json("async_api_annotation_integration.json") |
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.
@lorenzsimon - AsyncApiComponent is the class though not the function, we changed the behavior at your request. There can be multiple channels inside an AsyncApiComponent so you want me to replicate that logic to populate this map?
Something like this (which doesn't work):
is AsyncApiComponent -> asyncApiComponentProcessor.process(annotation, clazz).also { processedComponents ->
processedComponents.channels?.forEach { (channelName, _) ->
componentToChannelMapping[channelName] = channelName
}
}
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.
Ok. Looking at the integration test was the clue I needed to find the issues with my implementation. This should be resolved now.
…nnel behavior. added value attribute to AsyncApiComponent for parity.
btw sorry for the merge conflicts. I will resolve them once the PR is ready :) |
…ent and for Channel annotations to handle the fact that not all of them will be named based on their class and that will only happen if they target a class instead of a function.
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.
Let me know if you want me to finish the PR. I would have some time next weekend.
if (clazz.annotations.any { it is Channel }) { | ||
componentToChannelMapping[clazz.java.simpleName] = | ||
annotation.value.takeIf { it.isNotEmpty() } ?: clazz.java.simpleName | ||
} | ||
} |
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.
Why is this if check needed? We already made sure that class is annotated with @Channel
in the flatMap step before.
@@ -102,6 +104,10 @@ internal open class AsyncApiAnnotationAutoConfiguration { | |||
open fun channelProcessor() = | |||
ChannelProcessor() | |||
|
|||
@Bean | |||
open fun asyncApiDocumentationProcessor() = |
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.
open fun asyncApiDocumentationProcessor() = | |
open fun asyncApiComponentProcessor() = |
} | ||
}, | ||
"channels": { | ||
"my/channel": { |
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.
Should be TestChannel
} | ||
is AsyncApiComponent -> asyncApiComponentProcessor.process(annotation, clazz).also { processedComponents -> | ||
processedComponents.channels?.forEach { (channelName, _) -> | ||
componentToChannelMapping[channelName] = channelName |
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.
You could iterate over the class functions here already. Then you can change the component processor to process the functions instead of the class: private val asyncApiComponentProcessor: AnnotationProcessor<AsyncApiComponent, KFunction<*>>
Also you need to use componentToChannelMapping[function.name] = annotation.value
because there could be multiple channels in the class.
What
This change allows for a new annotation, @AsyncApiDocumentation, to be added to a class. This then allows you to have multiple "Channels" within that class that will each be processed for documentation correctly.
Why
This will allow @channel documentation to be used on functions that create beans (for example) to keep verboseness to a minimum in a code base. This fixes #75
How
A new annotation is created and it it finds classes with that annotation and then looks for functions within that have been annotated with Channel and builds documentation based on that.