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

feat: create new annotation for classes that allows you to have multiple channels within #76

Merged
merged 18 commits into from
Jan 6, 2025

Conversation

azizabah
Copy link
Contributor

@azizabah azizabah commented Dec 5, 2024

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.

Copy link

@github-actions github-actions bot left a 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.

@azizabah azizabah marked this pull request as ready for review December 5, 2024 17:07
@lorenzsimon
Copy link
Member

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 @Channel annotation because we don't rely on Spring annotation processor and would need to check all functions of all classes to find it otherwise.
Feel free to suggest another method for it though :)

@azizabah
Copy link
Contributor Author

azizabah commented Dec 9, 2024

@lorenzsimon Thanks for the pointer. I will take a look there.

@azizabah
Copy link
Contributor Author

@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.

@azizabah azizabah changed the title feat: allow annotation to target function also. feat: create new annotation for classes that allows you to have multiple channels within Dec 10, 2024
Copy link
Member

@lorenzsimon lorenzsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there 🤏

@@ -81,6 +83,7 @@ class AnnotationProvider(
componentToChannelMapping[clazz.java.simpleName] =
annotation.value.takeIf { it.isNotEmpty() } ?: clazz.java.simpleName
}
is AsyncApiComponent -> asyncApiComponentProcessor.process(annotation, clazz)
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link
Member

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:

Copy link
Contributor Author

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
                        }
                    }

Copy link
Contributor Author

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.

