Repository navigation
feat: UUID scalar, typed parameter refusal and repository findAll - #54
Merged
Merged
Conversation
graphql-kotlin has no built-in scalar for kotlin.uuid.Uuid: a DTO carrying a Uuid identifier fails schema generation and forces consumers into string-typed duplicates. UuidScalar speaks the canonical hyphenated hexadecimal form in both directions, and UuidSchemaGeneratorHooks registers it for every Uuid the schema meets while keeping the framework default hooks.
A malformed path or query parameter used to resolve to null and surface as a 500 through the call. The resolver now refuses the request with InvalidParameterException, and the request handler translates it to an HTTP 400 carrying a MessageDto error.
A whole-table read now comes from the repository layer: consumers call super.findAll() instead of reaching for the Exposed entity class directly.
A nullable parameter whose value does not match its type resolved through the conversion failure path and refused the request, making an optional parameter behave like a required one. It now falls back to null, exactly like an absent value; only a non-nullable parameter answers 400.
…sion A top-level value is camelCase: uuidScalar, not UuidScalar. The parameter conversion now guards its refusal first (a non-nullable parameter with an unconvertible value throws immediately) and assigns afterwards, instead of nesting the reason inside a conditional expression.
The query and path branches each carried their own conversion and refusal block; both now call parseParameter, which refuses first and returns the converted value. Guidelines updated with the new rules: typed controller parameters with the 400 contract, optional parameters never refusing the request, camelCase properties, and the UUID scalar documented in the graphql module.
Ziedelth
added a commit
that referenced
this pull request
Oct 16, 2026
* build: declare the graphql module
Adds a graphql module depending on core, ktor and graphql-kotlin 10.2.2, plus
the four Ktor artifacts the security stack needs (rate limit, body limit,
compression, double receive). exposed stays out: the framework has no
repository, so DataLoaders belong to the consumer.
* feat: bound the graphql cost model and expose its configuration
Adds the typed configuration, the static cost estimator and the security
bounds. Three of the tests found real holes in the first draft:
- repeated fragment spreads were priced once, so a fragment bomb cost
the same as a single expansion; a spread now pays every time it appears
- the depth walk did not follow spreads, so a document three levels deep
could expand to fifty through fragments
- the introspection exemption used an OR, which a query mixing schema and
business fields could exploit
The estimate stays static by design: argument-dependent cost is bounded by
the complexity instrumentation and the request timeout instead.
* feat: serve graphql behind rate limit, body limit and compression
Binds the endpoint with the full security stack and wires the engine.
Two ordering bugs surfaced while wiring it, both caught by the route tests:
- the refusal check was registered as a sibling POST handler, so the
plugin's own handler won by registration order and a 200-alias bomb was
executed instead of refused; the guard is now an interception on the route,
which runs before any handler
- DoubleReceive is route-scoped, so it had to move to the GraphQL route; on
the application it would not have cached the body requestWeight reads
A refused document answers 400 with a GraphQL errors array rather than the
200 GraphQL uses for execution errors: it never reached the engine, so a
caller branching on the status code must be able to tell the two apart.
50 tests.
* fix: close the bypasses the security review found
Five holes, each covered by a new test:
- isIntrospectionField accepted a business field merely containing a meta
field, so a heavy query could ride under the introspection exemption
- a batch request was priced at the maximum but still executed, since the
guard skips when no single document can be parsed; now refused outright
- an unparseable document was let through, which is the exact shape used to
slip past a check that only understands valid syntax; now refused
- rateLimit.enabled = false crashed at startup, because the route still
referenced a limiter that was never registered
- the refusal message blamed depth when introspection was the actual reason
Also reads the query from the query string on a GET, so that path is no
longer unchecked, and reports the bound that actually broke.
The route body moved to bindGraphQLRoute so the rate-limited and opt-out
branches share it instead of duplicating it.
58 tests.
* fix(graphql): bound the cost and depth walks so a guard cannot be made to hang
The security stack refused the right documents but priced them at an
unaffordable price: a fragment ladder where each level spreads the next one
twice costs 2^k, so 815 bytes of payload made pricing take 5.5 s of CPU, and
eight of them in parallel held both cores for 10.6 s. The rate limiter did not
help, because the cost is paid before a single token is spent.
Both walks now stop on a budget derived from the configured bounds, and a
truncated walk reports the ceiling instead of its partial total. Arithmetic is
Long so a saturated cost cannot wrap to zero, and the pricing recursion is
depth-bounded so a deeply nested document cannot raise a StackOverflowError.
The ceiling is derived from maxQueryComplexity rather than hard-coded: a fixed
ceiling below a consumer's own bound would saturate under it and let the
hostile document pass.
Also fixed along the way:
- Introspection was answered 429. Exempt from the document bounds, but still
priced against a 100-unit bucket at 181 units, so GraphiQL and every codegen
client were broken. The bucket now holds several documents at the bound.
- A refused document charged the whole bucket, so a few hundred bytes of
malformed input locked out every client behind the same address for a whole
refill period. It now charges a moderate penalty.
- The rate limit key was the caller's address with no way to change it, which
shares one budget behind a NAT. It is now a parameter of configureGraphQL,
outside RateLimitConfig: a function in a data class becomes part of equals
and hashCode.
- An oversized body was reported as "the document could not be read" instead of
413, hiding the body limit behind a parsing error.
The depth walk is as explosive as the cost walk, so it carries the same budget,
and cost is checked first so a document refused on cost never pays for it.
Tests: 79 pass. The performance tests assert a deterministic node count rather
than a wall-clock time, which would be flaky on a shared runner and would give
false confidence about a regression to exponential expansion.
The build host has 3.7 GiB of RAM and runs the Gradle daemon, the Kotlin daemon
and the test fork at once, which exhausted memory and got the build killed. The
fork is now capped and daemons are disabled.
* test(graphql): assert the pricing walk scales linearly, not quadratically
The node-count assertions prove the walk is bounded, but not that it stays
linear once it is. A memoisation bug or a double-counting would keep every
document under the budget and still multiply the work.
A wall-clock threshold on a single document would not help: pricing a
4000-field document measures 0.013 ms, so the 250 ms threshold that was
proposed would still pass after a 3800x regression. Comparing two documents
and asserting the ratio is what actually discriminates -- measured 2.25 for the
current linear walk, against 16 for a quadratic one.
* fix(graphql): close two guard bypasses found by the three-model review
Both guards could be stepped around while still reaching the GraphQL handler.
URL encoding skipped every check. The interception compared `call.request.local.uri`, so
`POST /%67raphql` did not match `/graphql` and fell straight through to the handler, which the
router resolves because it decodes. A 300-alias document refused on `/graphql` answered 200 with
4 kB on `/%67raphql`, and a batch there was executed outright.
The fix is not `request.path()`: measured, that returns the *encoded* form, so swapping it in
changed nothing. The guard now compares both spellings, decoding the path only when it holds a
percent escape — `URLDecoder` unconditionally would turn a literal `+` into a space.
A fragment ladder hid inside `__schema`. `{ __schema { types { ...F0 } } }` qualified as pure
introspection on name alone and skipped both bounds: 422 bytes, HTTP 200 in 2.2 s, cost 201 never
measured. `isIntrospectionOnly` now checks whether a fragment spreads the *same* fragment more than
once, counted at any depth below it — repetition, not presence. That distinction is what lets the
real codegen query through: `...FullType` reaches `...InputValue` once, a bomb reaches its target
twice, usually through two different fields.
`__typename` is a companion meta field rather than a schema read. It was exempting documents that
select only it, including one aliasing it thousands of times; it now accompanies a real schema read
without granting the exemption on its own.
Two analyses could be walked exhaustively rather than bounded. `depthOf` reported its current level
when it ran out of budget, accepting a document whose expansion it never walked; it now reports past
every bound. The schema-read probe ran ahead of the budgeted cost walk and expanded a ladder until
an answer came back — 329 s on one test — and now carries an allowance of its own.
Also fixed: a cancelled client was answered 400 instead of propagating, the refusal message was
double-encoded as JSON, `MAX_TRACKED_DEPTH` could not exceed `maxQueryDepth + 1`, and two tests sent
a malformed document that measured the wrong thing.
Reviewed by three models — gemini-3.8-flash-high, gemini-3.1-pro-high, claude-opus-5-5-high — over
four rounds; the last returns ALL FIXED / NO REGRESSION / READY TO MERGE. 80 tests pass.
* feat: UUID scalar, typed parameter refusal and repository findAll (#54)
* feat(graphql): expose a UUID scalar for kotlin.uuid.Uuid
graphql-kotlin has no built-in scalar for kotlin.uuid.Uuid: a DTO carrying a Uuid identifier
fails schema generation and forces consumers into string-typed duplicates. UuidScalar speaks the
canonical hyphenated hexadecimal form in both directions, and UuidSchemaGeneratorHooks registers
it for every Uuid the schema meets while keeping the framework default hooks.
* feat(ktor): answer 400 when a parameter fails its type conversion
A malformed path or query parameter used to resolve to null and surface as a 500 through the
call. The resolver now refuses the request with InvalidParameterException, and the request
handler translates it to an HTTP 400 carrying a MessageDto error.
* feat(exposed): add AbstractRepository.findAll()
A whole-table read now comes from the repository layer: consumers call super.findAll() instead of
reaching for the Exposed entity class directly.
* fix(ktor): an optional parameter never refuses the request
A nullable parameter whose value does not match its type resolved through the conversion failure
path and refused the request, making an optional parameter behave like a required one. It now
falls back to null, exactly like an absent value; only a non-nullable parameter answers 400.
* style: name the UUID scalar uuidScalar and guard the parameter conversion
A top-level value is camelCase: uuidScalar, not UuidScalar. The parameter conversion now guards
its refusal first (a non-nullable parameter with an unconvertible value throws immediately) and
assigns afterwards, instead of nesting the reason inside a conditional expression.
* refactor: factor the parameter conversion and document the new contracts
The query and path branches each carried their own conversion and refusal block; both now call
parseParameter, which refuses first and returns the converted value. Guidelines updated with the
new rules: typed controller parameters with the 400 contract, optional parameters never refusing
the request, camelCase properties, and the UUID scalar documented in the graphql module.
* chore: bump version to 0.0.22-SNAPSHOT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #47. Six commits reviewed with Ziedelth:
graphql
uuidScalar(UuidCoercing,UuidSchemaGeneratorHooks): graphql-kotlin has no built-in scalar forkotlin.uuid.Uuid, so a DTO carrying a Uuid identifier failed schema generation and forced consumers into string-typed duplicates. The scalar speaks the canonical hyphenated hexadecimal form in both directions and is registered for everyUuidthe schema meets.ktor
@PathParam("id") id: Uuid); a non-nullable parameter whose value fails its conversion is refused with an HTTP 400 carrying aMessageDtoerror (InvalidParameterException).null.parseParameterhelper: refuse first, then assign.exposed
AbstractRepository.findAll(): whole-table reads come from the repository layer, consumers callsuper.findAll()instead of reaching for the entity class.guidelines
findAll), graphql/AGENTS.md (the UUID scalar).