diff --git a/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpoints.kt b/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpoints.kt index 361351393..e68f29387 100644 --- a/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpoints.kt +++ b/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpoints.kt @@ -28,6 +28,19 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable public val detailPath: PathSpec1 = path.arg(Segment.Wildcard("id", info.idSerializer)) private val bulkPath = path.path("bulk") + // Errors actually thrown by the implementations below; declared so docs/SDKs advertise them and the + // W6 "undeclared error" advisory stays quiet. + private val notFoundError = LSError( + http = HttpStatus.NotFound.code, + detail = "", + message = "There was no known object by that ID.", + ) + private val uniqueViolationError = LSError( + http = HttpStatus.BadRequest.code, + detail = "unique", + message = "A unique constraint was violated.", + ) + public val permissions: ApiHttpHandler> = path.path("_permissions_").get bind explicitApiHttpHandler( summary = "Permissions", @@ -101,14 +114,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = Unit.serializer(), outputType = info.serializer, auth = info.auth.subscope(ModelInfo.Scopes.read), - errorCases = listOf( - LSError( - http = HttpStatus.NotFound.code, - detail = "", - message = "There was no known object by that ID.", - data = "" - ) - ), + errorCases = listOf(notFoundError), examples = emptyList(), implementation = { _: Unit -> info.table(this).get(route.arg1) ?: throw NotFoundException() @@ -123,7 +129,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = ListSerializer(info.serializer), outputType = ListSerializer(info.serializer), auth = info.auth.subscope(ModelInfo.Scopes.create), - errorCases = emptyList(), + errorCases = listOf(uniqueViolationError), examples = emptyList(), implementation = { values: List -> try { @@ -147,7 +153,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = info.serializer, outputType = info.serializer, auth = info.auth.subscope(ModelInfo.Scopes.create), - errorCases = emptyList(), + errorCases = listOf(uniqueViolationError), examples = emptyList(), implementation = { value: T -> try { @@ -171,7 +177,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = info.serializer, outputType = info.serializer, auth = info.auth.subscope(listOf(ModelInfo.Scopes.create, ModelInfo.Scopes.update)), - errorCases = emptyList(), + errorCases = listOf(notFoundError, uniqueViolationError), examples = emptyList(), implementation = { value: T -> try { @@ -197,7 +203,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = ListSerializer(info.serializer), outputType = ListSerializer(info.serializer), auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = emptyList(), + errorCases = listOf(uniqueViolationError), examples = emptyList(), implementation = { values: List -> try { @@ -221,7 +227,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = info.serializer, outputType = info.serializer, auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = emptyList(), + errorCases = listOf(notFoundError, uniqueViolationError), examples = emptyList(), implementation = { value: T -> try { @@ -247,7 +253,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = MassModification.serializer(info.serializer), outputType = Int.serializer(), auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = emptyList(), + errorCases = listOf(uniqueViolationError), examples = emptyList(), implementation = { input: MassModification -> try { @@ -271,14 +277,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = Modification.serializer(info.serializer), outputType = EntryChange.serializer(info.serializer), auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = listOf( - LSError( - http = HttpStatus.NotFound.code, - detail = "", - message = "There was no known object by that ID.", - data = "" - ) - ), + errorCases = listOf(notFoundError, uniqueViolationError), examples = emptyList(), implementation = { input: Modification -> try { @@ -303,14 +302,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = Modification.serializer(info.serializer), outputType = info.serializer, auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = listOf( - LSError( - http = HttpStatus.NotFound.code, - detail = "", - message = "There was no known object by that ID.", - data = "" - ) - ), + errorCases = listOf(notFoundError, uniqueViolationError), examples = emptyList(), implementation = { input: Modification -> try { @@ -336,14 +328,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = PartialSerializer(info.serializer), outputType = info.serializer, auth = info.auth.subscope(ModelInfo.Scopes.update), - errorCases = listOf( - LSError( - http = HttpStatus.NotFound.code, - detail = "", - message = "There was no known object by that ID.", - data = "" - ) - ), + errorCases = listOf(notFoundError, uniqueViolationError), examples = emptyList(), implementation = { input: Partial -> try { @@ -384,14 +369,7 @@ public class ModelRestEndpoints?, T : HasId, ID : Comparable inputType = Unit.serializer(), outputType = Unit.serializer(), auth = info.auth.subscope(ModelInfo.Scopes.delete), - errorCases = listOf( - LSError( - http = HttpStatus.NotFound.code, - detail = "", - message = "There was no known object by that ID.", - data = "" - ) - ), + errorCases = listOf(notFoundError), examples = emptyList(), implementation = { _: Unit -> if (!info.table(this).deleteOneById(route.arg1)) { diff --git a/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApi.kt b/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApi.kt index b9837d9ea..e3d70a772 100644 --- a/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApi.kt +++ b/typed/src/main/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApi.kt @@ -1,6 +1,7 @@ package com.lightningkite.lightningserver.typed.jsonschema import com.lightningkite.lightningserver.HttpMethod +import com.lightningkite.lightningserver.LSError import com.lightningkite.lightningserver.definition.generalSettings import com.lightningkite.lightningserver.pathing.PathSpec import com.lightningkite.lightningserver.pathing.plus @@ -218,9 +219,31 @@ private fun ?, INPUT, OUTPUT> ApiHttpHandler + OpenApiResponse( + description = cases.joinToString("\n") { case -> + val prefix = if (case.detail.isNotBlank()) "[${case.detail}] " else "" + prefix + case.message.ifBlank { "Error" } + }, + content = mapOf( + MediaType.Application.Json.toString() to OpenApiMediaType( + schema = builder[LSError.serializer()], + example = runtime.externalSerialization.json.encodeToJsonElement( + LSError.serializer(), + cases.first() + ) + ) + ) + ) + } + + mapOf(successCode.code.toString() to response) + errorResponses } - // TODO: Error codes ) context(runtime: ServerRuntime) diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpointsTest.kt b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpointsTest.kt index 4cea1d490..633fb1da5 100644 --- a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpointsTest.kt +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/ModelRestEndpointsTest.kt @@ -552,6 +552,37 @@ class ModelRestEndpointsTest { } } + // The endpoints below catch UniqueViolationException and rethrow BadRequestException(detail="unique"), + // or throw NotFoundException; declaring those cases keeps the W6 "undeclared error" advisory quiet and + // documents the errors. See ModelRestEndpoints implementations. + private fun declaresUnique(handler: ApiHttpHandler<*, *, *, *>) = + handler.errorCases.any { it.http == 400 && it.detail == "unique" } + + private fun declaresNotFound(handler: ApiHttpHandler<*, *, *, *>) = + handler.errorCases.any { it.http == 404 } + + @Test + fun endpoints_declare_the_errors_they_throw() { + val rest = CrudTestServer.rest + + // Every endpoint that catches UniqueViolationException declares the 400 "unique" case. + listOf(rest.insert, rest.insertBulk, rest.upsert, rest.bulkReplace, rest.replace, + rest.bulkModify, rest.modifyWithDiff, rest.modify, rest.modifySimple).forEach { + assertTrue(declaresUnique(it), "expected 400:unique in errorCases") + } + + // Every endpoint that can throw NotFoundException declares the 404 case. + listOf(rest.detail, rest.upsert, rest.replace, rest.modifyWithDiff, rest.modify, + rest.modifySimple, rest.deleteItem).forEach { + assertTrue(declaresNotFound(it), "expected 404 in errorCases") + } + + // Read-only endpoints that never throw keep an empty error list. + listOf(rest.list, rest.query, rest.count, rest.permissions).forEach { + assertTrue(it.errorCases.isEmpty(), "read-only endpoint should declare no errors") + } + } + @Test fun bulkDelete_with_Never_condition_deletes_nothing() = runBlocking { CrudTestServer.test(settings = { diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApiTest.kt b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApiTest.kt new file mode 100644 index 000000000..fbb287e41 --- /dev/null +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/jsonschema/OpenApiTest.kt @@ -0,0 +1,83 @@ +package com.lightningkite.lightningserver.typed.jsonschema + +import com.lightningkite.lightningserver.LSError +import com.lightningkite.lightningserver.auth.noAuth +import com.lightningkite.lightningserver.definition.builder.ServerBuilder +import com.lightningkite.lightningserver.http.* +import com.lightningkite.lightningserver.runtime.test.test +import com.lightningkite.lightningserver.serialization.registerBasicMediaTypeCoders +import com.lightningkite.lightningserver.typed.ApiHttpHandler +import com.lightningkite.services.data.MediaType +import kotlinx.coroutines.runBlocking +import kotlin.test.* + +/** + * Verifies that [openApiDescription] documents declared error cases as responses and emits path + * arguments as `in: path` parameters. + */ +class OpenApiTest { + + object TestServer : ServerBuilder() { + init { registerBasicMediaTypeCoders() } + + // Endpoint with a path argument and two declared error cases, one of them sharing a status code. + val getItem = path.path("items").arg("id").get bind ApiHttpHandler( + summary = "Get Item", + auth = noAuth, + errorCases = listOf( + LSError(http = 404, detail = "not-found", message = "No such item"), + LSError(http = 400, detail = "bad-id", message = "Malformed id"), + LSError(http = 400, detail = "blocked", message = "Access blocked"), + ), + implementation = { _: Unit -> "value" } + ) + + // Zero-argument endpoint to prove nothing breaks without path arguments. + val root = path.get bind ApiHttpHandler( + summary = "Root", + auth = noAuth, + implementation = { _: Unit -> "ok" } + ) + } + + @Test + fun errorCasesAppearAsResponses() = runBlocking { + TestServer.test({}) { + val op = openApiDescription.paths.entries.first { it.key.contains("items") }.value.get + assertNotNull(op) + + // Success response is still present. + assertTrue(op.responses.containsKey("200"), "success response should remain") + + // Declared error statuses are documented with the LSError schema and an example. + val notFound = op.responses["404"] + assertNotNull(notFound, "404 error case should be documented") + val notFoundMedia = notFound.content[MediaType.Application.Json.toString()] + assertNotNull(notFoundMedia) + assertNotNull(notFoundMedia.schema.ref, "error response should reference the LSError schema") + + // The two 400 cases are grouped into a single response with a combined description. + val badRequest = op.responses["400"] + assertNotNull(badRequest, "400 error cases should be documented") + assertTrue(badRequest.description.contains("bad-id")) + assertTrue(badRequest.description.contains("blocked")) + } + } + + @Test + fun pathArgumentsAppearAsParameters() = runBlocking { + TestServer.test({}) { + val paths = openApiDescription.paths + + val itemsPath = paths.entries.first { it.key.contains("items") }.value + val idParam = itemsPath.parameters.singleOrNull { it.name == "id" } + assertNotNull(idParam, "path argument should be emitted as a parameter") + assertEquals(OpenApiParameterType.path, idParam.inside) + assertTrue(idParam.required, "path parameters must be required") + + // Zero-argument endpoints emit no path parameters. + val rootPath = paths.entries.first { it.key == "/" || it.key.isEmpty() }.value + assertTrue(rootPath.parameters.none { it.inside == OpenApiParameterType.path }) + } + } +} diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/Api.ts b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/Api.ts index ebaf46e09..9f6c0118c 100644 --- a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/Api.ts +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/Api.ts @@ -1,5 +1,5 @@ import type { Query, MassModification, EntryChange, ListChange, Modification, Condition, GroupCountQuery, AggregateQuery, GroupAggregateQuery, Aggregate, SortPart, DataClassPath, DataClassPathPartial, QueryPartial, DeepPartial, Fetcher } from '@lightningkite/lightning-server-simplified' -import type { CollectionUpdates, Mask, Mode, ModelPermissions, Pair, Part, TestInput, TestModel, UpdateRestrictions, Uuid } from './models.ts' +import type { CollectionUpdates, Mask, ModelPermissions, Pair, TestInput, TestModel, UpdateRestrictions, UpdateRestrictionsMode, UpdateRestrictionsPart, Uuid } from './models.ts' export interface Api { index(): Promise diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/LiveApi.ts b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/LiveApi.ts index b96106ea6..30f0a2715 100644 --- a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/LiveApi.ts +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/LiveApi.ts @@ -1,5 +1,5 @@ import type { Query, MassModification, EntryChange, ListChange, Modification, Condition, GroupCountQuery, AggregateQuery, GroupAggregateQuery, Aggregate, SortPart, DataClassPath, DataClassPathPartial, QueryPartial, DeepPartial, Fetcher } from '@lightningkite/lightning-server-simplified' -import type { CollectionUpdates, Mask, Mode, ModelPermissions, Pair, Part, TestInput, TestModel, UpdateRestrictions, Uuid } from './models.ts' +import type { CollectionUpdates, Mask, ModelPermissions, Pair, TestInput, TestModel, UpdateRestrictions, UpdateRestrictionsMode, UpdateRestrictionsPart, Uuid } from './models.ts' import type { Api } from './Api.ts' export class LiveApi implements Api { diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/models.ts b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/models.ts index 5dc3749e2..358db0280 100644 --- a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/models.ts +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/models.ts @@ -11,11 +11,6 @@ export interface Mask { pairs: Array, Modification>> } -export enum Mode { - Blacklist = "Blacklist", - Whitelist = "Whitelist", -} - export interface ModelPermissions { create: Condition read: Condition @@ -31,12 +26,6 @@ export interface Pair { second: T1 } -export interface Part { - property: DataClassPathPartial - requires: Condition - limitedTo: Condition -} - export interface TestInput { id: number name: string @@ -48,8 +37,19 @@ export interface TestModel { } export interface UpdateRestrictions { - mode: Mode - fields: Array> + mode: UpdateRestrictionsMode + fields: Array> +} + +export enum UpdateRestrictionsMode { + Blacklist = "Blacklist", + Whitelist = "Whitelist", +} + +export interface UpdateRestrictionsPart { + property: DataClassPathPartial + requires: Condition + limitedTo: Condition } export type Uuid = string // kotlin.uuid.Uuid diff --git a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/sdk.ts b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/sdk.ts index 93c3e2ab4..2ea1d21a8 100644 --- a/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/sdk.ts +++ b/typed/src/test/kotlin/com/lightningkite/lightningserver/typed/sdk/generated/typescript/sdk.ts @@ -11,11 +11,6 @@ export interface Mask { pairs: Array, Modification>> } -export enum Mode { - Blacklist = "Blacklist", - Whitelist = "Whitelist", -} - export interface ModelPermissions { create: Condition read: Condition @@ -31,12 +26,6 @@ export interface Pair { second: T1 } -export interface Part { - property: DataClassPathPartial - requires: Condition - limitedTo: Condition -} - export interface TestInput { id: number name: string @@ -48,8 +37,19 @@ export interface TestModel { } export interface UpdateRestrictions { - mode: Mode - fields: Array> + mode: UpdateRestrictionsMode + fields: Array> +} + +export enum UpdateRestrictionsMode { + Blacklist = "Blacklist", + Whitelist = "Whitelist", +} + +export interface UpdateRestrictionsPart { + property: DataClassPathPartial + requires: Condition + limitedTo: Condition } export type Uuid = string // kotlin.uuid.Uuid