From 45d01af6c559dd1a243d09227f8b22876e17918b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Sat, 20 May 2023 22:43:56 +0200 Subject: [PATCH 1/3] feat(spine): Declare errors with type safety --- spine/src/commonMain/kotlin/Operation.kt | 14 +++--- spine/src/commonMain/kotlin/Resource.kt | 52 ++++++++++----------- spine/src/commonMain/kotlin/SpineFailure.kt | 37 +++++++++++++-- spine/src/commonTest/kotlin/ServiceTest.kt | 22 ++++----- 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/spine/src/commonMain/kotlin/Operation.kt b/spine/src/commonMain/kotlin/Operation.kt index 02a1162..ac3bd77 100644 --- a/spine/src/commonMain/kotlin/Operation.kt +++ b/spine/src/commonMain/kotlin/Operation.kt @@ -4,13 +4,13 @@ import arrow.core.raise.Raise import opensavvy.state.arrow.out import kotlin.js.JsName -typealias OperationValidator = suspend Operation.ValidatorScope.() -> Unit +typealias OperationValidator = suspend Operation.ValidatorScope.() -> Unit -class Operation( +class Operation( val resource: ResourceGroup.AbstractResource, val kind: Kind, val route: Route? = null, - @JsName("_validate") private val validate: OperationValidator, + @JsName("_validate") private val validate: OperationValidator, ) { /** @@ -20,7 +20,7 @@ class Operation, Unit> { val scope = ValidatorScope(this, id, body, parameters, context) scope.validate() } @@ -67,8 +67,8 @@ class Operation internal constructor( - private val scope: Raise, + class ValidatorScope internal constructor( + private val scope: Raise>, val id: Id, @@ -77,5 +77,5 @@ class Operation by scope + ) : Raise> by scope } diff --git a/spine/src/commonMain/kotlin/Resource.kt b/spine/src/commonMain/kotlin/Resource.kt index 9251014..c9cd21e 100644 --- a/spine/src/commonMain/kotlin/Resource.kt +++ b/spine/src/commonMain/kotlin/Resource.kt @@ -18,15 +18,15 @@ sealed class ResourceGroup { * * If no static resources were registered, this collection is empty. */ - val staticRoutes: Map> get() = _staticRoutes - private val _staticRoutes: HashMap> = HashMap() + val staticRoutes: Map> get() = _staticRoutes + private val _staticRoutes: HashMap> = HashMap() /** * The [dynamic resource][DynamicResource] that appears as a direct child of this resource group. * * If no dynamic resource were registered, this property is `null`. */ - var dynamicRoute: DynamicResource<*, *>? = null + var dynamicRoute: DynamicResource<*, *, *>? = null private set /** @@ -74,13 +74,13 @@ sealed class ResourceGroup { */ abstract val parent: ResourceGroup - protected val _operations = ArrayList>() - val operations: List> get() = _operations + protected val _operations = ArrayList>() + val operations: List> get() = _operations /** * Validates that [id] identifies this resource. */ - fun Raise.validateCorrectId(id: Id) { + fun Raise>.validateCorrectId(id: Id) { ensure( id.service == service.name ) { @@ -106,7 +106,7 @@ sealed class ResourceGroup { @Suppress("NAME_SHADOWING") // necessary for smart cast because 'resource' is mutable when (val resource: AbstractResource<*, *> = resource) { - is StaticResource<*, *, *> -> { + is StaticResource<*, *, *, *> -> { ensure( segment == resource.route ) { @@ -117,7 +117,7 @@ sealed class ResourceGroup { } } - is DynamicResource<*, *> -> { + is DynamicResource<*, *, *> -> { // There are no constraints on what IDs look like. // If we expect an ID, we can't make any verification on the value. } @@ -145,36 +145,36 @@ sealed class ResourceGroup { * For example, you can override this function to check access rights for read operations. * By default, this function does nothing. */ - open suspend fun Raise.validateId(id: Id, context: Context) {} + open suspend fun Raise>.validateId(id: Id, context: Context) {} - protected fun create( + protected fun create( route: Route? = null, - validate: OperationValidator = { }, - ) = Operation, Params, Context>(this, Operation.Kind.Create, route, validate) + validate: OperationValidator = { }, + ) = Operation, Params, Context>(this, Operation.Kind.Create, route, validate) .apply { _operations += this } - protected fun edit( + protected fun edit( route: Route? = null, - validate: OperationValidator = { }, - ) = Operation(this, Operation.Kind.Edit, route) { + validate: OperationValidator = { }, + ) = Operation(this, Operation.Kind.Edit, route) { validateCorrectId(id) validateId(id, context) Operation.ValidatorScope(this, id, body, parameters, context).validate() }.apply { _operations += this } - protected fun action( + protected fun action( route: Route, - validate: OperationValidator = { }, - ) = Operation(this, Operation.Kind.Action, route) { + validate: OperationValidator = { }, + ) = Operation(this, Operation.Kind.Action, route) { validateCorrectId(id) validateId(id, context) Operation.ValidatorScope(this, id, body, parameters, context).validate() }.apply { _operations += this } - protected fun delete( + protected fun delete( route: Route? = null, - validate: OperationValidator = { }, - ) = Operation(this, Operation.Kind.Delete, route) { + validate: OperationValidator = { }, + ) = Operation(this, Operation.Kind.Delete, route) { validateCorrectId(id) validateId(id, context) Operation.ValidatorScope(this, id, body, parameters, context).validate() @@ -212,7 +212,7 @@ sealed class ResourceGroup { * For example, top-level resources tend to be static: `/users`. * Static resources may also appear as children of other resources: `/users/{id}/emails`. */ - abstract inner class StaticResource protected constructor(route: String) : + abstract inner class StaticResource protected constructor(route: String) : AbstractResource() { val route = Route.Segment(route) @@ -229,11 +229,11 @@ sealed class ResourceGroup { * * You should override this function if the parameters impact the access rights. */ - open suspend fun Raise.validateGetParams(id: Id, params: GetParams, context: Context) {} + open suspend fun Raise.validateGetParams(id: Id, params: GetParams, context: Context) {} @Suppress("LeakingThis") // Not dangerous because Operation's constructor does nothing val get = - Operation(this, Operation.Kind.Read) { + Operation(this, Operation.Kind.Read) { validateCorrectId(id) validateId(id, context) validateGetParams(id, parameters, context) @@ -257,7 +257,7 @@ sealed class ResourceGroup { /** * A template for resources identified by IDs. */ - abstract inner class DynamicResource protected constructor( + abstract inner class DynamicResource protected constructor( /** * The name of the identifier. * @@ -274,7 +274,7 @@ sealed class ResourceGroup { } @Suppress("LeakingThis") // Not dangerous because Operation's constructor does nothing - val get = Operation(this, Operation.Kind.Read) { + val get = Operation(this, Operation.Kind.Read) { validateCorrectId(id) validateId(id, context) }.apply { _operations += this } diff --git a/spine/src/commonMain/kotlin/SpineFailure.kt b/spine/src/commonMain/kotlin/SpineFailure.kt index 6967dbf..341ff93 100644 --- a/spine/src/commonMain/kotlin/SpineFailure.kt +++ b/spine/src/commonMain/kotlin/SpineFailure.kt @@ -1,11 +1,28 @@ package opensavvy.spine -data class SpineFailure( - val type: Type, - val message: String, -) { +sealed class SpineFailure { - override fun toString() = "$type: “$message”" + abstract val type: Type + + abstract val payload: Payload? + + data class Message( + override val type: Type, + val message: String? = null, + ) : SpineFailure() { + override val payload: Nothing? + get() = null + + override fun toString() = "$type: “${message}”" + } + + data class Payload( + override val type: Type, + override val payload: Payload?, + ) : SpineFailure() { + + override fun toString() = "$type: “${payload}”" + } enum class Type { Unauthenticated, @@ -16,3 +33,13 @@ data class SpineFailure( ; } } + +fun SpineFailure( + type: SpineFailure.Type, + message: String? = null, +) = SpineFailure.Message(type, message) + +fun SpineFailure( + type: SpineFailure.Type, + payload: Payload?, +) = SpineFailure.Payload(type, payload) diff --git a/spine/src/commonTest/kotlin/ServiceTest.kt b/spine/src/commonTest/kotlin/ServiceTest.kt index e36e18b..2ab84c2 100644 --- a/spine/src/commonTest/kotlin/ServiceTest.kt +++ b/spine/src/commonTest/kotlin/ServiceTest.kt @@ -35,12 +35,12 @@ private data class User(val name: String, val admin: Boolean) { data class Rename(val name: String) } -private class Context(val user: Ref) +private class Context(val user: Ref, User>) private class Api : Service("v2") { - inner class Departments : StaticResource, Department.SearchParams, Context>("departments") { - inner class Unique : DynamicResource("department") { - inner class Users : StaticResource, Parameters.Empty, Context>("users") + inner class Departments : StaticResource, Nothing, Department.SearchParams, Context>("departments") { + inner class Unique : DynamicResource("department") { + inner class Users : StaticResource, Nothing, Parameters.Empty, Context>("users") val users = Users() } @@ -48,20 +48,20 @@ private class Api : Service("v2") { val id = Unique() } - inner class Users : StaticResource, Parameters.Empty, Context>("users") { - inner class Unique : DynamicResource("user") { - inner class Departments : StaticResource, Parameters.Empty, Context>("departments") + inner class Users : StaticResource, Nothing, Parameters.Empty, Context>("users") { + inner class Unique : DynamicResource("user") { + inner class Departments : StaticResource, Nothing, Parameters.Empty, Context>("departments") - val join = action(Route / "join") + val join = action(Route / "join") - val leave = action(Route / "leave") + val leave = action(Route / "leave") - val rename = edit(Route / "name") + val rename = edit(Route / "name") val departments = Departments() } - val create = create() + val create = create() val id = Unique() } -- 2.51.2 From 6ad27355d0e8e7526807796ca3cbd199bc15931c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Sat, 20 May 2023 22:44:23 +0200 Subject: [PATCH 2/3] feat(spine-ktor): Declare errors with type safety --- .../src/commonMain/kotlin/Client.kt | 78 ++++++++++++++++- .../kotlin/EndpointsAdvertisement.kt | 2 +- .../src/commonMain/kotlin/GenerateId.kt | 4 +- .../commonMain/kotlin/ResponseStateBuilder.kt | 6 +- .../src/commonMain/kotlin/Server.kt | 85 +++++++++++++++++-- .../src/commonTest/kotlin/TestApi.kt | 16 ++-- 6 files changed, 171 insertions(+), 20 deletions(-) diff --git a/spine-ktor/spine-ktor-client/src/commonMain/kotlin/Client.kt b/spine-ktor/spine-ktor-client/src/commonMain/kotlin/Client.kt index c783683..27eaba9 100644 --- a/spine-ktor/spine-ktor-client/src/commonMain/kotlin/Client.kt +++ b/spine-ktor/spine-ktor-client/src/commonMain/kotlin/Client.kt @@ -14,6 +14,8 @@ import opensavvy.spine.SpineFailure import opensavvy.spine.ktor.toHttp import opensavvy.spine.ktor.toSpine import opensavvy.state.arrow.out +import kotlin.js.JsName +import kotlin.jvm.JvmName /** * Executes a [HttpClient] request, with the information declared in an [Operation]. @@ -40,8 +42,78 @@ import opensavvy.state.arrow.out * @param configuration Additional configuration passed to Ktor's `request` function. * This configuration is applied after the parameters from this request are applied, it is possible to override data set by this function. */ +suspend inline fun HttpClient.request( + operation: Operation, + id: Id, + input: In, + parameters: Params, + context: Context, + contentType: ContentType = ContentType.Application.Json, + crossinline onResponse: (HttpResponse) -> Unit = {}, + crossinline configuration: HttpRequestBuilder.() -> Unit = {}, +) = out { + mapProgressTo(0.0..0.1) { + operation.validate(id, input, parameters, context).bind() + } + + val result = mapProgressTo(0.1..0.9) { + request { + method = operation.kind.toHttp() + + url { + // {baseUrl}/{service}/{path-to-resource}/{path-to-method} + + // /{service} + appendPathSegments(id.service.segment) + + // /{path-to-resource} + appendPathSegments(id.resource.segments.map { it.segment }) + + // /{path-to-method} (if present) + operation.route?.let { route -> + appendPathSegments(route.segments.map { it.segment }) + } + } + + for ((name, value) in parameters.data) + parameter(name, value) + + contentType(contentType) + setBody(input) + + configuration() + } + } + + mapProgressTo(0.9..0.95) { + onResponse(result) + } + + mapProgressTo(0.95..1.0) { + if (result.status.isSuccess()) { + result.body() + } else { + val kind = result.status.toSpine() + + val failure = try { + SpineFailure(kind, result.body()) + } catch (e: NoTransformationFoundException) { + SpineFailure(kind, result.body().ifBlank { "${result.status} with no provided body" }) + } + + raise(failure) + } + } +} + +// Yes, this is a copy-paste of the function above. +// For some reason, the compiler does not allow 'Nothing' as a reified type parameter, so I have to create an overload +// without that parameter. And because the entire function has to be inline, it has to be a copy. +// Spine is deprecated anyway, and will be completely rewritten when I have the time. +@JvmName("requestNoFailure") +@JsName("requestNoFailure") suspend inline fun HttpClient.request( - operation: Operation, + operation: Operation, id: Id, input: In, parameters: Params, @@ -91,9 +163,9 @@ suspend inline fun () } else { - val body = result.body().ifBlank { "${result.status} with no provided body" } val kind = result.status.toSpine() - raise(SpineFailure(kind, body)) + + raise(SpineFailure(kind, result.body().ifBlank { "${result.status} with no provided body" })) } } } diff --git a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/EndpointsAdvertisement.kt b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/EndpointsAdvertisement.kt index a0a3913..2c6c599 100644 --- a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/EndpointsAdvertisement.kt +++ b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/EndpointsAdvertisement.kt @@ -8,7 +8,7 @@ import opensavvy.spine.Route import opensavvy.spine.Route.Companion.div import opensavvy.spine.ktor.toHttp -fun ApplicationCall.advertiseEndpointsFor(operation: Operation<*, *, *, *, *>, id: Id) { +fun ApplicationCall.advertiseEndpointsFor(operation: Operation<*, *, *, *, *, *>, id: Id) { val resource = operation.resource val link = resource.operations diff --git a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/GenerateId.kt b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/GenerateId.kt index 9a2c734..a0d75af 100644 --- a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/GenerateId.kt +++ b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/GenerateId.kt @@ -11,11 +11,11 @@ fun ApplicationCall.generateId(resource: ResourceGroup.AbstractResourc var cursor: ResourceGroup = resource while (cursor is ResourceGroup.AbstractResource<*, *>) { values += when (cursor) { - is ResourceGroup.StaticResource<*, *, *> -> { + is ResourceGroup.StaticResource<*, *, *, *> -> { cursor.route.segment } - is ResourceGroup.DynamicResource<*, *> -> { + is ResourceGroup.DynamicResource<*, *, *> -> { val name = cursor.name val value = parameters[name] ?: error("Missing path parameter: '{$name}' in '${resource.routeTemplate}'") diff --git a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/ResponseStateBuilder.kt b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/ResponseStateBuilder.kt index 97920fa..b30a0ae 100644 --- a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/ResponseStateBuilder.kt +++ b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/ResponseStateBuilder.kt @@ -9,8 +9,8 @@ import opensavvy.spine.SpineFailure /** * Information available in [route]. */ -class ResponseStateBuilder( - builder: Raise, +class ResponseStateBuilder( + builder: Raise>, /** * The identifier of the resource being requested. @@ -36,4 +36,4 @@ class ResponseStateBuilder( * The current request's context. */ val context: Context, -) : Raise by builder +) : Raise> by builder diff --git a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/Server.kt b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/Server.kt index cc11f0d..a4a6233 100644 --- a/spine-ktor/spine-ktor-server/src/commonMain/kotlin/Server.kt +++ b/spine-ktor/spine-ktor-server/src/commonMain/kotlin/Server.kt @@ -9,6 +9,7 @@ import opensavvy.logger.Logger.Companion.warn import opensavvy.logger.loggerFor import opensavvy.spine.Operation import opensavvy.spine.Parameters +import opensavvy.spine.SpineFailure import opensavvy.spine.ktor.toHttp import opensavvy.state.arrow.toEither import kotlin.collections.component1 @@ -48,10 +49,81 @@ object Server { * * This function automatically calls the [operation]'s [validation][Operation.validate] code. */ +inline fun Route.route( + operation: Operation, + contextGenerator: ContextGenerator, + crossinline block: suspend ResponseStateBuilder.() -> Out, +) { + val path = buildString { + append(operation.resource.routeTemplate) + + for (segment in operation.route?.segments ?: emptyList()) { + append('/') + append(segment.segment) + } + } + + val method = operation.kind.toHttp() + route(path, method) { + handle { + val context = contextGenerator.generate(call) + + val id = call.generateId(operation.resource) + + val params: Params = when { + Params::class == Parameters.Empty::class -> Parameters.Empty as Params + else -> { + val params = Params::class.java + .getConstructor() + .newInstance() + for ((name, values) in call.parameters.entries()) + // if a parameter is added multiple times, only the first one is kept + params.data[name] = values.first() + params + } + } + + val body = when { + // If the expected input is Unit, don't even try to read the body + // Ktor fails to read the body on GET, DELETE and OPTIONS requests. Because we encode them as Unit, + // it's not a problem. + In::class == Unit::class -> Unit as In + // For any other type, delegate to the ContentNegotiation plugin + else -> call.receive() + } + + call.advertiseEndpointsFor(operation, id) + + either, Out> { + operation.validate(id, body, params, context).toEither().bind() + + val responseBuilder = ResponseStateBuilder(this, id, body, params, call, context) + responseBuilder.block() + }.fold( + ifLeft = { + Server.log.warn(it.type) { it.toString() } + when (it) { + is SpineFailure.Message -> call.respond(it.type.toHttp(), it.message ?: "No message") + is SpineFailure.Payload -> call.respond(it.type.toHttp(), it.payload ?: "No message") + } + }, + ifRight = { + call.respond(it) + } + ) + } + } +} + +// Yes, this is a copy-paste of the function above. +// For some reason, the compiler does not allow 'Nothing' as a reified type parameter, so I have to create an overload +// without that parameter. And because the entire function has to be inline, it has to be a copy. +// Spine is deprecated anyway, and will be completely rewritten when I have the time. +@JvmName("routeNoFailure") inline fun Route.route( - operation: Operation, + operation: Operation, contextGenerator: ContextGenerator, - crossinline block: suspend ResponseStateBuilder.() -> Out, + crossinline block: suspend ResponseStateBuilder.() -> Out, ) { val path = buildString { append(operation.resource.routeTemplate) @@ -93,15 +165,18 @@ inline fun , Out> { operation.validate(id, body, params, context).toEither().bind() val responseBuilder = ResponseStateBuilder(this, id, body, params, call, context) responseBuilder.block() }.fold( ifLeft = { - Server.log.warn(it.type, it.message) { it.toString() } - call.respond(it.type.toHttp(), it.message) + Server.log.warn(it.type) { it.toString() } + when (it) { + is SpineFailure.Message -> call.respond(it.type.toHttp(), it.message ?: "No message") + is SpineFailure.Payload -> call.respond(it.type.toHttp(), it.payload ?: "No message") + } }, ifRight = { call.respond(it) diff --git a/spine-ktor/spine-ktor-server/src/commonTest/kotlin/TestApi.kt b/spine-ktor/spine-ktor-server/src/commonTest/kotlin/TestApi.kt index 7955ec9..ffcae33 100644 --- a/spine-ktor/spine-ktor-server/src/commonTest/kotlin/TestApi.kt +++ b/spine-ktor/spine-ktor-server/src/commonTest/kotlin/TestApi.kt @@ -33,6 +33,10 @@ data class User(val id: Id, val name: String, val archived: Boolean) { class SearchParams : Parameters() { var includeArchived: Boolean by parameter("includeArchived") } + + sealed interface Failures { + data class InvalidUsername(val username: String) : Failures + } } //endregion @@ -40,17 +44,17 @@ data class User(val id: Id, val name: String, val archived: Boolean) { class TestApi : Service("test") { - inner class Users : StaticResource, User.SearchParams, Unit>("users") { - inner class Unique : DynamicResource("user") { + inner class Users : StaticResource, Nothing, User.SearchParams, Unit>("users") { + inner class Unique : DynamicResource("user") { - val archive = action(Route / "archive") + val archive = action(Route / "archive") - val unarchive = action(Route / "reopen") + val unarchive = action(Route / "reopen") - val delete = delete() + val delete = delete() } - val create = create() + val create = create() val id = Unique() } -- 2.51.2 From cbdc452b5adb0e657760fcf6278d0cb83b8127fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Sat, 20 May 2023 22:44:54 +0200 Subject: [PATCH 3/3] build(idea): Run tests as tests --- .idea/runConfigurations/All_tests.xml | 4 ++-- .idea/runConfigurations/All_tests__Browser_.xml | 2 +- .idea/runConfigurations/All_tests__JVM_.xml | 2 +- .idea/runConfigurations/All_tests__NodeJS_.xml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.idea/runConfigurations/All_tests.xml b/.idea/runConfigurations/All_tests.xml index af8c191..630d3c8 100644 --- a/.idea/runConfigurations/All_tests.xml +++ b/.idea/runConfigurations/All_tests.xml @@ -10,7 +10,7 @@ @@ -19,7 +19,7 @@ true true false - false + true \ No newline at end of file diff --git a/.idea/runConfigurations/All_tests__Browser_.xml b/.idea/runConfigurations/All_tests__Browser_.xml index c77ebb3..025962a 100644 --- a/.idea/runConfigurations/All_tests__Browser_.xml +++ b/.idea/runConfigurations/All_tests__Browser_.xml @@ -18,7 +18,7 @@ true true false - false + true \ No newline at end of file diff --git a/.idea/runConfigurations/All_tests__JVM_.xml b/.idea/runConfigurations/All_tests__JVM_.xml index 5f1e44f..a4583bd 100644 --- a/.idea/runConfigurations/All_tests__JVM_.xml +++ b/.idea/runConfigurations/All_tests__JVM_.xml @@ -19,7 +19,7 @@ true true false - false + true diff --git a/.idea/runConfigurations/All_tests__NodeJS_.xml b/.idea/runConfigurations/All_tests__NodeJS_.xml index ac1a547..bae4523 100644 --- a/.idea/runConfigurations/All_tests__NodeJS_.xml +++ b/.idea/runConfigurations/All_tests__NodeJS_.xml @@ -18,7 +18,7 @@ true true false - false + true \ No newline at end of file -- 2.51.2