Charles Bazeley and others added 3 commits December 11, 2024 16:29
…nnel behavior. added value attribute to AsyncApiComponent for parity.
* feat: Implement message model annotations (asyncapi#43)

* feat - Implement message model annotations

* chore - Remove unused dependencies

* feat: Process message and schema annotations (asyncapi#44)

* feat - Implement Message annotation processing

* feat - Merge annotation components

* feat - Add schema annotation processor

* refactor - Make annotation processor context dynamic
test - Add annotation provider integration test

* chore - ktlint format

* refactor - Refactor dependency management

---------

Co-authored-by: lorenzsimon <lorenz.simon.mail@gmail.com>

* feat: Channel annotation processing (asyncapi#45)

* feat - Add Channel and Operation annotations

* feat - Add Channel processing

* refactor - Annotation keys to values

* chore - Fix confusing test values

* refactor: Annotation mapping (asyncapi#47)

* refactor - Annotation mapping improvements

* refactor - Add option for inline messages and schemas

* refactor - Use classname for channel component keys if autogenerated

* fix - Typo

* test - Fix Schemas test

* feat: Add Kotlin module to model resolver (asyncapi#48)

feat - Add Kotlin module to model resolver

* feat: Bind channels to annotation components (asyncapi#49)

* refactor - Context providers

* feat - Bind channels to annotation components

* refactor - Annotation components binding

* chore - Format

* chore: Add Spring Boot example application (asyncapi#63)

* chore: Add Spring Boot example application

* fix: Java version

* fix: Test

* chore: Bump dependencies (asyncapi#65)

* chore: Bump dependencies

* chore: Bump dependencies

* chore: Set Java version in GH actions

* fix: Autoconfig migration

* fix: Migrate Jakarta

* chore: Refactor data objects (asyncapi#67)

* chore: Revert

* release: 3.0.3

* pre-release: 3.0.4

* feat: Add Ktor integration (asyncapi#72)

* docs: Update Spring support

* pre-release: 3.0.4 (asyncapi#70)

* docs: Link Spring Boot example

* Add Ktor integration

* Fix content type

* Fix test

* Fix test

* Add Script extension

* Add Script extension

* Refactor

* Add plugin integration test

* Add docs for ktor integration

* Move to new domain namespace

* Fix kts example usage

* Move domain

* Bump dependencies

---------

Co-authored-by: Lorenz Simon <95355196+lorenz-scalable@users.noreply.github.com>
@lorenzsimon
Copy link
Member

lorenzsimon commented Dec 15, 2024

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.
@azizabah azizabah requested a review from lorenzsimon December 19, 2024 14:41
@lorenzsimon lorenzsimon changed the base branch from master to dev December 22, 2024 07:58
Copy link
Member

@lorenzsimon lorenzsimon left a 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.

}
},
"channels": {
"my/channel": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be TestChannel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - in places where the channel is inside a class, it should be the value of the annotation not the name of the class. https://github.com/azizabah/kotlin-asyncapi/blob/bc83ab4f5825d56f7741f126fce481bad57e8c6b/kotlin-asyncapi-spring-web/src/test/kotlin/org/openfolder/kotlinasyncapi/springweb/controller/AsyncApiControllerIntegrationTest.kt#L251

If you do it as the value of the class, you will run into conflicts when you have multiple inside the same class which is the exact functionality this entire PR is about.

}
is AsyncApiComponent -> asyncApiComponentProcessor.process(annotation, clazz).also { processedComponents ->
processedComponents.channels?.forEach { (channelName, _) ->
componentToChannelMapping[channelName] = channelName
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am handling iterating over the values inside the AsyncApiComponentProcessor. If you look at that code, you can also see, I'm setting the value to the annotation value not the function name. I think this is clearer for users and provides a better experience than forcing the function name to be there.

@azizabah azizabah requested a review from lorenzsimon January 6, 2025 15:37
@lorenzsimon
Copy link
Member

@azizabah can you resolve the merge conflicts? I don't have write access to your fork.

Charles Bazeley and others added 3 commits January 6, 2025 14:51
# Conflicts:
#	kotlin-asyncapi-annotation/src/main/kotlin/com/asyncapi/kotlinasyncapi/annotation/AsyncApiComponent.kt
#	kotlin-asyncapi-context/src/main/kotlin/com/asyncapi/kotlinasyncapi/context/annotation/AnnotationProvider.kt
#	kotlin-asyncapi-context/src/main/kotlin/com/asyncapi/kotlinasyncapi/context/annotation/processor/AsyncApiComponentProcessor.kt
#	kotlin-asyncapi-context/src/test/kotlin/com/asyncapi/kotlinasyncapi/context/annotation/processor/AsyncApiComponentProcessorTest.kt
#	kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt
#	kotlin-asyncapi-spring-web/src/main/kotlin/com/asyncapi/kotlinasyncapi/springweb/AsyncApiAutoConfiguration.kt
#	kotlin-asyncapi-spring-web/src/test/kotlin/com/asyncapi/kotlinasyncapi/springweb/controller/AsyncApiControllerIntegrationTest.kt
@azizabah
Copy link
Contributor Author

azizabah commented Jan 6, 2025

can you resolve the merge conflicts? I don't have write access to your fork.

Merges resolved @lorenzsimon

@lorenzsimon lorenzsimon merged commit 2688d55 into asyncapi:dev Jan 6, 2025
9 checks passed
lorenzsimon added a commit that referenced this pull request Jan 6, 2025
* feat: Implement message model annotations (#43)

* feat - Implement message model annotations

* chore - Remove unused dependencies

* feat: Process message and schema annotations (#44)

* feat - Implement Message annotation processing

* feat - Merge annotation components

* feat - Add schema annotation processor

* refactor - Make annotation processor context dynamic
test - Add annotation provider integration test

* chore - ktlint format

* refactor - Refactor dependency management

---------

Co-authored-by: lorenzsimon <lorenz.simon.mail@gmail.com>

* feat: Channel annotation processing (#45)

* feat - Add Channel and Operation annotations

* feat - Add Channel processing

* refactor - Annotation keys to values

* chore - Fix confusing test values

* refactor: Annotation mapping (#47)

* refactor - Annotation mapping improvements

* refactor - Add option for inline messages and schemas

* refactor - Use classname for channel component keys if autogenerated

* fix - Typo

* test - Fix Schemas test

* feat: Add Kotlin module to model resolver (#48)

feat - Add Kotlin module to model resolver

* feat: Bind channels to annotation components (#49)

* refactor - Context providers

* feat - Bind channels to annotation components

* refactor - Annotation components binding

* chore - Format

* chore: Add Spring Boot example application (#63)

* chore: Add Spring Boot example application

* fix: Java version

* fix: Test

* chore: Bump dependencies (#65)

* chore: Bump dependencies

* chore: Bump dependencies

* chore: Set Java version in GH actions

* fix: Autoconfig migration

* fix: Migrate Jakarta

* chore: Refactor data objects (#67)

* chore: Revert

* release: 3.0.3

* pre-release: 3.0.4

* feat: Add Ktor integration (#72)

* docs: Update Spring support

* pre-release: 3.0.4 (#70)

* docs: Link Spring Boot example

* Add Ktor integration

* Fix content type

* Fix test

* Fix test

* Add Script extension

* Add Script extension

* Refactor

* Add plugin integration test

* Add docs for ktor integration

* Move to new domain namespace

* Fix kts example usage

* Move domain

* Bump dependencies

* feat: create new annotation for classes that allows you to have multiple channels within (#76)

* allow annotation to target function also.

* add support for channel on functions instead of just classes.

* split into a separate processor with new annotation to handle finding the correct classes.

* match previous white space

* name updates per PR comments.

* fix annotation location

* add AsyncApiComponent population of channel map behavior to match Channel behavior. added value attribute to AsyncApiComponent for parity.

* chore: move domain (#77)

* feat: Implement message model annotations (#43)

* feat - Implement message model annotations

* chore - Remove unused dependencies

* feat: Process message and schema annotations (#44)

* feat - Implement Message annotation processing

* feat - Merge annotation components

* feat - Add schema annotation processor

* refactor - Make annotation processor context dynamic
test - Add annotation provider integration test

* chore - ktlint format

* refactor - Refactor dependency management

---------

Co-authored-by: lorenzsimon <lorenz.simon.mail@gmail.com>

* feat: Channel annotation processing (#45)

* feat - Add Channel and Operation annotations

* feat - Add Channel processing

* refactor - Annotation keys to values

* chore - Fix confusing test values

* refactor: Annotation mapping (#47)

* refactor - Annotation mapping improvements

* refactor - Add option for inline messages and schemas

* refactor - Use classname for channel component keys if autogenerated

* fix - Typo

* test - Fix Schemas test

* feat: Add Kotlin module to model resolver (#48)

feat - Add Kotlin module to model resolver

* feat: Bind channels to annotation components (#49)

* refactor - Context providers

* feat - Bind channels to annotation components

* refactor - Annotation components binding

* chore - Format

* chore: Add Spring Boot example application (#63)

* chore: Add Spring Boot example application

* fix: Java version

* fix: Test

* chore: Bump dependencies (#65)

* chore: Bump dependencies

* chore: Bump dependencies

* chore: Set Java version in GH actions

* fix: Autoconfig migration

* fix: Migrate Jakarta

* chore: Refactor data objects (#67)

* chore: Revert

* release: 3.0.3

* pre-release: 3.0.4

* feat: Add Ktor integration (#72)

* docs: Update Spring support

* pre-release: 3.0.4 (#70)

* docs: Link Spring Boot example

* Add Ktor integration

* Fix content type

* Fix test

* Fix test

* Add Script extension

* Add Script extension

* Refactor

* Add plugin integration test

* Add docs for ktor integration

* Move to new domain namespace

* Fix kts example usage

* Move domain

* Bump dependencies

---------

Co-authored-by: Lorenz Simon <95355196+lorenz-scalable@users.noreply.github.com>

* chore: Update repositories

* change how the map is populated for channels found via AsyncApiComponent 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.

* Update CODEOWNERS

* chore: Update pom.xml

* Update CODEOWNERS

* changes per PR requests.

* updates to handle merge conflicts.

---------

Co-authored-by: Charles Bazeley <charles.bazeley@gm.com>
Co-authored-by: Lorenz Simon <lorenz.simon.mail@gmail.com>
Co-authored-by: Lorenz Simon <95355196+lorenz-scalable@users.noreply.github.com>

* Revert version constraint

---------

Co-authored-by: Lorenz Simon <95355196+lorenz-scalable@users.noreply.github.com>
Co-authored-by: CharlesB <azizabah@users.noreply.github.com>
Co-authored-by: Charles Bazeley <charles.bazeley@gm.com>
@azizabah azizabah deleted the azizabah/add-channel branch January 7, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] @Channel annotation should be able to target function in addition to class
2 participants