diff --git a/exposed/src/main/kotlin/repositories/AbstractRepository.kt b/exposed/src/main/kotlin/repositories/AbstractRepository.kt index 3cfab76..b67fa18 100644 --- a/exposed/src/main/kotlin/repositories/AbstractRepository.kt +++ b/exposed/src/main/kotlin/repositories/AbstractRepository.kt @@ -42,6 +42,14 @@ abstract class AbstractRepository>(protected val ent protected fun ENTITY.applyFlush(): ENTITY = this.apply { flush() } + /** + * Finds every entity of the table. + * + * @return a lazily evaluated Exposed entity sequence containing every entity. + */ + fun findAll(): SizedIterable = + entityClass.all() + /** * Finds every entity whose identifier belongs to [ids]. * diff --git a/graphql/AGENTS.md b/graphql/AGENTS.md index 41791d6..02bd3da 100644 --- a/graphql/AGENTS.md +++ b/graphql/AGENTS.md @@ -116,7 +116,14 @@ not a failed execution, and a caller branching on the status code must tell the Never surface an internal exception message to a client. -## 6. Out of Scope for This Module +## 6. Identifiers — `UUID` scalar + +graphql-kotlin has no built-in scalar for `kotlin.uuid.Uuid`. `uuidScalar` (with `UuidCoercing`) +exposes it as a `UUID` scalar and `UuidSchemaGeneratorHooks` registers it for every `Uuid` the schema +meets, so a DTO shared with REST carries its identifiers as `Uuid` and never as a string duplicate. +The scalar speaks the canonical hyphenated hexadecimal form in both directions. + +## 7. Out of Scope for This Module - Websocket subscriptions: the artifact ships `getSubscriptionServer()`, but the route does not advertise subscriptions, and `schemaSubscriptions` should stay empty unless that changes. diff --git a/graphql/src/main/kotlin/GraphQLModules.kt b/graphql/src/main/kotlin/GraphQLModules.kt index 034b1f9..4748e0a 100644 --- a/graphql/src/main/kotlin/GraphQLModules.kt +++ b/graphql/src/main/kotlin/GraphQLModules.kt @@ -109,6 +109,7 @@ fun Application.configureGraphQL( queries = schemaQueries mutations = schemaMutations subscriptions = schemaSubscriptions + hooks = UuidSchemaGeneratorHooks() } engine { diff --git a/graphql/src/main/kotlin/UuidScalar.kt b/graphql/src/main/kotlin/UuidScalar.kt new file mode 100644 index 0000000..a74e731 --- /dev/null +++ b/graphql/src/main/kotlin/UuidScalar.kt @@ -0,0 +1,121 @@ +package fr.shikkanime.graphql + +import com.expediagroup.graphql.generator.hooks.FlowSubscriptionSchemaGeneratorHooks +import com.expediagroup.graphql.generator.hooks.SchemaGeneratorHooks +import graphql.GraphQLContext +import graphql.execution.CoercedVariables +import graphql.language.StringValue +import graphql.language.Value +import graphql.schema.Coercing +import graphql.schema.CoercingParseLiteralException +import graphql.schema.CoercingParseValueException +import graphql.schema.CoercingSerializeException +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLType +import java.util.Locale +import kotlin.reflect.KType +import kotlin.uuid.Uuid + +/** + * GraphQL scalar carrying a [Uuid] across the schema. + * + * graphql-kotlin maps only a handful of Kotlin types to built-in scalars, so a shared DTO holding + * a [Uuid] identifier would otherwise fail schema generation and force a duplicated DTO typed on + * strings. The scalar speaks the canonical hyphenated hexadecimal form in both directions. + */ +val uuidScalar: GraphQLScalarType = + GraphQLScalarType.newScalar() + .name("UUID") + .description("kotlin.uuid.Uuid rendered as its hyphenated hexadecimal string") + .coercing(UuidCoercing()) + .build() + +/** + * Reads and writes [Uuid] values for [uuidScalar]. + * + * Every direction funnels through `Uuid.parseOrNull` and `Uuid.toString`, so the schema, the + * variables and the serialized response all agree on the same textual form. + */ +class UuidCoercing : Coercing { + /** + * Renders a resolver result as its textual identifier. + * + * @param dataFetcherResult value produced by a resolver, expected to be a [Uuid]. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the hyphenated hexadecimal form of [dataFetcherResult]. + * @throws CoercingSerializeException when the result is not a [Uuid]. + */ + override fun serialize( + dataFetcherResult: Any, + graphQLContext: GraphQLContext, + locale: Locale + ): String = + (dataFetcherResult as? Uuid)?.toString() + ?: throw CoercingSerializeException("Cannot serialize $dataFetcherResult as a UUID") + + /** + * Parses a variable value into a [Uuid]. + * + * @param input raw variable value, expected to be a textual identifier. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the parsed identifier. + * @throws CoercingParseValueException when [input] is not a valid identifier. + */ + override fun parseValue(input: Any, graphQLContext: GraphQLContext, locale: Locale): Uuid = + (input as? String)?.let(Uuid::parseOrNull) + ?: throw CoercingParseValueException("Cannot parse $input as a UUID") + + /** + * Parses an inline literal into a [Uuid]. + * + * @param input literal node from the query document. + * @param variables coerced variables of the running operation. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the parsed identifier. + * @throws CoercingParseLiteralException when [input] is not a valid identifier. + */ + override fun parseLiteral( + input: Value<*>, + variables: CoercedVariables, + graphQLContext: GraphQLContext, + locale: Locale + ): Uuid = + (input as? StringValue)?.value?.let(Uuid::parseOrNull) + ?: throw CoercingParseLiteralException("Cannot parse $input as a UUID") + + /** + * Renders a [Uuid] back into a literal node. + * + * @param input identifier to render. + * @param graphQLContext context of the running operation. + * @param locale locale of the running operation. + * @return the identifier as a string literal node. + */ + override fun valueToLiteral(input: Any, graphQLContext: GraphQLContext, locale: Locale): Value<*> = + StringValue.of((input as Uuid).toString()) +} + +/** + * Schema hooks exposing [uuidScalar] for every [Uuid] the schema meets. + * + * Every other hook keeps the framework default ([FlowSubscriptionSchemaGeneratorHooks]): the + * scalar is a widening of the generator, not a replacement of its behaviour. + */ +class UuidSchemaGeneratorHooks( + private val delegate: SchemaGeneratorHooks = FlowSubscriptionSchemaGeneratorHooks() +) : SchemaGeneratorHooks by delegate { + /** + * Resolves the GraphQL type of a Kotlin type. + * + * @param type Kotlin type the generator is building a schema type for. + * @return [uuidScalar] when [type] is a [Uuid], the delegate answer otherwise. + */ + override fun willGenerateGraphQLType(type: KType): GraphQLType? = + if (type.classifier == Uuid::class) + uuidScalar + else + delegate.willGenerateGraphQLType(type) +} diff --git a/guidelines/API_CONVENTIONS.md b/guidelines/API_CONVENTIONS.md index 2bdc43f..d12281b 100644 --- a/guidelines/API_CONVENTIONS.md +++ b/guidelines/API_CONVENTIONS.md @@ -24,6 +24,11 @@ class UserController { - Use `@PathParam("name")` for route path segments. - Use `@RequestBody` for JSON payload deserialization. - Add `@Valid` on parameters to automatically validate the deserialized object via `Validator.validate(...)`. +- Type a parameter with its real domain type, never with `String` to re-parse it: + `@PathParam("id") id: Uuid`. +- A non-nullable parameter whose value does not match its type is refused with an HTTP 400 carrying + a `MessageDto` error (`InvalidParameterException`). A nullable parameter is optional by nature: an + absent or unconvertible value resolves to `null` and never refuses the request. ## 3. Standardized Response Payloads diff --git a/guidelines/DATABASE.md b/guidelines/DATABASE.md index 8391141..a4167aa 100644 --- a/guidelines/DATABASE.md +++ b/guidelines/DATABASE.md @@ -22,7 +22,7 @@ Operations modifying state or executing Exposed queries must run within a transa ## 3. AbstractRepository & Upsert DSL All Exposed repositories should inherit from `AbstractRepository`. -It provides standard `findById`, `findAllByIds`, and a fluent upsert DSL: +It provides standard `findAll`, `findById`, `findAllByIds`, and a fluent upsert DSL: ```kotlin override fun persist(id: Int?, data: String): MyEntity { diff --git a/guidelines/KOTLIN_CONVENTIONS.md b/guidelines/KOTLIN_CONVENTIONS.md index a2d4860..67ada13 100644 --- a/guidelines/KOTLIN_CONVENTIONS.md +++ b/guidelines/KOTLIN_CONVENTIONS.md @@ -5,6 +5,10 @@ - **Prefer immutability**: use `val`, read-only collections (`List`, `Map`), and `data class`. - **Add KDoc comments** to all public framework classes, interfaces, annotations, and functions to document behavior and rules. - **Leverage Kotlin features safely**: use null safety features (`?.`, `?:`, `let`), inline functions where appropriate, and reflection extension functions cleanly. +- **Name a property in camelCase, a type in PascalCase**: a top-level value reads `uuidScalar`, never + `UuidScalar`, which would name a type. +- **Refuse first, then assign**: a guard `if (invalid) throw ...` at the top of the block followed by + the plain assignment, never the refusal nested inside a conditional expression. ```kotlin fun getLogger(clazz: Class<*>): Logger = diff --git a/ktor/src/main/kotlin/ControllerArgumentResolver.kt b/ktor/src/main/kotlin/ControllerArgumentResolver.kt index 65ae474..9d06202 100644 --- a/ktor/src/main/kotlin/ControllerArgumentResolver.kt +++ b/ktor/src/main/kotlin/ControllerArgumentResolver.kt @@ -14,6 +14,32 @@ import kotlin.uuid.Uuid * Resolves controller function arguments from a Ktor application call. */ internal object ControllerArgumentResolver { + /** + * Converts a textual parameter to the type of [kParameter], refusing a required parameter whose + * value does not match its type. + * + * A nullable parameter is optional by nature: an unconvertible value resolves to `null`, exactly + * like an absent value, so an optional parameter never refuses the request. A non-nullable + * parameter is required: an unconvertible value is a client error. + * + * @param kParameter controller function parameter being resolved. + * @param name HTTP name of the parameter, used in the refusal message. + * @param stringValue raw parameter value. + * @return the converted value, or `null` for a nullable parameter with no convertible value. + * @throws InvalidParameterException when a non-nullable parameter value fails its conversion. + */ + private fun parseParameter(kParameter: KParameter, name: String, stringValue: String): Any? { + val kClass = kParameter.type.classifier as? KClass<*> + val parsed = parseValue(kClass, stringValue) + + if (!kParameter.type.isMarkedNullable && parsed == null) + throw InvalidParameterException( + "Parameter '$name' must be a valid ${kClass?.simpleName ?: "value"}" + ) + + return parsed + } + /** * Converts a textual HTTP parameter to the type expected by a controller function. * @@ -69,8 +95,11 @@ internal object ControllerArgumentResolver { val stringValue = call.request.queryParameters[queryParamAnnotation.name] if (stringValue != null) { - val kClass = kParameter.type.classifier as? KClass<*> - args[kParameter] = parseValue(kClass, stringValue) + args[kParameter] = parseParameter( + kParameter, + queryParamAnnotation.name, + stringValue + ) } else if (!queryParamAnnotation.required && kParameter.type.isMarkedNullable) { args[kParameter] = null } @@ -81,8 +110,11 @@ internal object ControllerArgumentResolver { val stringValue = call.parameters[pathParamAnnotation.name] if (stringValue != null) { - val kClass = kParameter.type.classifier as? KClass<*> - args[kParameter] = parseValue(kClass, stringValue) + args[kParameter] = parseParameter( + kParameter, + pathParamAnnotation.name, + stringValue + ) } } diff --git a/ktor/src/main/kotlin/ControllerRequestHandler.kt b/ktor/src/main/kotlin/ControllerRequestHandler.kt index 9e72e46..6a52a68 100644 --- a/ktor/src/main/kotlin/ControllerRequestHandler.kt +++ b/ktor/src/main/kotlin/ControllerRequestHandler.kt @@ -65,6 +65,9 @@ internal object ControllerRequestHandler { is ObjectNotValidException -> call.respond(HttpStatusCode.BadRequest, MessageDto.error(cause.message)) + is InvalidParameterException -> + call.respond(HttpStatusCode.BadRequest, MessageDto.error(cause.message)) + else -> call.respond(HttpStatusCode.InternalServerError, MessageDto.error("An unexpected error occurred")) } diff --git a/ktor/src/main/kotlin/InvalidParameterException.kt b/ktor/src/main/kotlin/InvalidParameterException.kt new file mode 100644 index 0000000..f78fe43 --- /dev/null +++ b/ktor/src/main/kotlin/InvalidParameterException.kt @@ -0,0 +1,11 @@ +package fr.shikkanime.ktor + +/** + * Indicates that a request parameter could not be converted to the type the controller expects. + * + * A malformed path, query or body parameter is a client error, not a server failure: the request + * handler translates it to an HTTP 400 carrying a [fr.shikkanime.ktor.dtos.MessageDto] error. + * + * @property message human-readable description naming the parameter and the expected type. + */ +class InvalidParameterException(override val message: String) : IllegalArgumentException(message) diff --git a/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt b/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt index 784348a..9d7c54d 100644 --- a/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt +++ b/ktor/src/test/kotlin/ControllerArgumentResolverTest.kt @@ -30,6 +30,11 @@ class ControllerArgumentResolverTest { @QueryParam("optional", required = false) optional: String? ): ResponseEntity = ResponseEntity.ok("ok") + + fun optionalEndpoint( + @QueryParam("id", required = false) id: Uuid? + ): ResponseEntity = + ResponseEntity.ok(id?.toString() ?: "none") } @Nested @@ -84,7 +89,7 @@ class ControllerArgumentResolverTest { } @Test - suspend fun `should return null when numeric or boolean conversions fail`() { + suspend fun `should refuse the request when a numeric or boolean conversion fails`() { // Given val call = mockk(relaxed = true) every { call.request.queryParameters["longVal"] } returns "invalid_number" @@ -93,6 +98,24 @@ class ControllerArgumentResolverTest { val controller = SampleController() val function = SampleController::typeEndpoint + // When + val exception = org.junit.jupiter.api.assertThrows { + ControllerArgumentResolver.resolve(call, function, controller) + } + + // Then + assertTrue(exception.message.contains("must be a valid")) + } + + @Test + suspend fun `should resolve an optional parameter to null when its value does not match its type`() { + // Given + val call = mockk(relaxed = true) + every { call.request.queryParameters["id"] } returns "not-a-uuid" + + val controller = SampleController() + val function = SampleController::optionalEndpoint + // When val args = ControllerArgumentResolver.resolve(call, function, controller)