ktor typesafe routing #3832
ktor typesafe routing #3832
Conversation
mrf7
force-pushed
the
mrf/ktor-typesafe-routing
branch
from
55090a5 to
f6bd738
Compare
mrf7
force-pushed
the
mrf/ktor-typesafe-routing
branch
from
f6bd738 to
cb72f82
Compare
tKe
left a comment
tKe
left a comment
There was a problem hiding this comment.
We may want to consider using a separate module or variant to avoiding forcing a dependency on the resources plugin for consumers that aren't using it.
Can you also ensure the correct imports are used for the resources patch/post/put as currently they'll do the wrong thing and attempt to .recieve<TRoute>(). May be worth adding a test case for each to ensure the right things is being done.
| import io.ktor.server.routing.patch | ||
| import io.ktor.server.routing.post | ||
| import io.ktor.server.routing.put |
There was a problem hiding this comment.
call.receive<TRoute>() instead of using the resources plugin.
| @@ -0,0 +1,79 @@ | |||
| @file:Suppress("API_NOT_AVAILABLE") | |||
|
|
|||
| package arrow.raise.ktor.server.routing.typesafe | |||
There was a problem hiding this comment.
Can we use the .resources package naming to be consistent with the ktor package?
I'm not sure if it's best to fully align with
package arrow.raise.ktor.server.resourcesor keep to a subpackage of
package arrow.raise.ktor.server.routing.resources| import io.ktor.server.routing.put | ||
| import io.ktor.utils.io.KtorDsl | ||
|
|
||
| public typealias TypeSafeRespondingRaiseRoutingHandler<TRoute, TResponse> = suspend context(Raise<Response>) RoutingContext.(TRoute) -> TResponse |
There was a problem hiding this comment.
Can we be consistent with this naming? We seem to have Resourced, TypeSafe, and Typed (for the tests). I suggest sticking with Resources to align with the Resources plugin.
| public fun interface ResourcedReceivingRespondingRaiseRoutingHandler<TRoute, TBody, TResponse> { | ||
| context(_: Raise<Response>) | ||
| public suspend fun RoutingContext.handle(route: TRoute, body: TBody): TResponse | ||
| } |
There was a problem hiding this comment.
This shouldn't need to be a fun interface and could just be a typealias for the appropriate lambda (making the usage below simpler).
The reason for the fun interface before was to rely on SAM usage to deprioritise it against the zero-arity equivalents without received parameters (as otherwise the compiler finds it ambiguous if between a no-arg lambda or a one-arg lambda with implicit it parameter).
In this case, both candidates should be distinct by needing either one (explicit or implicit) or two explicit parameters on the lambda.
| commonMain { | ||
| dependencies { | ||
| api(libs.ktor.server.core) | ||
| implementation(libs.ktor.server.resources) |
There was a problem hiding this comment.
I'd rather we avoid bringing in a dependency on the ktor-server-resources plugin for all consumers of this library as it's unnecessary unless explicitly wanting to use this feature.
That said, I'm not sure if gradle feature variants are easily achievable with kotlin multiplatform libraries.
An alternative might be to add another arrow module with the resources support included, that depends on this one. Any thoughts @nomisRev / @serras?
| @@ -0,0 +1,131 @@ | |||
| package arrow.raise.ktor.server.response | |||
There was a problem hiding this comment.
Can we relocate the tests to the same package?
2 participants
An attempt at adding support for typesafe routing support to the ktor server raise handlers.
Temporarily named all the functionsNevermind i tried the rename and it seems fine*OrErrorinstead of*OrRaisebecause the conflicting overloads on params and type arguments were funky. Maybe there should be a module for the type safe routing variants and a module for the regular variants? Would be a little restrictive to only use one or the other without weird override conflicts