fix: lazily initialize AnnotationProvider in AsyncApiModule#238
fix: lazily initialize AnnotationProvider in AsyncApiModule#238Ishita-190 wants to merge 2 commits intoasyncapi:masterfrom
Conversation
Ishita-190
requested review from
asyncapi-bot-eve,
gimlet2 and
lorenzsimon
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughReplaced a conditional, nullable annotation-based AsyncApiExtension with a concrete anonymous AsyncApiExtension that is always present; it defines a fixed order and lazily delegates to an annotation-derived extension when annotation data exists, otherwise delegating to a no-op implementation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Ishita-190
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt (1)
61-71:⚠️ Potential issue | 🟡 MinorThe PR correctly defers classpath scanning to the first request.
The
AnnotationProviderconstructor does not trigger classpath scanning. The scanning happens lazily when theasyncApiproperty is first accessed (line 42 in AnnotationProvider.kt usesby lazy), and this property is only accessed whenextend()is called on line 78. So the change achieves the stated objective of deferring classpath scanning from module initialization to first request.However, the type declaration on line 73 is misleading:
asyncApiAnnotationExtensionis typed asAsyncApiExtension?but is always assigned a non-null object. Change toAsyncApiExtension. Additionally,AsyncApiExtension.from()is recreated on everyextend()call; consider caching it after the first call for efficiency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt` around lines 61 - 71, The asyncApiAnnotationExtension field is declared nullable but always assigned and AsyncApiExtension.from() is recreated on every extend() call; change asyncApiAnnotationExtension to non-null AsyncApiExtension and cache the result of AsyncApiExtension.from(...) when first created so subsequent calls to extend() reuse the same instance. Locate AnnotationProvider construction and the extend() method, update the declaration of asyncApiAnnotationExtension to type AsyncApiExtension (not AsyncApiExtension?), compute and store the AsyncApiExtension instance once (e.g., in a private val cachedAsyncApiExtension = AsyncApiExtension.from(...)) and have extend() reference that cachedAsyncApiExtension instead of calling AsyncApiExtension.from() repeatedly. Ensure nullability is removed and usages updated accordingly.
🧹 Nitpick comments (1)
kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt (1)
73-81: Nullable type is misleading — the extension is never null.
asyncApiAnnotationExtensionis typed asAsyncApiExtension?but is always assigned a non-null anonymous object. Previously (with the?.letpattern) it could be null when there were no annotations, causinglistOfNotNullon line 106 to filter it out. Now it's always present in the extensions list and acts as a no-op whenannotationProvider.asyncApireturns null.This is functionally fine but the nullable type is misleading. Consider changing to
AsyncApiExtension.Additionally, the linked issue
#237specifies guards that aren't implemented here:
- Skip creating the extension entirely when
scanAnnotations == false.- Log a warning when
scanAnnotations == truebutbaseClass == null.- Only scan when both conditions are met.
These requirements would prevent the unbounded classpath scan the issue describes.
Suggested type fix
- private val asyncApiAnnotationExtension: AsyncApiExtension? = + private val asyncApiAnnotationExtension: AsyncApiExtension = object : AsyncApiExtension {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt` around lines 73 - 81, The field asyncApiAnnotationExtension is declared nullable but always initialized to a non-null anonymous AsyncApiExtension, which is misleading and causes unintended scanning behavior; change its type to AsyncApiExtension (remove the ?) and modify construction so the extension is only created when scanAnnotations is true and baseClass is non-null: if scanAnnotations is false do not create/append the extension, if scanAnnotations is true but baseClass is null log a warning and do not create it, and when creating the extension make it consult annotationProvider.asyncApi as before (so listOfNotNull usage at listOfNotNull(...) will correctly include or exclude the extension); update references to asyncApiAnnotationExtension accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt`:
- Around line 61-71: The asyncApiAnnotationExtension field is declared nullable
but always assigned and AsyncApiExtension.from() is recreated on every extend()
call; change asyncApiAnnotationExtension to non-null AsyncApiExtension and cache
the result of AsyncApiExtension.from(...) when first created so subsequent calls
to extend() reuse the same instance. Locate AnnotationProvider construction and
the extend() method, update the declaration of asyncApiAnnotationExtension to
type AsyncApiExtension (not AsyncApiExtension?), compute and store the
AsyncApiExtension instance once (e.g., in a private val cachedAsyncApiExtension
= AsyncApiExtension.from(...)) and have extend() reference that
cachedAsyncApiExtension instead of calling AsyncApiExtension.from() repeatedly.
Ensure nullability is removed and usages updated accordingly.
---
Nitpick comments:
In
`@kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.kt`:
- Around line 73-81: The field asyncApiAnnotationExtension is declared nullable
but always initialized to a non-null anonymous AsyncApiExtension, which is
misleading and causes unintended scanning behavior; change its type to
AsyncApiExtension (remove the ?) and modify construction so the extension is
only created when scanAnnotations is true and baseClass is non-null: if
scanAnnotations is false do not create/append the extension, if scanAnnotations
is true but baseClass is null log a warning and do not create it, and when
creating the extension make it consult annotationProvider.asyncApi as before (so
listOfNotNull usage at listOfNotNull(...) will correctly include or exclude the
extension); update references to asyncApiAnnotationExtension accordingly.
|
@lorenzsimon I've made the changes, please take a look! |
1 participant
Fixes: #237 (comment)
What
Made changes to :
kotlin-asyncapi-ktor/src/main/kotlin/com/asyncapi/kotlinasyncapi/ktor/AsyncApiModule.ktso that the
AnnotationProviderperforms classpath scanning lazily on the first request, instead of eagerly at application startup.How
asyncApiAnnotationExtensionto defer execution until extend is called.All changes work as intended.
Summary by CodeRabbit