-
Notifications
You must be signed in to change notification settings - Fork 1.2k
KTOR-8308 Extension for using dependency injection within a route #5042
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
base: main
Are you sure you want to change the base?
KTOR-8308 Extension for using dependency injection within a route #5042
Conversation
Signed-off-by: solonovamax <[email protected]>
WalkthroughAdds Route-scoped access to DependencyRegistry alongside existing Application-level access by introducing new public API overloads/properties and a Route.dependencies extension delegating to Application attributes. No existing logic is changed; only the API surface is expanded. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api(1 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide
Use star imports for io.ktor.* packages (configured in .editorconfig)
Max line length: 120 characters
Document all public APIs including parameters, return types, and exceptions
Mark internal APIs with @internalapi annotation
Error handling follows Kotlin conventions with specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
**/*.{kt,kts,json,yaml,yml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Indent: 4 spaces (2 for JSON, YAML)
Files:
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
🧠 Learnings (3)
📚 Learning: 2025-05-14T18:17:03.059Z
Learnt from: bjhham
PR: ktorio/ktor#4855
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:37-38
Timestamp: 2025-05-14T18:17:03.059Z
Learning: When implementing CoroutineScope in Ktor components like DependencyRegistry, always ensure the scope is properly cancelled during application shutdown to prevent memory leaks and resource issues. In the DI plugin, this is handled by calling registry.cancel() in the ApplicationStopped event handler after all dependency cleanup is complete.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api
📚 Learning: 2025-05-14T18:17:03.059Z
Learnt from: bjhham
PR: ktorio/ktor#4855
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:37-38
Timestamp: 2025-05-14T18:17:03.059Z
Learning: In the Ktor DI plugin, when a DependencyRegistry implements CoroutineScope, it should be properly cancelled during application shutdown to prevent memory leaks and resource issues. This is handled by calling registry.cancel() in the ApplicationStopped event.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api
📚 Learning: 2025-07-21T12:06:11.772Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T12:06:11.772Z
Learning: Applies to **/*.{kt,kts} : Error handling follows Kotlin conventions with specific Ktor exceptions
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
🔇 Additional comments (3)
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api (1)
280-280: Public API addition for Java interop looks correctThe new
getDependencies(Route): DependencyRegistryentry correctly reflects the Kotlin extension propertyRoute.dependenciesfor Java/ABI consumers. No compatibility concerns.ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt (1)
8-8: Import conforms to repo style and is requiredUsing a star import for
io.ktor.server.routing.*matches the repo’s style guide and is necessary for the newRouteextension.ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api (1)
400-401: Klib ABI includes Route.dependencies across all targets — OKThe new Route-scoped
dependenciesproperty and its getter are exported for all targets as expected, aligning with the Kotlin source addition.
| public val Route.dependencies: DependencyRegistry | ||
| get() = application.attributes[DependencyRegistryKey] | ||
|
|
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.
🛠️ Refactor suggestion
Delegate to Application.dependencies to auto-install DI and avoid NoSuchElementException
Accessing application.attributes[DependencyRegistryKey] will throw if the DI plugin isn’t installed. The Application-level property already handles auto-install (install(DI)), so delegate to it for consistent and safer behavior.
Additionally, this is a new public API; it should have KDoc like the rest of this file.
Apply this diff to fix the getter:
-public val Route.dependencies: DependencyRegistry
- get() = application.attributes[DependencyRegistryKey]
+public val Route.dependencies: DependencyRegistry
+ get() = application.dependenciesAdd KDoc above the property (outside the changed lines):
/**
* Provides route-scoped access to the application's [DependencyRegistry].
*
* For use inside routing builders and extension functions:
* val service: MyService by dependencies
*
* This forwards to [Application.dependencies] and will install the DI plugin if missing.
*
* [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.plugins.di.Route.dependencies)
*/🤖 Prompt for AI Agents
In
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
around lines 193 to 195, the Route.dependencies getter currently accesses
application.attributes[DependencyRegistryKey] which can throw if the DI plugin
isn't installed and is inconsistent with Application.dependencies; change the
getter to delegate to application.dependencies so it will auto-install the DI
plugin and avoid NoSuchElementException, and add the provided KDoc block above
the public property (outside the changed lines) describing route-scoped access,
usage example, that it forwards to Application.dependencies and will install DI
if missing, and include the feedback URL.
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.
The rabbit is correct in this matter ☝️
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.
@bjhham Thank you for confirming! I'm glad the suggestion to delegate to Application.dependencies and add proper KDoc makes sense. This will indeed make the Route-level dependency access more robust and consistent with the existing Application-level behavior.
( ˶ᵔ ᵕ ᵔ˶ )
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.
The rabbit is correct in this matter ☝️
having it throw is intentional, as dependencies should be configured at an application-level rather than at a route level (as if you configured it at a route level, you would expect it to be scoped to the route, however it is not, as doing so is rather out of scope for the PR)
also, because Application.dependencies does not have any documentation, I omitted documentation on this.
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
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 am thinking now that we may want to avoid introducing this extension. It is better to always resolve dependencies during application startup, because it allows for validation during startup and avoids unnecessary lookups. I'm concerned this would encourage people to resolve dependencies during request processing, which is not ideal.
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 think it might be okay to introduce Route.dependencies, but not RoutingContext.dependencies. After fixing DSL scope annotations (#5058) usage of this extension in RoutingContext scope will throw a compilation error. So all dependencies will be resolved at the moment of routing initialization. @bjhham wdyt?
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 am thinking now that we may want to avoid introducing this extension. It is better to always resolve dependencies during application startup, because it allows for validation during startup and avoids unnecessary lookups. I'm concerned this would encourage people to resolve dependencies during request processing, which is not ideal.
they are resolved at application startup. can routes even be added at any point after that? and you could even already do it at runtime by just doing like
fun Application.whatever() {
routing {
get("/") {
val myService: MyService by dependencies
}
}
}however, I do actually think it would be actually useful to be able to resolve it at runtime
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.
Ah this is true, I mixed up Route and RoutingContext. The naming is a bit confusing in my defense. @solonovamax that snippet is valid currently in 3.0, but we're planning on fixing the DSL scope annotations so application references will need to be explicit. There can be uses for accessing dependencies after startup, but in general it's better practice to validate everything as early as possible. The routing DSL generally runs at the same time as the modules so we should be good to go there 👍
Subsystem
Server,
ktor-server-dimoduleMotivation
KTOR-8762
Allows the use of dependency injection within a route: