KTOR-8809 Fix the use of DSL markers #5058
Conversation
WalkthroughReorganized usage of the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
Show resolved
Hide resolved
| * [Report a problem](https://ktor.io/feedback/?fqname=io.ktor.server.jetty.jakarta.JettyApplicationEngineBase.Configuration.configureServer) | ||
| */ | ||
| public var configureServer: Server.() -> Unit = {} | ||
| public var configureServer: (@KtorDsl Server).() -> Unit = {} |
There was a problem hiding this comment.
This way the DSL scope works on Kotlin 2.2
| configuration() | ||
| } | ||
| } | ||
| public fun Route.hx(configuration: HxRoute.() -> Unit): Route = hx.apply(configuration) |
There was a problem hiding this comment.
DSL scope has helped to find incorrect code. Nice!
| /** | ||
| * Shortcut to get an [Application] for this [RoutingContext]. | ||
| */ | ||
| public val RoutingContext.application: Application |
There was a problem hiding this comment.
Added this to keep usage of application working inside RoutingScope
| public class RoutingContext( | ||
| public val call: RoutingCall | ||
| ) | ||
| ) : CoroutineScope by call |
There was a problem hiding this comment.
We have some calls of launch and async in RoutingContext in tests. Actually they're implicitly called on the Application instance. There are two ways of what to do:
- Make
RoutingContextimplementCoroutinesScopeto keep these calls working, but behavior might change (for better, I believe). This would also fix KTOR-8501 - Refactor these calls to
call.launch { ... }andcall.async { ... }. Breaks existing code, but explicitly declares behavior change.
There was a problem hiding this comment.
Okay. The current implementation of the first option is problematic. Calling coroutineContext on RoutingContext returns different CoroutineContext which doesn't contain some coroutine elements. So probably it's better to go with the second option, which is more breaking.
There was a problem hiding this comment.
Yeah option 2 would be best. If I recall correctly, one of the intentional breaking changes in 3.0 was to force the explicit call.launch instead of having the scope in the receiver, but with the broken DSL markers it's easy to accidentally launch from the application scope 😜
| } | ||
| get("/bank-teller") { | ||
| val service: BankTeller = dependencies.resolve() | ||
| val service: BankTeller = application.dependencies.resolve() |
There was a problem hiding this comment.
I don't know the best way to fix these ones. We can either resolve dependencies at the application level, or use application.depenencies in routing context
There was a problem hiding this comment.
Prefixing with application is best here.
| routing { | ||
| install(CORS) { | ||
| route("/abc") { | ||
| get { | ||
| call.respond("OK") | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This is not valid anymore. Declaring route inside Plugin configuration will throw a compilation error
There was a problem hiding this comment.
@Stexxe overzealous testing?
osipxd
force-pushed
the
osipxd/jetty-jakarta-connections
branch
4 times, most recently
from
1f44763 to
cccd2f3
Compare
| routing { | ||
| get("/fail") { | ||
| async { throw AsyncFailedException() }.await() | ||
| call.async { throw AsyncFailedException() }.await() |
There was a problem hiding this comment.
It seems this test is actually broken.
Failing inside call.async terminates the whole call CoroutineScope and even if we catch this exception the scope itself is cancelled and calling call.respondText(...) from handler re-throws the exception
There was a problem hiding this comment.
I haven't managed to fix this problem quickly, so I created an issue: KTOR-8824
osipxd
force-pushed
the
osipxd/jetty-jakarta-connections
branch
from
cccd2f3 to
ad727dd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt (1)
7-11: Fix import for@ExperimentalKtorApi(compile break).
ExperimentalKtorApilives inio.ktor.util, notio.ktor.utils.io. Adjust imports to avoid unresolved reference.Apply this diff:
import io.ktor.htmx.* import io.ktor.server.routing.* -import io.ktor.utils.io.* +import io.ktor.util.* import kotlin.jvm.JvmInlineAlso applies to: 17-18
♻️ Duplicate comments (2)
ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt (1)
46-46: @KtorDsl on receivers — approvedCorrectly constrains the configuration DSL while preserving defaults and signatures.
Also applies to: 54-54
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (1)
257-261: Marking RoutingContext with @KtorDsl is the right call.Prevents accidental cross-receiver resolution in handlers and aligns with prior decision to require explicit
call.launch/call.async.
🧹 Nitpick comments (8)
ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt (1)
33-36: Verify DSL marker behavior forHxRoutereceiver.
HxRouteno longer carries@KtorDsl. Since thehx { ... }block now has anHxRoutereceiver, please confirm that@KtorDslrestrictions still apply viaRoute’s annotation in your test suite. If not, re-annotateHxRoutewith@KtorDsl.ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt (1)
24-27: Public DSL marker added to ServerConfigBuilder — OK, add a brief KDoc hint.Change looks correct and aligns with the PR goals. Consider adding a one-line note in the class KDoc that this is a Ktor DSL receiver to set expectations about scope limitations inside
serverConfig { ... }.ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt (1)
125-127: Use of currentCoroutineContext for CallId — OK; add a nullability guard if flakiness appears.Looks fine; if any platform test flakiness shows up, consider asserting the element presence before string building to surface clearer failures.
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt (1)
300-305: Minor naming nit: prefer 'job' over 'j'.Tiny readability tweak; behavior is fine since you cancel the job immediately.
- val j = application.launch { + val job = application.launch { delay(1000000L) } - j.cancel() + job.cancel()ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt (1)
255-256: Use coroutineContext instead of currentCoroutineContext().Slightly simpler and avoids the suspend call overhead.
- call.respond(currentCoroutineContext()[KtorCallIdContextElement]?.callId ?: "not found") + call.respond(coroutineContext[KtorCallIdContextElement]?.callId ?: "not found")Add if missing:
- import kotlin.coroutines.coroutineContext
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (1)
424-431: Avoid name shadowing with 'application' Job.The outer val application: Job can be confused with RoutingContext.application in handlers. Rename the Job for clarity.
- val application = launch { + val appJob = launch { runTestApplication(coroutineContext) { application { routing { get("/hello") { call.launch { delay(50) registryDeferred.complete(application.dependencies) } val service: GreetingService = application.dependencies.resolve() call.respondText(service.hello()) } } } } } ... - application.cancelAndJoin() + appJob.cancelAndJoin()ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (1)
262-267: NewRoutingContext.applicationaccessor is good; consider deprecating the Route-based accessor.To steer users toward the new context-centric API, deprecate the
Route.applicationextension with a migration hint.Example (in the file where
Route.applicationis defined):@Deprecated( message = "Use RoutingContext.application from inside handlers. " + "Outside handlers, prefer accessing Application via RoutingRoot or explicit references.", replaceWith = ReplaceWith("[email protected]") ) public fun io.ktor.server.routing.Route.getApplication(): Application = /* existing body */ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt (1)
300-306: Prefer avoiding newFixedThreadPoolContext in testsnewFixedThreadPoolContext is delicate/deprecated. You can simplify and drop the @OptIn by using a bounded IO dispatcher.
Apply this diff:
-import kotlinx.coroutines.DelicateCoroutinesApi -import kotlinx.coroutines.newFixedThreadPoolContext +import kotlinx.coroutines.Dispatchers ... -@OptIn(DelicateCoroutinesApi::class) @Test fun `can fill MDC and survive context switch in IOCoroutineDispatcher`() = testApplication { var counter = 0 - val dispatcher = newFixedThreadPoolContext(1, "test-dispatcher") + val dispatcher = Dispatchers.IO.limitedParallelism(1)And remove
dispatcher.close()at the end of this test since Dispatchers.IO doesn’t need closing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (29)
ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt(0 hunks)ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt(0 hunks)ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt(5 hunks)ktor-server/ktor-server-core/api/ktor-server-core.api(1 hunks)ktor-server/ktor-server-core/api/ktor-server-core.klib.api(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt(0 hunks)ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt(2 hunks)ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt(2 hunks)ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt(0 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt(1 hunks)ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt(3 hunks)ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt(2 hunks)ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt(2 hunks)ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt(2 hunks)
💤 Files with no reviewable changes (7)
- ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt
- ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt
- ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt
- ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
- ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt
- ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.ktktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.ktktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.ktktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.ktktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.ktktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.ktktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.ktktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.ktktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.ktktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.ktktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.ktktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.ktktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.ktktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.ktktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.ktktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.ktktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-server/ktor-server-core/api/ktor-server-core.apiktor-server/ktor-server-core/api/ktor-server-core.klib.api
🧠 Learnings (11)
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
PR: ktorio/ktor#4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.ktktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.ktktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
📚 Learning: 2025-05-16T13:11:28.416Z
Learnt from: osipxd
PR: ktorio/ktor#4860
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.api:24-26
Timestamp: 2025-05-16T13:11:28.416Z
Learning: Binary incompatible changes (such as constructor signature changes) are acceptable in the ktor-server-di module when the version is not yet released, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt
📚 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/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Follow Kotlin error-handling conventions and use specific Ktor exceptions
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt
📚 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/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.{kt,kts} : Use star imports for io.ktor.* packages
Applied to files:
ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.ktktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.ktktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.ktktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt
📚 Learning: 2025-06-09T07:19:09.152Z
Learnt from: bjhham
PR: ktorio/ktor#4916
File: ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModules.jvm.kt:41-41
Timestamp: 2025-06-09T07:19:09.152Z
Learning: In Ktor's concurrent module loading (`LoadConcurrently` in ApplicationModules.jvm.kt), `limitedParallelism(1)` is intentionally used to achieve concurrent but not parallel execution. This allows modules to suspend during loading (e.g., while waiting for dependencies) so other modules can start, while avoiding threading issues with shared state and non-thread-safe collections. The concurrency comes from coroutine scheduling, not multi-threading.
Applied to files:
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
📚 Learning: 2025-06-17T12:25:55.269Z
Learnt from: bishiboosh
PR: ktorio/ktor#4940
File: ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/FileCacheTest.kt:20-23
Timestamp: 2025-06-17T12:25:55.269Z
Learning: In kotlinx.io, the Path constructor accepts multiple string arguments: Path(String, String, ...String). When the first argument is a Path object, it gets implicitly converted to its string representation, making Path(pathObject, "child") a valid pattern. Unlike Okio, kotlinx.io does not provide a `/` (div) operator for path joining.
Applied to files:
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt
📚 Learning: 2025-06-17T12:25:55.269Z
Learnt from: bishiboosh
PR: ktorio/ktor#4940
File: ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/FileCacheTest.kt:20-23
Timestamp: 2025-06-17T12:25:55.269Z
Learning: In kotlinx.io, the `/` (div) operator for path joining does not exist - it only exists in kotlin.io.path which wraps Java NIO Path. The kotlinx.io.files.Path constructor accepts multiple string arguments and automatically converts Path objects to their string representation when passed as arguments, making Path(pathObject, "child") a valid construction pattern.
Applied to files:
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt
📚 Learning: 2025-06-17T12:25:55.269Z
Learnt from: bishiboosh
PR: ktorio/ktor#4940
File: ktor-client/ktor-client-tests/common/test/io/ktor/client/tests/FileCacheTest.kt:20-23
Timestamp: 2025-06-17T12:25:55.269Z
Learning: In kotlinx.io, the `/` (div) operator for path joining does not exist - it only exists in kotlin.io.path which wraps Java NIO Path. The kotlinx.io.files.Path constructor accepts multiple string arguments and automatically converts Path objects to their string representation when passed as arguments, making Path(pathObject, "child") a valid construction pattern that works across all platforms.
Applied to files:
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt
🧬 Code graph analysis (2)
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (1)
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyMapExtension.kt (2)
get(21-30)get(29-29)
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (1)
ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt (1)
module(74-76)
🔇 Additional comments (30)
ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt (1)
26-26: Good refactor:hx(configuration)now exposes HxRoute DSL inside the blockDelegating to
hx.apply(configuration)makestarget/triggeravailable in thehx { ... }scope and still returns the created child route. Nice simplification; source compatibility should remain OK thanks to contravariant receiver types.ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt (1)
12-12: Import change — OK.Star import for
io.ktor.utils.io.*is consistent with repo conventions.ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt (1)
12-12: Import change — OK.Star import for
io.ktor.utils.io.*matches conventions.ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt (2)
15-17: Import cleanup — OK.Switch to
currentCoroutineContextand explicitkotlin.testimports improves clarity.
41-41: Assert argument order fixed — OK.Using
(expected, actual)is correct and consistent.Also applies to: 67-67, 95-95, 121-121
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt (1)
9-11: IOBridge import — OK.Importing
io.ktor.utils.io.*to accessDispatchers.IOBridgeand switching to explicit coroutine imports is correct.ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt (1)
296-298: Switch to application.async avoids call-scope cancellation; verify supervision.Good workaround for KTOR-8824. Please confirm the Application coroutine scope is backed by a SupervisorJob so a failure in this async does not cancel the whole app.
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt (1)
18-22: Import updates look correct.Adds currentCoroutineContext and kotlin.test symbols needed by changes below.
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.kt (1)
85-95: Annotating EngineConnectorBuilder with @KtorDsl is appropriate.This aligns connector configuration with other DSL receivers.
ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt (1)
12-18: Imports aligned with style.Using star imports for io.ktor.* and narrowing coroutines test import to runTest matches project guidelines.
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (3)
7-16: Import adjustments are correct for common tests.Client, logging, test dispatcher, and Ktor’s multiplatform CancellationException are appropriately imported.
329-339: Switching to application.dependencies in handlers is the right scope.This follows the new RoutingContext.application access and avoids ambiguous resolution from local delegates.
384-394: Consistent use of application.dependencies — LGTM.Keeps resolution within the Application DI container in async path as well.
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.kt (1)
11-11: @JvmName import added — correct and necessaryBrings the annotation into scope for the typed-path helpers; io.ktor imports already use star style per guideline.
ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt (2)
12-12: Import aligns with DSL usageThe added io.ktor.utils.io.* import ensures @KtorDsl is resolvable; star-import policy for io.ktor.* is followed.
73-73: DSL-marker on configuration receivers — good, source- and binary-safeAnnotating the lambda receivers with @KtorDsl scopes configuration correctly without changing JVM signatures or defaults.
Also applies to: 140-141
ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt (2)
10-10: Import added for @KtorDsl — OKKeeps the annotation available; io.ktor star-import convention is respected.
46-46: Scoped configuration via @KtorDsl receivers — looks goodConforms to the PR’s DSL strategy; no runtime behavior change and maintains API docs.
Also applies to: 54-54
ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt (1)
10-10: Import for @KtorDsl — consistent with the rest of the PRMatches the approach taken in Netty/Jetty modules.
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (3)
206-208: Explicit outer receiver qualifiers — nice clarificationUsing this@TestApplicationBuilder avoids accidental shadowing and is safer in nested builders.
315-315: Move DSL marker to the configure parameter — goodScopes plugin configuration precisely without over-marking the builder API itself.
344-344: DSL marker on overrides receiver — consistentKeeps overrides confined to the intended DSL scope.
ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt (1)
158-160: No ambiguousclientreferences found in plugin config blocks. The only bareclientusage (val originalClient = client) occurs outside anyinstallconfig, so all plugin DSL blocks correctly use[email protected].ktor-server/ktor-server-core/api/ktor-server-core.api (1)
1747-1753: API addition for Java interop looks good.
RoutingNodeKt.getApplication(RoutingContext)cleanly exposes the KotlinRoutingContext.applicationto Java and is properly tracked in the API dump.ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt (1)
280-323: @KtorDsl on Route restores DSL safety for nested routing — LGTM.This should address nested builder leakage (KTOR-8667) without breaking intended builder usage.
ktor-server/ktor-server-core/api/ktor-server-core.klib.api (1)
1697-1698: (Public API) New RoutingContext.application: confirm deprecation/migration plan for Route.applicationGood addition. Please ensure the older Route.application extension is explicitly deprecated with a ReplaceWith pointing to RoutingContext.application and that migration notes are added. Also confirm this /api dump is aligned with the intended compatibility window.
Would you like a PR snippet to add @deprecated on the Route extension in RoutingNode.kt?
ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt (3)
21-33: Imports update looks correctThe added logging/MDC and dispatcher/test imports align with the test changes.
196-196: Switch to application.environment.log inside handler is correctMatches the new RoutingContext.application access pattern in route bodies.
221-223: Consistent logging via application.environment.logBoth logs participate in MDC assertions as intended.
ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt (1)
12-12: Remove stray @KtorDsl annotations and legacy Route.application getter
- In
ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt, remove the three@KtorDslmarkers onRoutingNode(line 29),RoutingContext(line 257), andRoute(line 280).- In
ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt, remove the legacypublic val Route.application: Applicationaccessor (line 167).⛔ Skipped due to learnings
Learnt from: CR PR: ktorio/ktor#0 File: CLAUDE.md:0-0 Timestamp: 2025-08-26T06:07:28.352Z Learning: Project is Kotlin Multiplatform (JVM, JS, Native)
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
Show resolved
Hide resolved
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.kt
Show resolved
Hide resolved
.../ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt
Show resolved
Hide resolved
ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt
Show resolved
Hide resolved
| * | ||
| * @see [testApplication] | ||
| */ | ||
| @KtorDsl |
There was a problem hiding this comment.
The annotation must have worked this way at some point... There's no way we'd have repeated the same mistake this many times, right? 😅
There was a problem hiding this comment.
I don't know, but with this reproducer I failed to find any Kotlin version where compilation would fail for the annotated function (scopedFunctinon2)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ktor-io/common/src/io/ktor/utils/io/Annotations.kt (2)
25-27: Duplicate target in InternalAPI's @target list.
AnnotationTarget.PROPERTY_SETTERappears twice. Replace the duplicate withPROPERTY_GETTERor remove it.- AnnotationTarget.PROPERTY_SETTER, - AnnotationTarget.PROPERTY_SETTER + AnnotationTarget.PROPERTY_SETTER, + AnnotationTarget.PROPERTY_GETTER
106-108: KDoc grammar nit: “annotations” → “annotation”.Minor fix for user-facing docs.
- * A marker annotations for DSLs. + * A marker annotation for DSLs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ktor-io/common/src/io/ktor/utils/io/Annotations.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-io/common/src/io/ktor/utils/io/Annotations.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-io/common/src/io/ktor/utils/io/Annotations.kt
🧠 Learnings (2)
📓 Common learnings
Learnt from: bjhham
PR: ktorio/ktor#4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Annotate internal APIs with InternalAPI
Applied to files:
ktor-io/common/src/io/ktor/utils/io/Annotations.kt
| @Target(AnnotationTarget.CLASS, AnnotationTarget.TYPEALIAS, AnnotationTarget.TYPE) | ||
| public annotation class KtorDsl |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Removal of FUNCTION from KtorDsl target — good; please verify no lingering usages and call out the breaking change.
This aligns with the PR intent to move DSL scoping to receiver types; it will surface compile errors for any remaining function-annotated sites.
Run this to ensure no functions are still annotated with @KtorDsl:
Also add a release note entry explicitly stating the target list change for @KtorDsl (FUNCTION dropped) and the migration path (annotate receiver types instead).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Same-line annotation on functions
rg -nP --type=kt --type=kts '@KtorDsl\s+.*\bfun\b' -g '!**/build/**' || true
# Multiline: annotation on a line above the function (handles stacked annotations/modifiers)
rg -nPU --type=kt --type=kts '(?s)@KtorDsl\s*(?:\n\s*@\w[^\n]*)*\s*(?:public|private|internal|protected)?\s*(?:suspend\s+)?fun\b' -g '!**/build/**' || trueLength of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for @KtorDsl on the same line as fun in all Kotlin files
rg -nP '@KtorDsl.*\bfun\b' -g '*.kt' -g '*.kts' -g '!**/build/**' || true
# Search for @KtorDsl annotation on a line above function declarations (handles stacked annotations)
rg -nPU '(?s)@KtorDsl\s*(?:\n\s*@\w[^\n]*)*\s*(?:public|private|internal|protected)?\s*(?:suspend\s+)?fun\b' -g '*.kt' -g '*.kts' -g '!**/build/**' || trueLength of output: 2615
Remove @KtorDsl annotations from functions in DI plugin and test-host modules
- DI plugin overloads (common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt lines 20, 36, 51, 67, 84, 102, 121)
- TestApplication APIs (common/src/io/ktor/server/testing/TestApplication.kt lines 58–59, 136–137, 163–164)
@KtorDsl no longer targets FUNCTION, so these sites will fail to compile. Remove the annotation from each function or migrate to annotating the receiver type instead. Don’t forget to add a release-note entry covering the dropped FUNCTION target and migration path.
🤖 Prompt for AI Agents
In ktor-io/common/src/io/ktor/utils/io/Annotations.kt around lines 111-112,
@KtorDsl no longer targets FUNCTION which breaks call sites: remove @KtorDsl
from all function declarations in the DI plugin overloads
(common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt lines
~20, 36, 51, 67, 84, 102, 121) and TestApplication APIs
(common/src/io/ktor/server/testing/TestApplication.kt lines ~58–59, 136–137,
163–164); alternatively, if DSL scoping is needed, move the annotation to the
receiver type (annotate the receiver class/type instead of the functions);
update or add a release-note entry describing the dropped FUNCTION target and
the migration path (remove annotations from functions or annotate receiver
types).
osipxd
force-pushed
the
osipxd/jetty-jakarta-connections
branch
from
965f0e7 to
231f712
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (1)
420-424: Rename misspelled property: testApplicationProvder → testApplicationProviderDeclaration and all call sites are misspelled; rename the property and usages consistently.
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/client/DelegatingTestClientEngine.kt
- Line 97: change
lateinit var testApplicationProvder: () -> TestApplication→lateinit var testApplicationProvider: () -> TestApplication- Line 25: change
config.testApplicationProvder().server.engine→config.testApplicationProvider().server.engine- Line 28: change
config.testApplicationProvder().externalApplications.forEach { … }→config.testApplicationProvider().externalApplications.forEach { … }- Line 48: change
config.testApplicationProvder().start()→config.testApplicationProvider().start()ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
- Line 421: change
testApplicationProvder = this@ApplicationTestBuilder::testApplication→testApplicationProvider = this@ApplicationTestBuilder::testApplication
♻️ Duplicate comments (1)
ktor-io/common/src/io/ktor/utils/io/Annotations.kt (1)
111-112: Dropped FUNCTION target for @KtorDsl — verify repo-wide usage and note breaking change.Good move; enables type-use like
(@KtorDsl X).() -> Unit. Please ensure no functions remain annotated with@KtorDsland add a release-note entry with the migration (annotate receiver types instead of functions).Run to confirm no lingering function annotations:
#!/bin/bash set -euo pipefail # Same-line annotation on functions rg -nP --type=kt --type=kts '@KtorDsl\s+.*\bfun\b' -g '!**/build/**' || true # Multiline (annotation above fun) rg -nPU --type=kt --type=kts '(?s)@KtorDsl\s*(?:\n\s*@\w[^\n]*)*\s*(?:public|private|internal|protected)?\s*(?:suspend\s+)?fun\b' -g '!**/build/**' || true
🧹 Nitpick comments (3)
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (2)
341-356: Don’t mark plain MutableMap receivers with @KtorDslApplying
@KtorDsltoMutableMap<String, String>.() -> Unitcan unintentionally restrict nested DSLs and isn’t semantically tied to a Ktor builder type. Prefer leaving it unannotated (or introduce a small DSL-specific builder type if you need scoping).- overrides: (@KtorDsl MutableMap<String, String>.() -> Unit)? = null + overrides: (MutableMap<String, String>.() -> Unit)? = null
323-326: Optional: annotate routing lambda receiver for consistency
Routeis annotated with@KtorDsl, so this is redundant functionally, but adding it here mirrors theinstallpattern and documents intent.- public fun routing(configuration: Route.() -> Unit) { + public fun routing(configuration: @KtorDsl Route.() -> Unit) {ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (1)
329-331: Nit: prefercall.applicationfor clarity.Even where no shadowing exists,
call.applicationis explicit and future‑proof against local variables namedapplication.Also applies to: 384-386
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt(0 hunks)ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt(0 hunks)ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt(5 hunks)ktor-io/common/src/io/ktor/utils/io/Annotations.kt(1 hunks)ktor-server/ktor-server-core/api/ktor-server-core.api(1 hunks)ktor-server/ktor-server-core/api/ktor-server-core.klib.api(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt(1 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt(2 hunks)ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt(0 hunks)ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt(2 hunks)ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt(2 hunks)ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt(7 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt(0 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt(1 hunks)ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt(1 hunks)ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt(3 hunks)ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt(2 hunks)ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt(2 hunks)ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt(2 hunks)
💤 Files with no reviewable changes (7)
- ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt
- ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt
- ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingRoot.kt
- ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt
- ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
- ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt
🚧 Files skipped from review as they are similar to previous changes (16)
- ktor-server/ktor-server-jetty/jvm/src/io/ktor/server/jetty/JettyApplicationEngineBase.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt
- ktor-server/ktor-server-core/api/ktor-server-core.api
- ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt
- ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingNode.kt
- ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt
- ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RegexRouting.kt
- ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt
- ktor-server/ktor-server-jetty-jakarta/jvm/src/io/ktor/server/jetty/jakarta/JettyApplicationEngineBase.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/engine/EngineConnectorConfig.kt
- ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt
- ktor-server/ktor-server-plugins/ktor-server-htmx/common/src/io/ktor/server/htmx/HxRouting.kt
- ktor-server/ktor-server-core/common/src/io/ktor/server/routing/RoutingBuilder.kt
- ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-io/common/src/io/ktor/utils/io/Annotations.ktktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-io/common/src/io/ktor/utils/io/Annotations.ktktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.ktktor-server/ktor-server-test-host/common/test/TestApplicationTest.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
**/api/**
📄 CodeRabbit inference engine (CLAUDE.md)
Track all public API changes in the /api/ directories
Files:
ktor-server/ktor-server-core/api/ktor-server-core.klib.api
🧠 Learnings (9)
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.{kt,kts} : Use star imports for io.ktor.* packages
Applied to files:
ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
📚 Learning: 2025-05-14T18:05:02.321Z
Learnt from: bjhham
PR: ktorio/ktor#4855
File: ktor-server/ktor-server-plugins/ktor-server-di/api/ktor-server-di.klib.api:334-336
Timestamp: 2025-05-14T18:05:02.321Z
Learning: Breaking changes in constructor parameter order are acceptable for the ktor-server-di module when the code hasn't been released yet, as confirmed by the development team.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.ktktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.ktktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 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/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Follow Kotlin error-handling conventions and use specific Ktor exceptions
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 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/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Annotate internal APIs with InternalAPI
Applied to files:
ktor-io/common/src/io/ktor/utils/io/Annotations.kt
📚 Learning: 2025-05-13T10:24:59.359Z
Learnt from: bjhham
PR: ktorio/ktor#4849
File: ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt:114-115
Timestamp: 2025-05-13T10:24:59.359Z
Learning: The `provide` function in DependencyRegistry intentionally accepts `DependencyResolver.() -> T?` (nullable return) while passing it to `set` which expects `DependencyResolver.() -> T` (non-nullable return). This is a deliberate design choice to support Java interoperability through Kotlin's flexible types system.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt
📚 Learning: 2025-06-09T07:19:09.152Z
Learnt from: bjhham
PR: ktorio/ktor#4916
File: ktor-server/ktor-server-core/jvm/src/io/ktor/server/application/ApplicationModules.jvm.kt:41-41
Timestamp: 2025-06-09T07:19:09.152Z
Learning: In Ktor's concurrent module loading (`LoadConcurrently` in ApplicationModules.jvm.kt), `limitedParallelism(1)` is intentionally used to achieve concurrent but not parallel execution. This allows modules to suspend during loading (e.g., while waiting for dependencies) so other modules can start, while avoiding threading issues with shared state and non-thread-safe collections. The concurrency comes from coroutine scheduling, not multi-threading.
Applied to files:
ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt
🧬 Code graph analysis (2)
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (2)
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyMapExtension.kt (2)
get(21-30)get(29-29)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyResolution.kt (1)
get(149-150)
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (1)
ktor-server/ktor-server-core/common/src/io/ktor/server/application/Application.kt (1)
module(74-76)
🔇 Additional comments (11)
ktor-server/ktor-server-core/api/ktor-server-core.klib.api (1)
1698-1699: Approve: add RoutingContext.application — verify API dumps & DSL annotationsGood parity and backwards‑compatible; mirrors Route.application so handlers can avoid call.application.
- Verify the accessor appears in all generated API dumps (.api / klib / JVM / JS): match "RoutingContext).(): io.ktor.server.application.Application" or "RoutingContext.application: io.ktor.server.application.Application".
- Confirm the source declares the extension exactly once:
val RoutingContext.application: io.ktor.server.application.Application.- Confirm
@KtorDslis applied to RoutingContext and Route (KTOR-8809).- Add KDoc for the property stating it forwards to
call.applicationand add a short note in the routing DSL docs.Automated sandbox verification failed (missing
tee); re-run these checks in your local/CI environment or re-run with an adjusted script.ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt (2)
12-12: Correct import for KtorDsl.
import io.ktor.utils.io.*is the right package; aligns with style to star-import io.ktor.*.
54-55: Receiver-type DSL marker on configureTomcat — approved.
Usages found: TomcatApplicationEngine calls configuration.configureTomcat(this) (tomcat & tomcat-jakarta); tests assign configuration.configureTomcat = { addAttributesFilter() }. No receiver-shadowing/hidden-receiver issues observed.ktor-server/ktor-server-core/common/src/io/ktor/server/engine/ApplicationEngine.kt (2)
9-11: Imports cleanup — conforms to guidelines.Star import for
io.ktor.*and explicit coroutines imports are fine;Dispatchers.IOBridgeremains available viaio.ktor.utils.io.*.
26-27: Mark ApplicationEngine.Configuration with @KtorDsl — approve; add release note and revalidate nested builders.Tightens DSL scope for engine configuration — add a release-note entry describing potential impact on nested builders and verify all callers that take ApplicationEngine.Configuration.() -> Unit (examples found: ktor-server/ktor-server-servlet/jvm/src/io/ktor/server/servlet/ServletApplicationEngine.kt, ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/Embedded.kt, ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestEngine.kt, ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/EngineConnectorConfigJvm.kt).
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (2)
203-205: Explicit receiver qualifiers prevent DSL scope leakage — LGTMUsing
this@TestApplicationBuilderforapplicationModulesandjobmakes the scope unambiguous and aligns with the PR intent.
312-316: Moving @KtorDsl to the configure receiver is correctAnnotating the lambda receiver (not the function) is the right pattern here.
ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt (1)
158-161: Qualified receiver is correct
[email protected]avoids capturing the plugin config receiver and matches the new DSL scoping.ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt (2)
329-331: Good: resolution via application-scoped container in routing.Switching to application.dependencies.* clarifies scope and avoids accidental capture of a test-local registry.
Also applies to: 337-339, 384-386, 392-394
7-16: Imports look consistent with style; verify CancellationException type.Star imports for io.ktor.* align with guidelines. Confirm the thrown exception is io.ktor.utils.io.CancellationException (and not kotlinx.coroutines.CancellationException) to avoid false positives in the final assertion.
Run to confirm which CancellationException is thrown in DI cancelation paths:
ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt (1)
18-22: Removing @KtorDsl from provide overloads — OK; verify base provide annotation & run API dump.Overloads in ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt no longer have @KtorDsl. Confirm the base function DependencyRegistry.provide(DependencyResolver.() -> E) remains annotated/accessible in ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt (matches at lines 28 and 159) and run the API/ABI dump/CI to ensure no surface changes.
| registryDeferred.complete(application.dependencies) | ||
| } | ||
| val service: GreetingService = dependencies.resolve() | ||
| val service: GreetingService = application.dependencies.resolve() | ||
| call.respondText(service.hello()) |
There was a problem hiding this comment.
Bug: name collision with local val application breaks resolution inside route.
Inside this test, val application = launch { ... } shadows the routing context’s application. application.dependencies here resolves to the Job, causing a compile error or wrong binding. Use call.application to disambiguate.
Apply:
- registryDeferred.complete(application.dependencies)
+ registryDeferred.complete(call.application.dependencies)
@@
- val service: GreetingService = application.dependencies.resolve()
+ val service: GreetingService = call.application.dependencies.resolve()Optionally, rename the outer variable to appJob to prevent future collisions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| registryDeferred.complete(application.dependencies) | |
| } | |
| val service: GreetingService = dependencies.resolve() | |
| val service: GreetingService = application.dependencies.resolve() | |
| call.respondText(service.hello()) | |
| registryDeferred.complete(call.application.dependencies) | |
| } | |
| val service: GreetingService = call.application.dependencies.resolve() | |
| call.respondText(service.hello()) |
🤖 Prompt for AI Agents
In
ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
around lines 428 to 431, the local variable `val application = launch { ... }`
shadows the routing `application`, so `application.dependencies` points to the
Job instead of the call's application; change the reference to
`call.application.dependencies` to disambiguate the routing context and resolve
the correct dependencies, and optionally rename the outer `application` launch
variable to `appJob` (or similar) to avoid future collisions.
231f712 to
8e43e42
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt (1)
300-305: Use cancelAndJoin and a clearer name to avoid stray coroutinesCancel and join to ensure the launched job doesn’t linger; also prefer a descriptive variable name.
- val j = application.launch { + val job = application.launch { delay(1000000L) } - j.cancel() + job.cancelAndJoin() call.respondText("OK")Add missing import:
import kotlinx.coroutines.cancelAndJoinktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt (3)
21-33: Imports: align with repo style and keep only what’s usedAll added imports are used; good. To match the stated guideline “Use star imports for io.ktor.* packages,” consider consolidating io.ktor.* imports where applicable (e.g., routing/testing blocks) to reduce churn across test files.
196-196: Prefer application.log over application.environment.log for consistencyFunctionally equivalent, but application.log is the idiomatic entry point used elsewhere in this file.
Apply this diff:
- application.environment.log.info("test message") + application.log.info("test message")
221-223: Use application.log consistentlyMatch the rest of the tests for readability.
Apply this diff:
- application.environment.log.info("test1") - application.environment.log.info("test2") + application.log.info("test1") + application.log.info("test2")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt(0 hunks)ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt(0 hunks)ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt(0 hunks)ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt(5 hunks)ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcClient.kt(0 hunks)ktor-io/common/src/io/ktor/utils/io/Annotations.kt(1 hunks)ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt(3 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt(7 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt(0 hunks)ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt(4 hunks)ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt(1 hunks)ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt(3 hunks)ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt(1 hunks)ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt(2 hunks)ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt(2 hunks)ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt(2 hunks)
💤 Files with no reviewable changes (7)
- ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyRegistry.kt
- ktor-client/ktor-client-core/jsAndWasmShared/src/io/ktor/client/HttpClientJs.kt
- ktor-client/ktor-client-webrtc/common/src/io/ktor/client/webrtc/WebRtcClient.kt
- ktor-client/ktor-client-core/posix/src/io/ktor/client/HttpClient.kt
- ktor-client/ktor-client-plugins/ktor-client-auth/common/src/io/ktor/client/plugins/auth/providers/BasicAuthProvider.kt
- ktor-client/ktor-client-core/common/src/io/ktor/client/HttpClient.kt
- ktor-client/ktor-client-core/jvm/src/io/ktor/client/HttpClientJvm.kt
🚧 Files skipped from review as they are similar to previous changes (10)
- ktor-server/ktor-server-test-host/common/test/TestApplicationTest.kt
- ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CORSTest.kt
- ktor-server/ktor-server-netty/jvm/src/io/ktor/server/netty/NettyApplicationEngine.kt
- ktor-server/ktor-server-tomcat/jvm/src/io/ktor/server/tomcat/TomcatApplicationEngine.kt
- ktor-io/common/src/io/ktor/utils/io/Annotations.kt
- ktor-server/ktor-server-tests/common/test/io/ktor/tests/server/plugins/CallIdTest.kt
- ktor-client/ktor-client-plugins/ktor-client-call-id/common/test/io/ktor/client/plugins/callid/CallIdTest.kt
- ktor-server/ktor-server-plugins/ktor-server-di/common/src/io/ktor/server/plugins/di/DependencyProviderLambdaOverloads.kt
- ktor-server/ktor-server-plugins/ktor-server-di/common/test/io/ktor/server/plugins/di/DependencyInjectionTest.kt
- ktor-server/ktor-server-tomcat-jakarta/jvm/src/io/ktor/server/tomcat/jakarta/TomcatApplicationEngine.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{kt,kts}: Follow Kotlin official style guide for all Kotlin source and build scripts
Use star imports for io.ktor.* packages
Max line length is 120 characters
Indent with 4 spaces in Kotlin code
Include a copyright header in new Kotlin files
Files:
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.kt: Document all public Kotlin APIs, including parameters, return types, and exceptions
Annotate internal APIs with @internalapi
Follow Kotlin error-handling conventions and use specific Ktor exceptions
Files:
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.ktktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt
🧠 Learnings (2)
📚 Learning: 2025-06-23T12:49:56.883Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:49:56.883Z
Learning: Error handling should follow Kotlin conventions and use specific Ktor exceptions.
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.ktktor-server/ktor-server-plugins/ktor-server-call-logging/jvm/test/io/ktor/server/plugins/calllogging/CallLoggingTest.kt
📚 Learning: 2025-08-26T06:07:28.352Z
Learnt from: CR
PR: ktorio/ktor#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-26T06:07:28.352Z
Learning: Applies to **/*.kt : Follow Kotlin error-handling conventions and use specific Ktor exceptions
Applied to files:
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt
🔇 Additional comments (4)
ktor-server/ktor-server-plugins/ktor-server-status-pages/common/test/io/ktor/server/plugins/statuspages/StatusPagesTest.kt (1)
296-298: Application scope workaround for KTOR-8824 — OKSwitching from call.async to application.async avoids call-scope cancellation and makes the test deterministic. Please confirm this is intended across all targets; if not, fall back to call.application for clarity.
ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt (3)
203-205: Good: explicit DSL scope qualificationUsing this@TestApplicationBuilder prevents accidental resolution into the serverConfig DSL and fixes the prior scope leakage.
341-342: Receiver‑scoped overrides block looks goodMarking the MutableMap receiver with @KtorDsl prevents outer test DSL from leaking into overrides.
312-313: Move @KtorDsl to the configure receiver — tighten DSL scopingAnnotate the builder receiver (B) with @KtorDsl instead of annotating the enclosing function; verify other plugin/config Builder/Config/Configuration classes/interfaces also declare @KtorDsl to prevent nested DSL leakage.
Location: ktor-server/ktor-server-test-host/common/src/io/ktor/server/testing/TestApplication.kt:312–313
3 participants
Subsystem
Client, Server
Motivation
Solution
@KtorDslto theRouteandRoutingContextto fix routing DSL@KtorDslto theApplicationEngine.ConfigurationandEngineConnectorBuilder@KtorDslto ensure they're correctThese changes are breaking so we should discuss if this PR should be merged at once, or we should extract only some of the changes. It's better to review commit-by-commit.