Skip to content

feat(openapi): derive query parameters from a typed query contract#417

Open
bburda wants to merge 1 commit into
mainfrom
fix/openapi-query-params
Open

feat(openapi): derive query parameters from a typed query contract#417
bburda wants to merge 1 commit into
mainfrom
fix/openapi-query-params

Conversation

@bburda

@bburda bburda commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Make query parameters a typed, descriptor-driven contract like request and response bodies, fixing the generated spec which carried zero query parameters.

Handlers read query parameters ad-hoc via req.query_param(), so the OpenAPI generator never saw them and the exported spec - and every client generated from it - had no query parameters even though the gateway reads them at runtime.

  • dto::QueryParamWriter<T> emits the OpenAPI parameters (all in: query) for a query DTO from its dto_fields, mirroring SchemaWriter.
  • TypedRequest::query<T>() parses the request query string into a typed query DTO via the same descriptor.
  • RouteEntry::query<T>() declares those parameters on the route.

A handler can only read fields that exist on its query DTO, and those fields are exactly what the route advertises, so the parsed object and the spec cannot drift. Adding a parameter means adding a member - which appears in the spec automatically. A pre-commit guard bans raw query reads in handlers so the typed path is the only way in.

Applied to the endpoints whose handlers read query parameters:

  • GET /faults, GET /{entity}/faults: status, include_muted, include_clusters
  • DELETE /faults: status
  • GET /{entity}/logs: severity, context
  • GET /updates: origin, target-version

The exported spec now declares 26 query parameters (was 0).


Issue


Type

  • Bug fix
  • New feature or tests
  • Breaking change
  • Documentation only

Testing

  • colcon build clean; gateway unit suite green, including new RouteRegistry test (typed .query<T>() emits the parameters) and TypedRequest tests (typed query<T>() parsing of present/absent params).
  • End-to-end: ran the gateway and fetched /api/v1/docs; the spec now declares 26 in: query parameters across the faults/logs/updates endpoints with the correct schema types (string/boolean) and required: false.
  • clang-format / clang-tidy clean on changed files; the new pre-commit guard passes.

Checklist

  • Breaking changes are clearly described (none - additive query parameters)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed (the generated OpenAPI spec is the API surface; it now includes the query parameters)

Query parameters were the one part of the HTTP contract not derived from a
descriptor. Handlers read them ad-hoc via req.query_param(), and the OpenAPI
generator had no way to see them, so the exported spec - and every client
generated from it - carried zero query parameters even though the gateway reads
them at runtime.

Make query parameters a typed, descriptor-driven contract, like request and
response bodies:

- dto::QueryParamWriter<T> emits the OpenAPI `parameters` (all in: query) for a
  query DTO from its dto_fields, mirroring SchemaWriter.
- TypedRequest::query<T>() parses the request query string into a typed query
  DTO via the same descriptor.
- RouteEntry::query<T>() declares those parameters on the route.

A handler can only read fields that exist on its query DTO, and those fields are
exactly what the route advertises, so the parsed object and the spec cannot
drift. Adding a parameter means adding a member - which appears in the spec
automatically.

Applied to the endpoints whose handlers read query parameters:
- GET /faults and GET /{entity}/faults: status, include_muted, include_clusters
- DELETE /faults: status
- GET /{entity}/logs: severity, context
- GET /updates: origin, target-version

A pre-commit guard (check_handlers_typed_query.sh) bans raw query reads in
handlers so the typed path stays the only way in. Adds RouteRegistry and
TypedRequest tests covering the writer and reader.
Copilot AI review requested due to automatic review settings June 11, 2026 15:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes missing query parameters in the generated OpenAPI spec by introducing a typed, descriptor-driven “query DTO” contract that drives both OpenAPI parameters emission and handler-side query parsing, preventing drift between runtime behavior and the spec.

Changes:

  • Add dto::QueryParamWriter<T> + TypedRequest::query<T>() and RouteEntry::query<T>() to derive/query parameters from dto_fields<T>.
  • Migrate faults/logs/updates handlers and routes to the typed query DTOs, and update the OpenAPI route registry to accept bulk query parameters.
  • Add unit tests for typed query parsing and OpenAPI query-parameter emission, plus a pre-commit guard to block raw query reads in handlers.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/ros2_medkit_gateway/test/test_typed_router.cpp Adds unit tests for TypedRequest::query<T>() parsing into a query DTO.
src/ros2_medkit_gateway/test/test_route_registry.cpp Adds unit tests asserting OpenAPI query parameters are emitted (manual + typed DTO derived).
src/ros2_medkit_gateway/src/openapi/route_registry.hpp Introduces RouteEntry::add_query_parameters() and RouteEntry::query<T>() for typed query declaration.
src/ros2_medkit_gateway/src/http/rest_server.cpp Declares typed query DTOs on affected routes via .query<dto::…>().
src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp Switches update filtering to req.query<dto::UpdateListQuery>().
src/ros2_medkit_gateway/src/http/handlers/log_handlers.cpp Switches log query parsing to req.query<dto::LogQuery>().
src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp Switches faults filtering to typed DTOs; refactors status parsing away from raw request access.
src/ros2_medkit_gateway/src/core/openapi/route_registry.cpp Implements RouteEntry::add_query_parameters() to append generated parameter objects.
src/ros2_medkit_gateway/scripts/check_handlers_typed_query.sh Adds a pre-commit guard intended to ban raw query reads in handlers.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/typed_router.hpp Adds TypedRequest::query<T>() for descriptor-driven typed query parsing.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/updates.hpp Adds UpdateListQuery DTO + dto_fields for /updates query parameters.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/query.hpp Adds query DTO contract utilities (QueryParamWriter, assign_query_field, optional-unwrapping).
src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/logs.hpp Adds LogQuery DTO + dto_fields for /{entity}/logs query parameters.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/faults.hpp Adds FaultListQuery / FaultClearQuery DTOs + dto_fields for faults endpoints.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/http_utils.hpp Refactors fault status parsing utility to take an optional string (decoupled from httplib request).
.pre-commit-config.yaml Registers the new typed-query guard hook.

Comment on lines +33 to +40
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
HANDLERS_DIR="$(cd "${SCRIPT_DIR}/../src/http/handlers" && pwd)"

# Path reads (req.path), header reads (req.header), and fan-out (raw_for_framework
# for path/Authorization) are legitimate; only query-string reads are banned.
pattern='\.query_param\(|->query_param\(|get_param_value\('

hits="$(grep -rnE "${pattern}" "${HANDLERS_DIR}" --include='*.cpp' || true)"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Generated OpenAPI spec omits all query parameters

2 participants