Skip to content

Commit 10c2f36

Browse files
committed
fix(gateway): preserve plugin data-list fields; harden DTO schema and 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.
1 parent 44bab19 commit 10c2f36

23 files changed

Lines changed: 314 additions & 387 deletions

src/ros2_medkit_gateway/CHANGELOG.rst

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,16 @@ Unreleased
4444
gateway still omits absent optional fields, and ``JsonReader`` continues
4545
to accept absent fields; the schema change only opts the published spec
4646
into round-tripping a literal ``null`` value cleanly for clients that
47-
prefer to send one. The ``rtmaps_medkit`` variant is explicitly NOT
48-
covered by this PR - its handlers continue to run on the pre-typed
49-
HandlerContext surface and will be migrated separately
47+
prefer to send one. As part of this, a handful of fields that were
48+
previously emitted as an explicit JSON ``null`` when absent are now omitted
49+
entirely (consistent with the optional-omission policy): the script
50+
execution fields ``progress`` / ``started_at`` / ``completed_at`` /
51+
``parameters`` / ``error`` (``GET .../scripts/{id}/executions/{eid}``) and
52+
the script ``parameters_schema`` field (``GET .../scripts/{id}``). Clients
53+
that tested ``field === null`` or relied on the key always being present must
54+
treat an absent key the same as ``null``. The ``rtmaps_medkit`` variant is
55+
explicitly NOT covered by this PR - its handlers continue to run on the
56+
pre-typed HandlerContext surface and will be migrated separately
5057
(`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
5158
* Synchronous operation-execution service-call failures
5259
(``POST /api/v1/{entity-path}/operations/{id}/executions`` when the underlying
@@ -59,14 +66,34 @@ Unreleased
5966
parsed ``error.code`` / ``error.details`` for this specific failure must read
6067
``vendor_code`` / ``parameters`` instead
6168
(`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
69+
* ``GET /api/v1/{entity-path}/data`` now publishes the opaque ``DataListResult``
70+
schema (``{type: object, additionalProperties: true, x-medkit-opaque: true}``),
71+
matching how ``GET .../faults`` already publishes ``FaultListResult``. The wire
72+
payload is unchanged for runtime (ROS 2) entities - it is still
73+
``{"items": [...], "x-medkit": {...}}`` built from the typed
74+
``Collection<DataItem, DataListXMedkit>`` - but for plugin-owned entities the
75+
provider's free-form per-item shape now passes through verbatim instead of
76+
being re-parsed into ``Collection<DataItem>``. This fixes a regression in which
77+
plugin per-item fields (the OPC-UA plugin's ``value`` / ``unit`` / ``data_type``
78+
/ ``writable``) were silently dropped by the typed re-parse. Clients that
79+
generated a typed ``DataItem`` model from the previous spec for this route now
80+
see an opaque object instead
81+
(`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
82+
* Entity responses (areas, components, apps, functions - list items and detail)
83+
now always carry a top-level ``type`` discriminator (an enum of
84+
``area`` / ``component`` / ``app`` / ``function``). Previously list items had no
85+
``type`` and detail responses exposed it only inside ``x-medkit.entityType``.
86+
Additive and tolerant-client-safe; consumers keying on the entity kind can now
87+
read the top-level ``type``
88+
(`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
6289
* ``ros2_medkit_msgs/srv/ClearFault`` request gains a ``bool skip_correlation_auto_clear`` field (see the per-entity fault scope entry below for the in-tree motivation). Adding a request field changes the service type hash, so out-of-tree callers that invoke the service directly (for example ``ros2 service call /fault_manager/clear_fault ros2_medkit_msgs/srv/ClearFault ...`` as documented in the ``ros2_medkit_fault_manager`` README) must rebuild against the new ``ros2_medkit_msgs`` to keep talking to ``fault_manager``. The in-tree gateway client and server are updated together (`#395 <https://github.com/selfpatch/ros2_medkit/issues/395>`_)
6390
* Per-entity fault routes are now correctly scoped to the entity's hosted apps. ``GET /api/v1/{entity-path}/faults/{fault_code}``, ``DELETE /api/v1/{entity-path}/faults/{fault_code}``, ``GET /api/v1/{entity-path}/faults``, and ``DELETE /api/v1/{entity-path}/faults`` previously fell back to a prefix match against the entity's ``namespace_path``; when that was empty (host-derived / synthetic components, manifest components without a ``namespace`` field, Areas, Functions, and Apps with a wildcard ``ros_binding.namespace_pattern``) the scope filter was silently disabled and the routes exposed - and on ``DELETE``, cleared - faults reported by apps that belonged to entirely different entities. All four handlers now resolve the addressed entity to its hosted-app FQN set (via the new ``HandlerContext::resolve_entity_source_fqns`` helper) and apply a strict all-sources scope check: a fault counts as in scope only when **every** entry in its ``reporting_sources`` is owned by the entity (exact FQN match, or strict path-child via ``<fqn>/...``). Per-fault routes return ``404 Resource Not Found`` for any fault that fails the check; collection routes return an empty ``items`` array. The underlying ``GetFault.srv`` contract is unchanged; ``ClearFault.srv`` gains a new ``skip_correlation_auto_clear`` request flag so per-entity DELETE can opt out of cascade-clearing correlated symptom fault codes that may live in other entities. Per-entity collection responses no longer include the global ``muted_count`` / ``cluster_count`` / ``muted_faults`` / ``clusters`` correlation metadata; those remain on the global ``GET /api/v1/faults`` route. Behavior changes visible to clients: (a) faults reported by apps outside the addressed entity are no longer returned or cleared via that entity's route, (b) **mixed-source** faults that include at least one out-of-entity reporter are likewise rejected with ``404`` on per-fault routes and excluded from per-entity collection responses (use the global ``GET /api/v1/faults`` to see them), (c) per-entity DELETE no longer cascade-clears correlated symptoms outside the entity (`#395 <https://github.com/selfpatch/ros2_medkit/issues/395>`_)
6491
* ``GET /api/v1/updates/{id}/status`` no longer returns ``404`` for a registered-but-idle package; ``POST /api/v1/updates`` now seeds a ``pending`` status, so the endpoint returns ``200 {"status": "pending"}`` immediately after registration. ``404`` is reserved for packages that are not registered. Clients that used ``404`` as a signal for "registered but nothing started yet" must adapt (`#378 <https://github.com/selfpatch/ros2_medkit/issues/378>`_)
6592

6693
**Features:**
6794

6895
* Typed ``fan_out_collection<T>`` aggregating helper replaces raw-JSON ``merge_peer_items`` on the typed collection routes (data, operations, config, logs). Peer items are decoded via ``dto::JsonReader<T>``; items that fail validation are removed from the merged ``items`` array, recorded in ``x-medkit.peer_dropped_items`` with the JsonReader error plus a best-effort ``source_id``, and logged at ``WARN``. Items that parse successfully are re-serialized through the local ``dto::JsonWriter<T>``, so any peer-supplied fields outside the local DTO schema are dropped from the merged response (the previous raw passthrough preserved unknown peer fields verbatim). Previously, malformed peer items silently disappeared into the merged response; fleet operators can now detect inter-gateway schema drift directly on the wire (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
69-
* ``Collection<T, XMedkitT>`` is now a 2-parameter template. Domain list endpoints (faults, config, data, logs) reference their richer per-domain collection x-medkit struct (``FaultListXMedkit``, ``ConfigListXMedkit``, ``DataListXMedkit``, ``LogListXMedkit``) directly in the published schema instead of the generic ``XMedkitCollection``, so generated clients see aggregation counts, peer provenance, and ``peer_dropped_items`` from the schema (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
96+
* ``Collection<T, XMedkitT>`` is now a 2-parameter template. Domain list endpoints (faults, config, logs) reference their richer per-domain collection x-medkit struct (``FaultListXMedkit``, ``ConfigListXMedkit``, ``LogListXMedkit``) directly in the published schema instead of the generic ``XMedkitCollection``, so generated clients see aggregation counts, peer provenance, and ``peer_dropped_items`` from the schema. The data list builds the same typed ``Collection<DataItem, DataListXMedkit>`` internally (so the wire still carries those fields) but publishes the opaque ``DataListResult`` envelope, because plugin-owned data entities can return vendor per-item fields the typed item schema cannot describe (see the data-list breaking-change entry above) (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
7097
* New ``opaque_object("key", &T::field)`` DTO field descriptor in ``dto/contract.hpp``. Binds a ``nlohmann::json`` member as a typed "any JSON object" field: ``JsonWriter`` emits it verbatim, ``JsonReader`` rejects scalars / arrays / null, ``SchemaWriter`` emits ``{type: object, additionalProperties: true, x-medkit-opaque: true}``. Used for fields whose runtime shape is decided by an upstream component the gateway cannot introspect (live ROS message payloads, plugin-defined fault envelopes, action results) (`#403 <https://github.com/selfpatch/ros2_medkit/issues/403>`_)
7198
* ``GET /api/v1/faults/stream`` event payloads now carry an optional ``x-medkit`` SOVD payload-extension object with ``entity_type`` and ``entity_id`` fields. When the gateway can resolve the fault's first reporting source back to a SOVD entity (via the manifest-mode linking index, or a runtime-mode last-segment match against an existing App), consumers can hit ``/{entity_type}/{entity_id}/bulk-data/rosbags/{fault_code}`` directly instead of HEAD-probing every entity. Resolution is snapshotted at event arrival, so a discovery refresh between enqueue and stream-out cannot retroactively change the entity reported to consumers. The ``x-medkit`` object is omitted entirely when no entity can be resolved, so existing SSE consumers ignore the addition (`#380 <https://github.com/selfpatch/ros2_medkit/issues/380>`_)
7299
* Plugin API version bumped to v7. Adds ``PluginContext::notify_entities_changed(EntityChangeScope)`` lifecycle hook for plugins that mutate the entity surface at runtime; default no-op keeps v6 source code compiling unchanged against v7 headers. Binary compatibility is not provided: the plugin loader uses a strict equality check on ``plugin_api_version()``, so out-of-tree plugins must be recompiled (`#376 <https://github.com/selfpatch/ros2_medkit/issues/376>`_)

src/ros2_medkit_gateway/design/dto_contract.rst

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -690,10 +690,21 @@ endpoints (areas, components, apps, functions) use the default
690690
``XMedkitCollection`` x-medkit; the domain collection endpoints specialise
691691
``XMedkitT`` to their richer per-domain shape (``FaultListXMedkit``,
692692
``FaultListAggXMedkit``, ``ConfigListXMedkit``, ``DataListXMedkit``,
693-
``LogListXMedkit``), so the published schema for each list response now
694-
references the actual collection x-medkit struct. Generated clients see the
695-
exact aggregation, peer-provenance, and ``peer_dropped_items`` fields that
696-
appear on the wire.
693+
``LogListXMedkit``). For the config and log list routes the published schema
694+
references the actual collection x-medkit struct directly, so generated clients
695+
see the exact aggregation, peer-provenance, and ``peer_dropped_items`` fields
696+
that appear on the wire.
697+
698+
The fault and data list routes are the exception: they publish the opaque
699+
``FaultListResult`` / ``DataListResult`` envelopes rather than the typed
700+
``Collection<...>`` schema, because plugin-owned entities can return
701+
vendor-specific per-item shapes that the typed item schema cannot describe.
702+
The data list handler still *builds* a typed
703+
``Collection<DataItem, DataListXMedkit>`` for runtime (ROS 2) entities and
704+
serializes it into the envelope (so the wire shape - including
705+
``peer_dropped_items`` - is unchanged), but the plugin branch passes the
706+
provider's free-form payload through verbatim. See "Opaque Object Policy" and
707+
the "Provider ABI" section above.
697708

698709
Key Files
699710
---------

src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/data_handlers.hpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,16 @@ class DataHandlers {
5252
*
5353
* GET /{entities}/{id}/data
5454
*
55-
* Returns SOVD ValueMetadata items (typed `DataItem`) with x-medkit ROS2-
56-
* specific extensions plus typed fan-out via `fan_out_collection<DataItem>`.
55+
* Returns the opaque `DataListResult` envelope. For runtime entities the
56+
* handler builds a typed `Collection<DataItem, DataListXMedkit>` (SOVD
57+
* ValueMetadata items with x-medkit ROS2 extensions plus typed fan-out via
58+
* `fan_out_collection<DataItem>`) and serializes it into the envelope, so the
59+
* wire shape is unchanged. For plugin entities the provider's free-form item
60+
* shape (which may carry vendor per-item fields the gateway cannot describe at
61+
* compile time) is passed through verbatim, instead of being re-parsed into
62+
* `Collection<DataItem>` (which would silently drop those fields).
5763
*/
58-
http::Result<dto::Collection<dto::DataItem, dto::DataListXMedkit>> list_data(const http::TypedRequest & req);
64+
http::Result<dto::DataListResult> list_data(const http::TypedRequest & req);
5965

6066
/**
6167
* @brief Get a single data item (topic value) for an entity.

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/registry.hpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,18 @@ using AllDtos =
8585
VersionInfoEntry, XMedkitVersionInfo, VersionInfo, RootCapabilities, RootAuth, RootTls, RootOverview>;
8686

8787
namespace detail {
88+
template <class T>
89+
void collect_one(nlohmann::json & schemas) {
90+
static_assert(!dto_name<T>.empty(),
91+
"collect_component_schemas: a DTO in AllDtos is missing a dto_name<T> specialization "
92+
"(would write its schema under an empty \"\" key and collide with others)");
93+
schemas[std::string(dto_name<T>)] = SchemaWriter<T>::schema();
94+
}
95+
8896
template <class Tuple, std::size_t... I>
8997
nlohmann::json collect_impl(std::index_sequence<I...> /*seq*/) {
9098
nlohmann::json schemas = nlohmann::json::object();
91-
((schemas[std::string(dto_name<std::tuple_element_t<I, Tuple>>)] =
92-
SchemaWriter<std::tuple_element_t<I, Tuple>>::schema()),
93-
...);
99+
(collect_one<std::tuple_element_t<I, Tuple>>(schemas), ...);
94100
return schemas;
95101
}
96102
} // namespace detail

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/schema_writer.hpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ nlohmann::json variant_schema(std::index_sequence<I...> /*seq*/) {
4040
template <class U>
4141
nlohmann::json schema_of() {
4242
if constexpr (is_dto_v<U>) {
43+
static_assert(!dto_name<U>.empty(),
44+
"schema_of: DTO type is missing a dto_name<T> specialization "
45+
"(would emit an empty \"$ref\" and a \"\" schema key)");
4346
return nlohmann::json{{"$ref", "#/components/schemas/" + std::string(dto_name<U>)}};
4447
} else if constexpr (is_optional_v<U>) {
4548
auto inner = schema_of<typename U::value_type>();
@@ -88,7 +91,16 @@ struct SchemaWriter {
8891
for (std::size_t i = 0; i < f.enum_count; ++i) {
8992
values.push_back(std::string(f.enum_values[i]));
9093
}
91-
prop["enum"] = values;
94+
// For optional members schema_of() yields {anyOf:[<inner>, {null}]};
95+
// attach the enum to the non-null branch ([0]) so the nullable claim
96+
// and the enum constraint agree (a top-level enum lacking "null" would
97+
// reject the null that anyOf advertises). Required members get the
98+
// enum at the top level.
99+
if (prop.contains("anyOf") && prop["anyOf"].is_array() && !prop["anyOf"].empty()) {
100+
prop["anyOf"][0]["enum"] = values;
101+
} else {
102+
prop["enum"] = values;
103+
}
92104
}
93105
props[std::string(f.key)] = prop;
94106
if (f.presence == Presence::kRequired) {
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2026 bburda
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#pragma once
16+
17+
#include <string>
18+
#include <type_traits>
19+
#include <utility>
20+
#include <variant>
21+
22+
#include <nlohmann/json.hpp>
23+
24+
#include "ros2_medkit_gateway/core/models/error_info.hpp"
25+
#include "ros2_medkit_gateway/http/handlers/handler_context.hpp"
26+
#include "ros2_medkit_gateway/http/typed_router.hpp"
27+
28+
namespace ros2_medkit_gateway {
29+
namespace handlers {
30+
31+
/// Build a SOVD-shaped ErrorInfo. Empty `params` are dropped so the wire body
32+
/// matches the legacy `send_error` default and integration tests stay byte-
33+
/// identical. Shared by every typed handler (was duplicated per handler TU).
34+
inline ErrorInfo make_error(int status, const std::string & code, std::string message, nlohmann::json params = {}) {
35+
ErrorInfo err;
36+
err.code = code;
37+
err.message = std::move(message);
38+
err.http_status = status;
39+
if (!params.is_null() && !params.empty()) {
40+
err.params = std::move(params);
41+
}
42+
return err;
43+
}
44+
45+
/// Collapse a `validate_entity_for_route` validator error into a single
46+
/// `ErrorInfo`: the local-error branch passes through, while the `Forwarded`
47+
/// branch becomes the framework-internal sentinel that the RouteRegistry
48+
/// wrapper recognises and skips error rendering for. Shared by every typed
49+
/// handler (was duplicated per handler TU).
50+
inline ErrorInfo flatten_validator_error(const std::variant<ErrorInfo, http::Forwarded> & err) {
51+
return std::visit(
52+
[](auto && alt) -> ErrorInfo {
53+
using T = std::decay_t<decltype(alt)>;
54+
if constexpr (std::is_same_v<T, ErrorInfo>) {
55+
return alt;
56+
} else {
57+
return HandlerContext::forwarded_sentinel_error();
58+
}
59+
},
60+
err);
61+
}
62+
63+
} // namespace handlers
64+
} // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/src/http/handlers/bulkdata_handlers.cpp

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "ros2_medkit_gateway/dto/bulkdata.hpp"
3636
#include "ros2_medkit_gateway/dto/entities.hpp"
3737
#include "ros2_medkit_gateway/gateway_node.hpp"
38+
#include "ros2_medkit_gateway/http/handlers/handler_support.hpp"
3839

3940
using json = nlohmann::json;
4041

@@ -43,37 +44,6 @@ namespace handlers {
4344

4445
namespace {
4546

46-
/// Build a SOVD-shaped ErrorInfo. Empty `params` are dropped so the wire body
47-
/// matches the legacy `send_error` default and integration tests stay byte-
48-
/// identical.
49-
ErrorInfo make_error(int status, const std::string & code, std::string message, json params = {}) {
50-
ErrorInfo err;
51-
err.code = code;
52-
err.message = std::move(message);
53-
err.http_status = status;
54-
if (!params.is_null() && !params.empty()) {
55-
err.params = std::move(params);
56-
}
57-
return err;
58-
}
59-
60-
/// Convert a ValidatorResult's error variant into a typed Result<T> error.
61-
/// When the validator returned Forwarded, the proxy already wrote the wire
62-
/// response, so the handler signals "do not render" via the framework-internal
63-
/// sentinel (ERR_X_INTERNAL_FORWARDED) the typed wrapper detects.
64-
ErrorInfo flatten_validator_error(const std::variant<ErrorInfo, http::Forwarded> & err) {
65-
return std::visit(
66-
[](auto && alt) -> ErrorInfo {
67-
using T = std::decay_t<decltype(alt)>;
68-
if constexpr (std::is_same_v<T, ErrorInfo>) {
69-
return alt;
70-
} else {
71-
return HandlerContext::forwarded_sentinel_error();
72-
}
73-
},
74-
err);
75-
}
76-
7747
/// Resolve the entity_id from the typed request. Bulk-data routes embed the
7848
/// entity reference in the URL path; the registered route patterns capture
7949
/// the entity id as group 1 (single-entity) or group 2 (nested subarea /

0 commit comments

Comments
 (0)