Skip to content

feat(gateway): typed DTO contract for HTTP responses and OpenAPI schema#403

Merged
bburda merged 7 commits into
mainfrom
feat/338-openapi-dto-contract
Jun 1, 2026
Merged

feat(gateway): typed DTO contract for HTTP responses and OpenAPI schema#403
bburda merged 7 commits into
mainfrom
feat/338-openapi-dto-contract

Conversation

@bburda
Copy link
Copy Markdown
Collaborator

@bburda bburda commented May 18, 2026

Pull Request

Summary

The gateway's HTTP payloads were maintained as three independent, hand-written representations - the handler's JSON construction, the OpenAPI schema factories, and the XMedkit extension builder - with nothing keeping them in sync. They drifted: entity list responses advertised a minimal schema while the gateway actually emitted rich x-medkit metadata, so generated clients could not see those fields.

This introduces a typed DTO contract. Each payload is a plain C++17 struct described once by a constexpr field list. Three visitors fold over that single description:

  • JsonWriter - struct to wire JSON
  • SchemaWriter - type to OpenAPI schema
  • JsonReader - request body to struct, with validation

Wire output and OpenAPI schema are therefore generated from one source and cannot drift. All HTTP handler domains (entities, faults, operations, configurations, data, locks, triggers, cyclic-subscriptions, bulk-data, logs, scripts, updates, auth, health) were migrated to the contract, and components/schemas is now generated from the DTO registry. The legacy XMedkit fluent builder and the ~50 hand-written schema factories were removed.

