Skip to content

Conversation

@solonovamax
Copy link
Contributor

Subsystem
Server, ktor-server-di module

Motivation
KTOR-8762

Allows the use of dependency injection within a route:

fun Application.module() {
    dependencies {
        provide<MyService> { MyServiceImpl() }
        provide<MyOtherService> { MyOtherServiceImpl() }
    }

    routing {
        route("/api" {
            myServiceRoutes()
            myOtherServiceRoutes()
        }
    }
}

fun Route.myServiceRoutes() {
    val service: MyService by dependencies

    /* ... */
}

fun Route.myOtherServiceRoutes() {
    val service: MyOtherService by dependencies

    /* ... */
}

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
Public API descriptors
ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api, ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api
Added Route-based DI access: new getDependencies(Route) overload; new Route.dependencies getter; Application.dependencies getter/setter exposed. No removals or behavior changes.
DI extension implementation
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
Implemented Route.dependencies extension returning Application’s DependencyRegistry via attributes; added routing import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • osipxd

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7153013 and 7c80769.

📒 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 correct

The new getDependencies(Route): DependencyRegistry entry correctly reflects the Kotlin extension property Route.dependencies for 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 required

Using a star import for io.ktor.server.routing.* matches the repo’s style guide and is necessary for the new Route extension.

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 — OK

The new Route-scoped dependencies property and its getter are exported for all targets as expected, aligning with the Kotlin source addition.

Comment on lines +193 to +195
public val Route.dependencies: DependencyRegistry
get() = application.attributes[DependencyRegistryKey]

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 13, 2025

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

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

Copy link
Contributor

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 ☝️

Copy link
Contributor

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.

( ˶ᵔ ᵕ ᵔ˶ )

Copy link
Contributor Author

@solonovamax solonovamax Aug 14, 2025

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.

Copy link
Contributor

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!

Copy link
Contributor

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.

Copy link
Member

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?

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

Copy link
Contributor

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 👍

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.

3 participants