Client-observable changes

  • The OpenAPI components/schemas is regenerated from the DTOs; several schema names changed (for example the entity list/detail schemas). Clients generated from the previous spec need to be regenerated.
  • Entity responses now always include the type discriminator (previously absent on list items).
  • Optional fields previously emitted as empty strings or empty arrays are now omitted when empty. A few fields previously emitted as an explicit null when absent (the script execution progress / started_at / completed_at / parameters / error, and the script parameters_schema) are likewise now omitted; treat an absent key the same as null.
  • Request-body validation failures on some PUT/POST endpoints now return the error code invalid-request (HTTP status 400 is unchanged).
  • Synchronous operation-execution failures (when the underlying ROS 2 service call fails) now return the standard SOVD GenericError envelope (error_code / vendor_code / parameters) instead of the previous bespoke nested {"error": {"code", "message", "details"}} object (HTTP 500 unchanged).
  • Aggregated list endpoints (data, operations, configurations, logs) now re-serialize peer items through the local DTO: peer items that fail validation are dropped and recorded in x-medkit.peer_dropped_items, and peer-supplied fields outside the local schema are no longer preserved verbatim.
  • GET /{entity}/data now publishes the opaque DataListResult schema (like GET /{entity}/faults) instead of a typed Collection<DataItem>. The wire payload is unchanged for runtime (ROS 2) entities; for plugin-owned entities, vendor per-item fields (for example the OPC-UA plugin's value / unit / data_type / writable) are now returned verbatim, whereas the previous typed re-parse silently dropped any field outside id / name / category. Clients that generated a typed DataItem model for this route should treat the list response as a free-form object.

Known limitation

The config/log list endpoints emit a richer collection-level x-medkit than the generic Collection<T> wrapper schema types it as. The wire payload stays a valid instance of the published schema; this is a schema-precision gap, documented in design/dto_contract.rst.


Issue

Part of #338.

This PR makes the gateway spec accurate. #338 stays open until generated clients are regenerated against the corrected spec and downstream consumers updated, which is follow-up work in separate repositories.


Type

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

Testing

  • New test_dto_contract unit suite round-trips every registered DTO through all three visitors and validates each generated schema.
  • Integration tests extended: live endpoint responses are validated against the served OpenAPI spec, and the typed x-medkit sub-schemas are asserted present in the spec.
  • Full ros2_medkit_gateway and ros2_medkit_integration_tests suites pass (unit, integration, linters); clang-tidy is clean.

Reviewers can verify by building ros2_medkit_gateway, running colcon test, and hitting GET /api/v1/docs to confirm components/schemas contains the typed entity and x-medkit schemas.


Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed

@bburda bburda marked this pull request as ready for review May 19, 2026 10:15
Copilot AI review requested due to automatic review settings May 19, 2026 10:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces a typed DTO contract for the gateway's HTTP responses so that wire JSON, OpenAPI schemas, and x-medkit extensions are all generated from a single C++17 source-of-truth, eliminating drift between handler output and the published spec. All HTTP handler domains were migrated, the legacy XMedkit fluent builder and ~50 hand-written schema factories were removed, and integration tests now validate live responses against the served spec.

Changes:

  • Add Draft202012/Draft7 jsonschema validator import with Humble fallback and new skip patterns in test_openapi_callability.test.py for schema limitations (Configuration/Trigger/CyclicSubscription).
  • Add new integration test methods to assert typed x-medkit sub-schemas (XMedkitArea, XMedkitRos2, AreaListItem) are present and that live entity GET responses conform to the served OpenAPI spec (with $ref-inlining helper).
  • Delete test_x_medkit.cpp since the XMedkit fluent builder is replaced by the DTO/visitor contract.

Reviewed changes

Copilot reviewed 70 out of 70 changed files in this pull request and generated no comments.

File Description
src/ros2_medkit_integration_tests/test/features/test_openapi_callability.test.py Adds DTO/schema presence checks, live-response-vs-spec conformance test, and skip patterns for known schema-precision gaps.
src/ros2_medkit_gateway/test/test_x_medkit.cpp Removed; superseded by the new DTO contract test suite (test_dto_contract).

Note: the PR description states the change touches all handler domains, OpenAPI builder, and removes ~50 schema factories, but only two files are visible in this diff slice. The bulk of the migration (handlers, DTO/visitor framework, OpenAPI builder rewiring, test_dto_contract) is outside the reviewed hunks and cannot be assessed here.

@bburda bburda self-assigned this May 19, 2026
@bburda bburda requested a review from mfaferek93 May 19, 2026 10:51
@bburda bburda force-pushed the feat/338-openapi-dto-contract branch from 1bed0f8 to 0d50544 Compare May 27, 2026 13:45
mfaferek93

This comment was marked as outdated.

mfaferek93

This comment was marked as outdated.

Comment thread src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/json_reader.hpp Outdated
Comment thread src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/scripts.hpp Outdated
Comment thread src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/updates.hpp
Comment thread src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/faults.hpp
Comment thread src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/faults.hpp Outdated
bburda added a commit that referenced this pull request May 28, 2026
…error renderer

3 auth routes migrate to typed handlers. Per-route .error_renderer(
kOAuth2Error) on RouteEntry wires errors through the framework's
write_oauth2_error primitive (RFC 6749 §5.2 shape: {error, error_description})
instead of SOVD GenericError.

Adds dto::TokenRequest, TokenResponse, TokenRevokeRequest,
TokenRevokeResponse for the auth endpoints (revoke was a missed
migration in PR #403). Wire format unchanged.
bburda added a commit that referenced this pull request May 28, 2026
7 operation routes migrate to typed handlers. create_execution uses
post_alternates<OperationExecutionResult, ExecutionCreateAsync>
for sync 200 / async 202 dispatch via dto_alternate_status trait.

Adds dto::ExecutionId + Collection<ExecutionId> for the previously
hand-built list_executions endpoint (missed migration from PR #403).
Removes the legacy pragma block now that all calls go through the
typed validators. Wire format unchanged.
bburda added a commit that referenced this pull request May 28, 2026
Updates design/dto_contract.rst with:
- Typed router section (reg.get<T>, Result<T>, static_assert is_dto_v)
- Escape hatches enumerated (sse, binary_download, multipart_upload,
  static_asset, docs_endpoint, alternates, plugin passthrough)
- Provider ABI typed-only policy
- Fan-out observability via peer_dropped_items
- Opaque object policy for runtime-shape fields
- OpenAPI generation pipeline now single-source from AllDtos

CHANGELOG.rst BREAKING entries:
- HandlerContext::send_*/parse_body removed
- Provider ABI returns typed DTOs
- RouteRegistry raw overloads deleted
- SchemaWriter optional fields emit anyOf+null (OpenAPI 3.1 idiom)
- Plugin ABI unchanged (test_plugin_abi_conformance locks it in)
- rtmaps_medkit not supported by this PR
bburda added 4 commits May 28, 2026 19:25
Introduces the typed DTO contract that lets a single C++ struct describe
each HTTP payload once, with three visitors folding over that description:
JsonWriter (DTO -> wire JSON), JsonReader (request body -> DTO with
validation), and SchemaWriter (type -> OpenAPI schema). Wire output and
OpenAPI schema therefore cannot drift because both are generated from the
same dto_fields<T> tuple.

Adds opaque_object(...) for fields whose runtime shape depends on context
(live ROS messages, plugin-defined payloads). Schema emits
{type:object, additionalProperties:true, x-medkit-opaque:true}; the
envelope around the opaque value stays typed.

Adds the typed RouteRegistry layer on top of cpp-httplib:
- reg.get<TResponse>(path, handler) where handler is
  Result<TResponse>(TypedRequest); the framework writes the response,
  handlers never see httplib::Response.
- static_assert(dto::is_dto_v<TResponse>) makes the typed path the only
  way to register a route.
- Named escape hatches: sse<TEvent>, binary_download, multipart_upload<T>,
  static_asset, docs_endpoint/docs_subtree, post_alternates<TBody,TAlt...>,
  del_alternates<TAlt...> for routes that return one of several typed
  shapes (200/202, 204/207). Per-route ErrorRenderer knob (SOVD generic
  error vs OAuth2 RFC 6749).

Framework primitives (write_json_body / write_generic_error /
write_oauth2_error) live behind a FrameworkOrPluginAccess friend gate so
handler code cannot reach them directly. The HandlerContext validator
surface (validate_entity_for_route, validate_collection_access,
validate_lock_access) returns tl::expected<T, ErrorInfo>; the typed
router wrapper writes the error and the framework owns response writing.

Closes the legacy invisible-drift gap on aggregation fan-out via
fan_out_collection<T>: peer items are parsed through JsonReader<T>,
malformed items are dropped into x-medkit.peer_dropped_items with a
WARN log so operators can see version skew in a 2-version cluster.

Collection<T, XMedkit = XMedkitCollection> 2-parameter template lets
per-domain list endpoints plug their richer XMedkit type directly into
the wrapper; previously the precision gap was documented and accepted,
now it's resolved.

SchemaWriter emits OpenAPI 3.1 anyOf+null idiom for std::optional<T>
fields so generated clients can express T | null instead of degrading
to T | undefined.

DTO sentinel trap is now a static_assert with an actionable message
pointing the developer at the right dto/<domain>.hpp header.
Provider virtual methods now return tl::expected<TypedDto, ErrorInfo>
instead of tl::expected<nlohmann::json, ErrorInfo>. The four provider
interfaces that previously returned raw JSON are migrated:

- UpdateProvider::get_update -> tl::expected<UpdateDetail, ...>
- OperationProvider::list_operations / get_operation / execute_operation
  -> typed Collection<OperationItem> / OperationItem /
  OperationExecutionResult. The execution result envelope is typed but
  carries an opaque ROS action payload (opaque_object marker).
- DataProvider::list_data / read_data / write_data -> opaque DataList /
  DataValue / DataWriteResult envelopes. DataValue's ros payload field
  is opaque because the shape depends on the ROS message type resolved
  at runtime.
- FaultProvider::list_faults / get_fault / clear_fault -> opaque
  FaultListResult / FaultDetailResult / FaultClearResult envelopes.
  Plugins frequently extend fault items with vendor-specific keys (UDS
  DTC environment records, OPC-UA alarm metadata); the opaque envelope
  preserves byte-for-byte plugin extensibility while exposing the typed
  C++ ABI.

Opaque envelope rationale: where plugin implementations need to attach
keys outside the SOVD vocabulary, the C++ ABI returns a typed wrapper
whose JsonWriter/JsonReader/SchemaWriter are specialised to pass the
inner json verbatim, with the schema marked x-medkit-opaque:true so
generated clients see "object whose runtime shape is implementation-
defined".

Wire format unchanged for every existing provider response.

In-tree open-source ros2_medkit_opcua plugin and the in-tree test
plugins are migrated to the typed signatures. Commercial plugin .so
files (UDS, OPC-UA gateway plugin, Uptane OTA, OTA) need parallel
migration PRs in their own repositories - their handlers compile
against the new typed signatures, returning typed wrappers around
their existing json payloads.

Tutorial / plugin-system.rst updated to show the typed signatures.
All 14 handler files migrate from raw void(httplib::Request, Response)
signatures to typed Result<TResponse>(TypedRequest [, TBody]):

- health, discovery (18 routes), lock (5 + 201/Location), trigger
  (6 incl. SSE), cyclic_subscription (6 incl. SSE), auth (3 + OAuth2
  error renderer), update (8 incl. 202/Location), log (3 typed
  fan-out), script (8 incl. multipart upload, HATEOAS _links typed),
  bulkdata (11 incl. binary_download + multipart_upload), config (5
  incl. del_alternates<NoContent, ConfigurationDeleteMultiStatus> for
  204/207), operation (7 incl. post_alternates for sync/async, plugin
  passthrough for free-form body), data (5 incl. opaque DataValue),
  fault (6 + SSE incl. del_alternates and X-Medkit-Local-Only
  attachment).

rest_server.cpp setup_routes is now exclusively typed reg.get<T>/
post<TB,T>/put<TB,T>/del<T>/patch<TB,T>/sse<TEvent>/binary_download/
multipart_upload<T>/static_asset/docs_endpoint. The deprecated raw
overloads on RouteRegistry are removed.

Issue #338 final compliance: per-item wire shape for fault list and
configuration list endpoints is now enforced by JsonReader<T> (closes
the "schema declares Items[T] but wire is raw json" gap).

Forwarded path through aggregation: validators that proxy to a peer
return Forwarded; the framework wrapper detects an internal sentinel
ErrorInfo and skips the response write so the proxied response stays
intact. Wire format byte-identical with the legacy direct-write
forwarded path.

operation_handlers plugin branch passes the raw request body to
OperationProvider::execute_operation rather than the typed
ExecutionCreateRequest envelope, preserving the pre-migration ABI for
plugin operations that read vendor-specific top-level keys (UDS
session_type / reset_type, OPC-UA acknowledge_fault arguments).

Tests:
- test_*_handlers.cpp rewritten to assert against Result<T> return
  values rather than http response state.
- Integration tests run unchanged - wire format byte-identical.
…HANGELOG

- test_plugin_abi_conformance.cpp: loads test_gateway_plugin.so via the
  production PluginLoader and verifies the plugin ABI is byte-stable.
  Twelve compile-time static_asserts guard the public layout of
  GatewayPlugin, PluginRoute, PluginRequest, PluginResponse; six runtime
  tests cover route registration, send_json content-type/indent, send_error
  vendor-error remapping and status clamping. If a future change breaks the
  plugin contract this test fails before commercial plugin builds catch it.

- typed_test_fixture.hpp: shared TypedRequest synthesis helper for the
  handler test suites that exercise the typed handlers directly.

- tsan_suppressions.txt: narrowly-scoped suppressions for libstdc++ future
  internals, yaml-cpp, vendored dynmsg, and the libyaml-cpp shared object.
  These races trace into third-party code reached via rclcpp GenericClient
  / dynamic ROS type introspection; our own JSON/transport paths remain
  visible to TSan.

- design/dto_contract.rst: typed router section, escape hatch enumeration,
  provider ABI typed-only policy, fan-out observability via
  peer_dropped_items, opaque object policy, OpenAPI generation pipeline.

- CHANGELOG.rst, README.md, docs/tutorials/openapi.rst: BREAKING entries
  on HandlerContext::send_* removal, provider ABI typed returns,
  RouteRegistry raw overload deletion, and the OpenAPI 3.1 anyOf+null
  representation for optional fields.

- test_openapi_callability.test.py: parametrized x-medkit presence test
  across all four entity types (Area / Component / App / Function) and
  updated optional-field probes to walk the new anyOf shape.
@bburda bburda force-pushed the feat/338-openapi-dto-contract branch from 86015c4 to 3048b43 Compare May 28, 2026 17:29
…se contract

Aggregation forwarding only proxied GET and SSE: the typed body, alternates,
delete-alternates, multipart, and binary wrappers never installed the
ForwardResponseScope, so writes and uploads to a remote entity returned an
empty no-op response instead of the peer's. Install the scope uniformly in
every wrapper (matching the documented "around every typed handler invocation"
invariant) and add forwarding regression tests for PUT, POST-alternates,
DELETE-alternates, and multipart routes.

Tighten the DTO contract:

- JsonReader treats an explicit JSON null as a value for free-form json and
  optional<json> members, so PUT data/configurations with {"data": null} no
  longer returns 400; null on other member types is still treated as absent.
- ScriptControlRequest.action uses plain field() instead of field_enum so
  plugin script backends may accept actions beyond the built-in
  stop/forced_termination; the provider validates the value.
- Restore the FaultListItem.severity_label response enum (now including
  UNKNOWN, which fault_to_json emits for out-of-range severities) and pin
  fault_to_json output to the FaultListItem schema with a round-trip test, so
  the verbatim fault-list wire cannot drift from its published schema.
- JsonWriter's value encoder ends in a static_assert like the reader and schema
  visitors, so an unsupported field type fails to compile instead of silently
  taking the scalar branch.
- Drop a no-op multipart parts initialization.

Document the synchronous operation service-call failure error-envelope
normalization and the fan-out re-serialization semantics in the changelog.
@bburda bburda force-pushed the feat/338-openapi-dto-contract branch from 226d169 to d3f3e42 Compare May 29, 2026 11:06
bburda added 2 commits May 31, 2026 21:59
… dedup handler helpers

GET /{entity}/data now returns the opaque DataListResult envelope (mirroring
the fault list route) instead of the typed Collection<DataItem>. The plugin
branch previously re-parsed the provider payload through a lenient
JsonReader<Collection<DataItem>>, which silently dropped any per-item field
outside {id,name,category} - including the OPC-UA plugin's value/unit/data_type/
writable. It now passes the provider payload through verbatim. Runtime entities
keep a byte-identical {items, x-medkit} wire shape, built from the typed
Collection<DataItem, DataListXMedkit> and wrapped into the envelope.

Also:
- schema_writer: optional enum fields attach `enum` to the non-null anyOf
  branch instead of as a top-level sibling, so the nullable claim and the enum
  constraint no longer contradict each other.
- Add a compile-time guard (static_assert on a non-empty dto_name<T>) in the
  schema registry and in schema_of, so a missing dto_name fails the build
  instead of producing an empty schema key and a dangling $ref.
- Hoist the duplicated make_error and flatten_validator_error helpers (copied
  across 12 handler translation units) into a shared handler_support.hpp.

Tests:
- Restore the sorted_contributors local-first/alphabetical ordering regression
  test that was lost with the removed x-medkit suite.
- Add JsonReader enum-rejection and optional-enum schema tests.
- Add a DataListResult regression test asserting the opaque envelope preserves
  vendor per-item fields and that the typed re-parse would have dropped them.

Docs: document the opaque data-list schema, the optional fields that change
from explicit null to omitted, and the always-present entity type discriminator.
… merged survivors)

The OpenAPI tutorial and the DTO contract design doc overstated the
field-walking path:

- components/schemas is now exactly collect_component_schemas(); the
  from_ros_msg / *_srv_* / binary_schema / generic_object_schema factories are
  no longer merged into it. They feed inline path-operation schemas for the
  per-topic / per-service / per-action routes whose shape comes from the live
  ROS 2 type, not from a named DTO.
- The Data*Result / Fault*Result / OperationExecutionResult opaque DTOs have no
  dto_fields; they carry a hand-written JsonWriter/JsonReader/SchemaWriter trio
  and publish an opaque object schema (type: object, additionalProperties: true,
  x-medkit-opaque: true) rather than a field-walked struct or a bare {}.
@mfaferek93 mfaferek93 self-requested a review June 1, 2026 14:47
@bburda bburda merged commit 3b3e01a into main Jun 1, 2026
19 checks passed
@bburda bburda deleted the feat/338-openapi-dto-contract branch June 1, 2026 15:21
@bburda bburda mentioned this pull request Jun 1, 2026
31 tasks
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.

3 participants