diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca4c7ad4..9a34a9c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,7 +280,8 @@ jobs: - name: Install dependencies run: | apt-get update - apt-get install -y lcov ros-jazzy-test-msgs + # gpg is required by codecov/codecov-action@v5 dependency check + apt-get install -y lcov ros-jazzy-test-msgs gpg source /opt/ros/jazzy/setup.bash rosdep update rosdep install --from-paths src --ignore-src -r -y diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 415ec189..eabac9d6 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -229,8 +229,11 @@ jobs: steps: - name: Free disk space run: | + # Host paths are no-ops inside the container - safe to keep + # for the case where the workflow is run without `container:`. rm -rf /usr/local/lib/android /usr/share/dotnet /opt/ghc || true apt-get clean + df -h / - name: Install Git run: | @@ -271,10 +274,18 @@ jobs: CCACHE_SLOPPINESS: pch_defines,time_macros run: | source /opt/ros/jazzy/setup.bash + # RelWithDebInfo (not Debug): Debug uses -O0 which produces + # binaries ~2x the size of -O1, and the ASan/UBSan instrumentation + # already inflates them several times over. On github-hosted + # runners (~14 GB /) the linker hits 'No space left on device' on + # PR branches that miss the ccache warm-up. The sanitizer cmake + # module overrides optimisation back to -O1 either way, so this + # only changes debug info size. colcon build --symlink-install \ - --cmake-args -DCMAKE_BUILD_TYPE=Debug -DSANITIZER=asan,ubsan \ + --cmake-args -DCMAKE_BUILD_TYPE=RelWithDebInfo -DSANITIZER=asan,ubsan \ --event-handlers console_direct+ ccache -s + df -h / - name: Extend test timeouts for ASan overhead run: | diff --git a/docs/api/rest.rst b/docs/api/rest.rst index f2b1bcfb..ee21d7e4 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -277,6 +277,49 @@ Apps Unknown apps return ``404 App not found`` with ``parameters.app_id``. +``GET /api/v1/apps/{app_id}/belongs-to`` + Return the area that contains this app via its parent component. + + Per SOVD (ISO 17978-3 §7.6), the corresponding + ``belongs-to`` URI reference in ``GET /apps/{app_id}`` is only emitted when + the app has a parent component (i.e. is not standalone). Standalone apps do + not expose this subresource in HATEOAS and the endpoint will return an empty + ``items`` collection if called directly. + + The response follows the standard ``items`` wrapper and returns: + + - ``0`` items when the app has no associated host component (standalone app) + - ``0`` items when the parent component has no assigned area + - ``1`` item when the area is resolved + - ``1`` item with ``x-medkit.missing=true`` when the parent component references + an area that cannot currently be resolved + - ``1`` item with ``x-medkit.missing=true`` and ``x-medkit.unresolved_component`` + set to the dangling component id when the app references a parent component + that cannot currently be resolved (manifest broken / component removed) + + **Example Response:** + + .. code-block:: json + + { + "items": [ + { + "id": "engine", + "name": "Engine", + "href": "/api/v1/areas/engine" + } + ], + "x-medkit": { + "total_count": 1 + }, + "_links": { + "self": "/api/v1/apps/engine-temp-sensor/belongs-to", + "app": "/api/v1/apps/engine-temp-sensor" + } + } + + Unknown apps return ``404 App not found`` with ``parameters.app_id``. + Functions ~~~~~~~~~ diff --git a/docs/requirements/specs/discovery.rst b/docs/requirements/specs/discovery.rst index 901c838b..24ec2f25 100644 --- a/docs/requirements/specs/discovery.rst +++ b/docs/requirements/specs/discovery.rst @@ -84,3 +84,13 @@ Discovery :tags: Discovery The endpoint shall return the component that hosts the addressed application. + +.. req:: GET /apps/{id}/belongs-to + :id: REQ_INTEROP_106 + :status: verified + :tags: Discovery + + The endpoint shall return the area that contains the addressed application, + resolved transitively via the app's parent component and that component's + area assignment. The response uses the standard ``items`` wrapper and + contains zero or one element. diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/include/ros2_medkit_param_beacon/param_beacon_plugin.hpp b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/include/ros2_medkit_param_beacon/param_beacon_plugin.hpp index 8c85724e..97e1bd5c 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/include/ros2_medkit_param_beacon/param_beacon_plugin.hpp +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/include/ros2_medkit_param_beacon/param_beacon_plugin.hpp @@ -45,7 +45,7 @@ class ParameterBeaconPlugin : public ros2_medkit_gateway::GatewayPlugin, public ros2_medkit_gateway::IntrospectionProvider { public: ParameterBeaconPlugin() = default; - ~ParameterBeaconPlugin() override; + ~ParameterBeaconPlugin() noexcept override; /// Constructor with injectable client factory (for testing). explicit ParameterBeaconPlugin(ros2_medkit_param_beacon::ParameterClientFactory factory) diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/src/param_beacon_plugin.cpp b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/src/param_beacon_plugin.cpp index 9a53e84d..93f2d87e 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/src/param_beacon_plugin.cpp +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_param_beacon/src/param_beacon_plugin.cpp @@ -34,8 +34,14 @@ using ros2_medkit_gateway::PLUGIN_API_VERSION; using ros2_medkit_gateway::PluginContext; using ros2_medkit_gateway::SovdEntityType; -ParameterBeaconPlugin::~ParameterBeaconPlugin() { - shutdown(); +ParameterBeaconPlugin::~ParameterBeaconPlugin() noexcept { + // On Rolling, ~rclcpp::Node can throw graph_listener::NodeNotFoundError + // once rclcpp::shutdown() has invalidated the context. An exception + // escaping a destructor calls std::terminate(), so swallow it here. + try { + shutdown(); + } catch (...) { + } } std::string ParameterBeaconPlugin::name() const { @@ -151,7 +157,14 @@ void ParameterBeaconPlugin::shutdown() { backoff_counts_.clear(); skip_remaining_.clear(); } - param_node_.reset(); + // ~rclcpp::Node can throw graph_listener::NodeNotFoundError on Rolling + // when the context was already torn down by rclcpp::shutdown(). Swallow + // it so the plugin_manager shutdown sequence (and the plugin destructor + // that calls back into us) does not abort the process. + try { + param_node_.reset(); + } catch (...) { + } } std::vector ParameterBeaconPlugin::get_routes() { diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/include/ros2_medkit_topic_beacon/topic_beacon_plugin.hpp b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/include/ros2_medkit_topic_beacon/topic_beacon_plugin.hpp index 0be750cd..e5753100 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/include/ros2_medkit_topic_beacon/topic_beacon_plugin.hpp +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/include/ros2_medkit_topic_beacon/topic_beacon_plugin.hpp @@ -81,7 +81,7 @@ class TopicBeaconPlugin : public ros2_medkit_gateway::GatewayPlugin, public ros2 void configure(const nlohmann::json & config) override; void set_context(ros2_medkit_gateway::PluginContext & context) override; void shutdown() override; - ~TopicBeaconPlugin() override; + ~TopicBeaconPlugin() noexcept override; std::vector get_routes() override; ros2_medkit_gateway::IntrospectionResult introspect(const ros2_medkit_gateway::IntrospectionInput & input) override; diff --git a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/src/topic_beacon_plugin.cpp b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/src/topic_beacon_plugin.cpp index e2ecdf31..aa8ce05a 100644 --- a/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/src/topic_beacon_plugin.cpp +++ b/src/ros2_medkit_discovery_plugins/ros2_medkit_topic_beacon/src/topic_beacon_plugin.cpp @@ -32,8 +32,14 @@ using ros2_medkit_gateway::PLUGIN_API_VERSION; using ros2_medkit_gateway::PluginContext; using ros2_medkit_gateway::SovdEntityType; -TopicBeaconPlugin::~TopicBeaconPlugin() { - shutdown(); +TopicBeaconPlugin::~TopicBeaconPlugin() noexcept { + // On Rolling, ~rclcpp::Subscription can throw graph_listener::NodeNotFoundError + // once rclcpp::shutdown() has invalidated the context. An exception + // escaping a destructor calls std::terminate(), so swallow it here. + try { + shutdown(); + } catch (...) { + } } std::string TopicBeaconPlugin::name() const { @@ -107,7 +113,13 @@ void TopicBeaconPlugin::shutdown() { if (shutdown_requested_.exchange(true)) { return; } - subscription_.reset(); + // ~rclcpp::Subscription can throw on Rolling when the rclcpp context + // was torn down before us; swallow so plugin_manager shutdown and the + // plugin destructor calling back into us do not abort the process. + try { + subscription_.reset(); + } catch (...) { + } } std::vector TopicBeaconPlugin::get_routes() { diff --git a/src/ros2_medkit_gateway/config/examples/demo_nodes_manifest.yaml b/src/ros2_medkit_gateway/config/examples/demo_nodes_manifest.yaml index f7f01133..7947a315 100644 --- a/src/ros2_medkit_gateway/config/examples/demo_nodes_manifest.yaml +++ b/src/ros2_medkit_gateway/config/examples/demo_nodes_manifest.yaml @@ -74,6 +74,14 @@ areas: namespace: /perception/lidar description: "LiDAR sensor system" + # Isolated area used by HATEOAS edge-case manifest entries below; not bound + # to a running ROS 2 node, exists only to give the unresolved/standalone/ + # area-less tests deterministic targets. + - id: hateoas-edge-area + name: "HATEOAS Edge Area" + namespace: /hateoas_edge + description: "Reserved for integration test edge cases" + # ============================================================================= # COMPONENTS - Hardware units # ============================================================================= @@ -135,6 +143,20 @@ components: name: "LiDAR Unit" type: "sensor" area: lidar + + # HATEOAS edge-case fixtures (manifest-only, no runtime nodes). + # See test_hateoas.test.py for the corresponding test cases. + - id: hateoas-component-no-area + name: "HATEOAS Edge: component without area" + type: "sensor" + # area intentionally omitted - exercises the + # 'parent component resolved but has no area_id' branch. + + # Note: the 'dangling area reference' branch (component points at a + # non-existent area id) is covered by unit test + # AppBelongsToReturnsMissingItemWhenAreaUnresolved. We can't seed a + # component with a missing area_id here because manifest validator R006 + # rejects it and disables manifest-driven discovery for the whole suite. description: "360° laser scanner with fault reporting" # ============================================================================= @@ -231,6 +253,28 @@ apps: node_name: lidar_sensor namespace: /perception/lidar + # === HATEOAS edge-case apps (manifest-only, no ros_binding) === + # Each targets a specific branch of handle_app_belongs_to / + # handle_app_is_located_on so the integration suite exercises the same + # shapes the unit tests cover. + - id: hateoas-app-standalone + name: "HATEOAS Edge: standalone app" + category: "fixture" + description: "App without a parent component (is_located_on omitted)" + + # Note: the 'unresolved parent component' branch is covered by unit test + # AppBelongsToReturnsMissingItemWhenParentComponentUnresolved instead - the + # manifest validator (R007) rejects apps that reference a non-existent + # component even with manifest_strict_validation=false, so we can't seed it + # here as an integration fixture without disabling discovery for the rest + # of the suite. + + - id: hateoas-app-on-area-less-component + name: "HATEOAS Edge: app on component without area" + category: "fixture" + description: "Parent component resolves but has no area_id assigned" + is_located_on: hateoas-component-no-area + # ============================================================================= # FUNCTIONS - High-level capabilities # ============================================================================= diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/capability_builder.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/capability_builder.hpp index 02040479..b40cb143 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/capability_builder.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/capability_builder.hpp @@ -52,6 +52,8 @@ class CapabilityBuilder { RELATED_APPS, ///< Entity has related apps (components only) HOSTS, ///< Entity has host apps (functions/components) DEPENDS_ON, ///< Entity has dependencies (components only) + IS_LOCATED_ON, ///< Entity has parent component (apps only) + BELONGS_TO, ///< Entity has parent area (apps only) LOGS, ///< Entity has application log entries (components and apps) BULK_DATA, ///< Entity has bulk data endpoints (rosbags) CYCLIC_SUBSCRIPTIONS, ///< Entity has cyclic subscription endpoints diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/discovery_handlers.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/discovery_handlers.hpp index 98566cc7..7f0fabf7 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/discovery_handlers.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/handlers/discovery_handlers.hpp @@ -130,6 +130,12 @@ class DiscoveryHandlers { */ void handle_app_is_located_on(const httplib::Request & req, httplib::Response & res); + /** + * @brief Handle GET /apps/{app-id}/belongs-to - get parent area via component. + * @verifies REQ_INTEROP_106 + */ + void handle_app_belongs_to(const httplib::Request & req, httplib::Response & res); + // ========================================================================= // Function endpoints // ========================================================================= diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/models/thread_safe_entity_cache.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/models/thread_safe_entity_cache.hpp index 0e8c7124..7bb1f8d3 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/models/thread_safe_entity_cache.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/models/thread_safe_entity_cache.hpp @@ -208,6 +208,46 @@ class ThreadSafeEntityCache { std::optional get_app(const std::string & id) const; std::optional get_function(const std::string & id) const; + /// Atomic snapshot of an App together with its parent Component and Area. + /// + /// Three sequential get_app/get_component/get_area calls each acquire a + /// fresh shared_lock and a writer refresh can advance the generation + /// between them, so handlers that traverse App -> Component -> Area can + /// observe a mixed-generation view (e.g. app from gen N, component from + /// N+1, area from N). This helper resolves the chain under a single + /// shared_lock so the result is internally consistent. + /// + /// `app` is empty if `app_id` is not in the cache. `component` is empty if + /// the app has no `component_id` or the referenced component is missing. + /// `area` is empty if the component has no `area` or the referenced area + /// is missing. The two latter cases are distinguishable from "no parent" + /// because `app.component_id` / `component.area` remain set on the + /// returned models. + struct AppLinksSnapshot { + std::optional app; + std::optional component; + std::optional area; + }; + AppLinksSnapshot get_app_with_links(const std::string & id) const; + + /// Atomic snapshot of an App together with its declared `depends_on` apps. + /// + /// `handle_app_depends_on` iterates `app.depends_on` and resolves each id + /// via `get_app()` - one shared_lock per dependency. A writer refresh can + /// land between the app fetch and any of the per-dependency fetches, so a + /// 5-dependency app can return 5 apps from 5 different cache generations. + /// This helper takes a single shared_lock and returns the app together + /// with every dependency resolved in the same generation. + /// + /// `app` is empty if `app_id` is not in the cache. `dependencies` lists + /// every entry in `app->depends_on` in declaration order; the optional is + /// empty when the referenced dependency cannot be resolved (broken ref). + struct AppDependenciesSnapshot { + std::optional app; + std::vector>> dependencies; + }; + AppDependenciesSnapshot get_app_with_dependencies(const std::string & id) const; + // --- Check existence (O(1)) --- bool has_area(const std::string & id) const; bool has_component(const std::string & id) const; diff --git a/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp b/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp index 8f4c6598..7e5c47a4 100644 --- a/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp +++ b/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp @@ -47,6 +47,8 @@ const std::unordered_map> & AuthConfig "GET:/api/v1/components/*/hosts", "GET:/api/v1/components/*/depends-on", "GET:/api/v1/apps/*/depends-on", + "GET:/api/v1/apps/*/is-located-on", + "GET:/api/v1/apps/*/belongs-to", "GET:/api/v1/functions/*/hosts", // Data: all entity types "GET:/api/v1/components/*/data", @@ -186,6 +188,8 @@ const std::unordered_map> & AuthConfig "GET:/api/v1/components/*/hosts", "GET:/api/v1/components/*/depends-on", "GET:/api/v1/apps/*/depends-on", + "GET:/api/v1/apps/*/is-located-on", + "GET:/api/v1/apps/*/belongs-to", "GET:/api/v1/functions/*/hosts", // Data: all entity types (read) "GET:/api/v1/components/*/data", @@ -386,6 +390,8 @@ const std::unordered_map> & AuthConfig "GET:/api/v1/components/*/hosts", "GET:/api/v1/components/*/depends-on", "GET:/api/v1/apps/*/depends-on", + "GET:/api/v1/apps/*/is-located-on", + "GET:/api/v1/apps/*/belongs-to", "GET:/api/v1/functions/*/hosts", // Data: all entity types (read) "GET:/api/v1/components/*/data", diff --git a/src/ros2_medkit_gateway/src/core/discovery/models/app.cpp b/src/ros2_medkit_gateway/src/core/discovery/models/app.cpp index 2048d8af..5eff4cd8 100644 --- a/src/ros2_medkit_gateway/src/core/discovery/models/app.cpp +++ b/src/ros2_medkit_gateway/src/core/discovery/models/app.cpp @@ -125,6 +125,7 @@ json App::to_capabilities(const std::string & base_url) const { // Relationships (SOVD standard) if (!component_id.empty()) { j["is-located-on"] = base_url + "/components/" + component_id; + j["belongs-to"] = app_base + "/belongs-to"; } if (!depends_on.empty()) { j["depends-on"] = app_base + "/depends-on"; diff --git a/src/ros2_medkit_gateway/src/core/http/handlers/capability_builder.cpp b/src/ros2_medkit_gateway/src/core/http/handlers/capability_builder.cpp index bc8448d7..43adf2a3 100644 --- a/src/ros2_medkit_gateway/src/core/http/handlers/capability_builder.cpp +++ b/src/ros2_medkit_gateway/src/core/http/handlers/capability_builder.cpp @@ -41,6 +41,10 @@ std::string CapabilityBuilder::capability_to_name(Capability cap) { return "hosts"; case Capability::DEPENDS_ON: return "depends-on"; + case Capability::IS_LOCATED_ON: + return "is-located-on"; + case Capability::BELONGS_TO: + return "belongs-to"; case Capability::LOGS: return "logs"; case Capability::BULK_DATA: diff --git a/src/ros2_medkit_gateway/src/core/models/entity_capabilities.cpp b/src/ros2_medkit_gateway/src/core/models/entity_capabilities.cpp index 99dad755..ee8a8948 100644 --- a/src/ros2_medkit_gateway/src/core/models/entity_capabilities.cpp +++ b/src/ros2_medkit_gateway/src/core/models/entity_capabilities.cpp @@ -29,8 +29,11 @@ EntityCapabilities EntityCapabilities::for_type(SovdEntityType type) { ResourceCollection::LOGS, ResourceCollection::TRIGGERS, ResourceCollection::SCRIPTS, ResourceCollection::UPDATES, }; - // SERVER resources - caps.resources_ = {"docs", "version-info", "logs", "belongs-to", "depends-on", "data-categories", "data-groups"}; + // SERVER resources. SOVD (ISO 17978-3 §7.6) does not define + // /belongs-to for server, only for apps - advertising it here would + // make supports_resource("belongs-to") return true and clients would + // get 404 when following it. + caps.resources_ = {"docs", "version-info", "logs", "depends-on", "data-categories", "data-groups"}; break; case SovdEntityType::AREA: @@ -56,8 +59,11 @@ EntityCapabilities EntityCapabilities::for_type(SovdEntityType type) { ResourceCollection::LOGS, ResourceCollection::TRIGGERS, ResourceCollection::SCRIPTS, ResourceCollection::UPDATES, }; - caps.resources_ = {"docs", "logs", "hosts", "belongs-to", - "depends-on", "subcomponents", "data-categories", "data-groups"}; + // SOVD (ISO 17978-3 §7.6) defines /belongs-to only for apps; component + // exposes parent area via /is-located-on (which is itself app-only in + // the spec, but ros2_medkit treats it as the canonical area pointer). + // Listing belongs-to here would be a 404 promise. + caps.resources_ = {"docs", "logs", "hosts", "depends-on", "subcomponents", "data-categories", "data-groups"}; break; case SovdEntityType::APP: diff --git a/src/ros2_medkit_gateway/src/core/models/thread_safe_entity_cache.cpp b/src/ros2_medkit_gateway/src/core/models/thread_safe_entity_cache.cpp index c1947b17..4b3316f2 100644 --- a/src/ros2_medkit_gateway/src/core/models/thread_safe_entity_cache.cpp +++ b/src/ros2_medkit_gateway/src/core/models/thread_safe_entity_cache.cpp @@ -159,6 +159,63 @@ std::optional ThreadSafeEntityCache::get_app(const std::string & id) const return std::nullopt; } +ThreadSafeEntityCache::AppLinksSnapshot ThreadSafeEntityCache::get_app_with_links(const std::string & id) const { + AppLinksSnapshot snapshot; + std::shared_lock lock(mutex_); + + auto app_it = app_index_.find(id); + if (app_it == app_index_.end() || app_it->second >= apps_.size()) { + return snapshot; + } + snapshot.app = apps_[app_it->second]; + + const auto & component_id = snapshot.app->component_id; + if (component_id.empty()) { + return snapshot; + } + + auto comp_it = component_index_.find(component_id); + if (comp_it == component_index_.end() || comp_it->second >= components_.size()) { + return snapshot; // component referenced but missing - leave .component empty + } + snapshot.component = components_[comp_it->second]; + + const auto & area_id = snapshot.component->area; + if (area_id.empty()) { + return snapshot; + } + + auto area_it = area_index_.find(area_id); + if (area_it == area_index_.end() || area_it->second >= areas_.size()) { + return snapshot; // area referenced but missing + } + snapshot.area = areas_[area_it->second]; + return snapshot; +} + +ThreadSafeEntityCache::AppDependenciesSnapshot +ThreadSafeEntityCache::get_app_with_dependencies(const std::string & id) const { + AppDependenciesSnapshot snapshot; + std::shared_lock lock(mutex_); + + auto app_it = app_index_.find(id); + if (app_it == app_index_.end() || app_it->second >= apps_.size()) { + return snapshot; + } + snapshot.app = apps_[app_it->second]; + + snapshot.dependencies.reserve(snapshot.app->depends_on.size()); + for (const auto & dep_id : snapshot.app->depends_on) { + auto dep_it = app_index_.find(dep_id); + if (dep_it != app_index_.end() && dep_it->second < apps_.size()) { + snapshot.dependencies.emplace_back(dep_id, apps_[dep_it->second]); + } else { + snapshot.dependencies.emplace_back(dep_id, std::nullopt); + } + } + return snapshot; +} + std::optional ThreadSafeEntityCache::get_function(const std::string & id) const { std::shared_lock lock(mutex_); auto it = function_index_.find(id); diff --git a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp index 1e391c01..4541456c 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp @@ -972,6 +972,7 @@ void DiscoveryHandlers::handle_get_app(const httplib::Request & req, httplib::Re if (!app.component_id.empty()) { response["is-located-on"] = "/api/v1/components/" + app.component_id; + response["belongs-to"] = base_uri + "/belongs-to"; } if (!app.depends_on.empty()) { @@ -981,6 +982,16 @@ void DiscoveryHandlers::handle_get_app(const httplib::Request & req, httplib::Re using Cap = CapabilityBuilder::Capability; std::vector caps = {Cap::DATA, Cap::OPERATIONS, Cap::CONFIGURATIONS, Cap::FAULTS, Cap::LOGS, Cap::BULK_DATA, Cap::CYCLIC_SUBSCRIPTIONS, Cap::TRIGGERS}; + // Relationship endpoints are gated the same way as the top-level URI keys + // above so the three advertising surfaces (top-level URIs, `_links`, + // `capabilities` array) describe the same set of available collections. + if (!app.component_id.empty()) { + caps.push_back(Cap::IS_LOCATED_ON); + caps.push_back(Cap::BELONGS_TO); + } + if (!app.depends_on.empty()) { + caps.push_back(Cap::DEPENDS_ON); + } if (ctx_.node()->get_script_manager() && ctx_.node()->get_script_manager()->has_backend()) { caps.push_back(Cap::SCRIPTS); } @@ -994,6 +1005,7 @@ void DiscoveryHandlers::handle_get_app(const httplib::Request & req, httplib::Re links.self("/api/v1/apps/" + app.id).collection("/api/v1/apps"); if (!app.component_id.empty()) { links.add("is-located-on", "/api/v1/components/" + app.component_id); + links.add("belongs-to", base_uri + "/belongs-to"); } response["_links"] = links.build(); @@ -1032,35 +1044,41 @@ void DiscoveryHandlers::handle_app_depends_on(const httplib::Request & req, http std::string app_id = req.matches[1]; - auto validation_result = ctx_.validate_entity_id(app_id); - if (!validation_result) { - HandlerContext::send_error(res, 400, ERR_INVALID_PARAMETER, "Invalid app ID", - {{"details", validation_result.error()}, {"app_id", app_id}}); + // validate_entity_for_route forwards the request to the owning peer when + // the app is remote (aggregation setup) - validate_entity_id alone would + // resolve locally only and return 404 for remote apps. + auto entity_opt = ctx_.validate_entity_for_route(req, res, app_id); + if (!entity_opt) { return; } - // Cache-first lookup: EntityCache has merged entities from peers + // Atomic snapshot keeps the app + every dependency resolved in the same + // cache generation; a writer refresh between per-dependency lookups could + // otherwise yield a mix from N different generations. const auto & cache = ctx_.node()->get_thread_safe_cache(); - auto app_opt = cache.get_app(app_id); - if (!app_opt) { + auto snapshot = cache.get_app_with_dependencies(app_id); + if (!snapshot.app) { auto discovery = ctx_.node()->get_discovery_manager(); - app_opt = discovery->get_app(app_id); + snapshot.app = discovery->get_app(app_id); + if (snapshot.app) { + snapshot.dependencies.reserve(snapshot.app->depends_on.size()); + for (const auto & dep_id : snapshot.app->depends_on) { + snapshot.dependencies.emplace_back(dep_id, discovery->get_app(dep_id)); + } + } } - if (!app_opt) { + if (!snapshot.app) { HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "App not found", {{"app_id", app_id}}); return; } - const auto & app = *app_opt; - json items = json::array(); - for (const auto & dep_id : app.depends_on) { + for (const auto & [dep_id, dep_opt] : snapshot.dependencies) { json item; item["id"] = dep_id; item["href"] = "/api/v1/apps/" + dep_id; - auto dep_opt = cache.get_app(dep_id); if (dep_opt) { item["name"] = dep_opt->name.empty() ? dep_id : dep_opt->name; @@ -1098,7 +1116,7 @@ void DiscoveryHandlers::handle_app_depends_on(const httplib::Request & req, http } } -void DiscoveryHandlers::handle_app_is_located_on(const httplib::Request & req, httplib::Response & res) { +void DiscoveryHandlers::handle_app_belongs_to(const httplib::Request & req, httplib::Response & res) { try { if (req.matches.size() < 2) { HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Invalid request"); @@ -1107,40 +1125,138 @@ void DiscoveryHandlers::handle_app_is_located_on(const httplib::Request & req, h std::string app_id = req.matches[1]; - auto validation_result = ctx_.validate_entity_id(app_id); - if (!validation_result) { - HandlerContext::send_error(res, 400, ERR_INVALID_PARAMETER, "Invalid app ID", - {{"details", validation_result.error()}, {"app_id", app_id}}); + // Forward to peer if app is remote (aggregation setup); locally + // resolved entity falls through to the cache/discovery lookup below. + auto entity_opt = ctx_.validate_entity_for_route(req, res, app_id); + if (!entity_opt) { return; } - // Cache-first lookup: EntityCache has merged entities from peers + // Atomic snapshot avoids a mixed-generation view of App -> Component -> + // Area across concurrent cache refreshes. Fall back to the discovery + // manager (no atomic guarantee, but rare path) when the app is not in + // the cache. const auto & cache = ctx_.node()->get_thread_safe_cache(); - auto app_opt = cache.get_app(app_id); - if (!app_opt) { + auto snapshot = cache.get_app_with_links(app_id); + if (!snapshot.app) { auto discovery = ctx_.node()->get_discovery_manager(); - app_opt = discovery->get_app(app_id); + snapshot.app = discovery->get_app(app_id); + if (snapshot.app && !snapshot.app->component_id.empty()) { + snapshot.component = discovery->get_component(snapshot.app->component_id); + if (snapshot.component && !snapshot.component->area.empty()) { + snapshot.area = discovery->get_area(snapshot.component->area); + } + } } - if (!app_opt) { + if (!snapshot.app) { HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "App not found", {{"app_id", app_id}}); return; } json items = json::array(); - const auto & app = *app_opt; + const auto & app = *snapshot.app; if (!app.component_id.empty()) { - auto component_opt = cache.get_component(app.component_id); - if (!component_opt) { - auto discovery = ctx_.node()->get_discovery_manager(); - component_opt = discovery->get_component(app.component_id); + if (!snapshot.component) { + // Mirror handle_app_is_located_on: surface broken parent reference + // with x-medkit.missing=true so HATEOAS clients can distinguish + // 'no parent area' from 'manifest broken, component gone'. + json item; + item["id"] = ""; + item["name"] = ""; + item["href"] = ""; + XMedkit ext; + ext.add("missing", true); + ext.add("unresolved_component", app.component_id); + item["x-medkit"] = ext.build(); + items.push_back(item); + RCLCPP_WARN(HandlerContext::logger(), "App '%s' belongs-to unresolvable: parent component '%s' is unknown", + app_id.c_str(), app.component_id.c_str()); + } else if (!snapshot.component->area.empty()) { + const auto & area_id = snapshot.component->area; + json item; + item["id"] = area_id; + item["href"] = "/api/v1/areas/" + area_id; + + if (snapshot.area) { + item["name"] = snapshot.area->name.empty() ? area_id : snapshot.area->name; + } else { + item["name"] = area_id; + XMedkit ext; + ext.add("missing", true); + item["x-medkit"] = ext.build(); + RCLCPP_WARN(HandlerContext::logger(), "App '%s' belongs to unknown area '%s' (via component '%s')", + app_id.c_str(), area_id.c_str(), app.component_id.c_str()); + } + items.push_back(item); + } + // If component resolves but has no area_id assigned, items stays + // empty - that is a legitimate manifest configuration (component + // without parent area), not a broken reference. + } + + json response; + response["items"] = items; + + XMedkit resp_ext; + resp_ext.add("total_count", items.size()); + response["x-medkit"] = resp_ext.build(); + + json links; + links["self"] = "/api/v1/apps/" + app_id + "/belongs-to"; + links["app"] = "/api/v1/apps/" + app_id; + response["_links"] = links; + + HandlerContext::send_json(res, response); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, "Internal server error", {{"details", e.what()}}); + RCLCPP_ERROR(HandlerContext::logger(), "Error in handle_app_belongs_to: %s", e.what()); + } +} + +void DiscoveryHandlers::handle_app_is_located_on(const httplib::Request & req, httplib::Response & res) { + try { + if (req.matches.size() < 2) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Invalid request"); + return; + } + + std::string app_id = req.matches[1]; + + // Forward to peer if app is remote (aggregation setup). + auto entity_opt = ctx_.validate_entity_for_route(req, res, app_id); + if (!entity_opt) { + return; + } + + // Atomic snapshot keeps app -> component consistent across a concurrent + // cache refresh. Fall back to discovery manager when the app is not in + // the cache. + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto snapshot = cache.get_app_with_links(app_id); + if (!snapshot.app) { + auto discovery = ctx_.node()->get_discovery_manager(); + snapshot.app = discovery->get_app(app_id); + if (snapshot.app && !snapshot.app->component_id.empty()) { + snapshot.component = discovery->get_component(snapshot.app->component_id); } - if (component_opt) { + } + + if (!snapshot.app) { + HandlerContext::send_error(res, 404, ERR_ENTITY_NOT_FOUND, "App not found", {{"app_id", app_id}}); + return; + } + + json items = json::array(); + const auto & app = *snapshot.app; + + if (!app.component_id.empty()) { + if (snapshot.component) { json item; - item["id"] = component_opt->id; - item["name"] = component_opt->name.empty() ? component_opt->id : component_opt->name; - item["href"] = "/api/v1/components/" + component_opt->id; + item["id"] = snapshot.component->id; + item["name"] = snapshot.component->name.empty() ? snapshot.component->id : snapshot.component->name; + item["href"] = "/api/v1/components/" + snapshot.component->id; items.push_back(item); } else { json item; diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index b214bc8f..709cf6c1 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -1131,6 +1131,18 @@ void RESTServer::setup_routes() { .response(200, "Host component(s)", SB::ref("EntityList")) .operation_id("getAppHost"); + reg.get(entity_path + "/belongs-to", + [this](auto & req, auto & res) { + discovery_handlers_->handle_app_belongs_to(req, res); + }) + .tag("Discovery") + .summary("Get app parent area") + .description( + "Returns the area this app belongs to via its parent component, as a 0-or-1 element " + "collection.") + .response(200, "Parent area", SB::ref("EntityList")) + .operation_id("getAppArea"); + reg.get(entity_path + "/depends-on", [this](auto & req, auto & res) { discovery_handlers_->handle_app_depends_on(req, res); diff --git a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp index bbc5a438..c88cd228 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_handlers.cpp @@ -599,9 +599,11 @@ TEST_F(DiscoveryHandlersFixtureTest, GetAppReturnsLinksAndCapabilities) { auto body = parse_json(res); EXPECT_EQ(body["is-located-on"], "/api/v1/components/lidar_unit"); + EXPECT_EQ(body["belongs-to"], "/api/v1/apps/mapper/belongs-to"); EXPECT_EQ(body["depends-on"], "/api/v1/apps/mapper/depends-on"); EXPECT_EQ(body["_links"]["self"], "/api/v1/apps/mapper"); EXPECT_EQ(body["_links"]["is-located-on"], "/api/v1/components/lidar_unit"); + EXPECT_EQ(body["_links"]["belongs-to"], "/api/v1/apps/mapper/belongs-to"); EXPECT_EQ(body["_links"]["depends-on"][0], "/api/v1/apps/planner"); EXPECT_EQ(body["_links"]["depends-on"][1], "/api/v1/apps/ghost_app"); EXPECT_EQ(body["x-medkit"]["source"], "manifest"); @@ -689,6 +691,145 @@ TEST_F(DiscoveryHandlersFixtureTest, AppIsLocatedOnUnknownAppReturns404) { EXPECT_EQ(res.status, 404); } +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToReturnsParentArea) { + auto req = make_request_with_match("/api/v1/apps/mapper/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + auto body = parse_json(res); + ASSERT_EQ(body["items"].size(), 1); + EXPECT_EQ(body["items"][0]["id"], "sensors"); + EXPECT_EQ(body["items"][0]["name"], "Sensors"); + EXPECT_EQ(body["items"][0]["href"], "/api/v1/areas/sensors"); + EXPECT_EQ(body["x-medkit"]["total_count"], 1); + EXPECT_EQ(body["_links"]["self"], "/api/v1/apps/mapper/belongs-to"); + EXPECT_EQ(body["_links"]["app"], "/api/v1/apps/mapper"); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToReturnsEmptyWhenAppHasNoComponent) { + auto req = make_request_with_match("/api/v1/apps/standalone/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + auto body = parse_json(res); + ASSERT_TRUE(body["items"].empty()); + EXPECT_EQ(body["x-medkit"]["total_count"], 0); + EXPECT_EQ(body["_links"]["self"], "/api/v1/apps/standalone/belongs-to"); + EXPECT_EQ(body["_links"]["app"], "/api/v1/apps/standalone"); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToReturnsMissingItemWhenParentComponentUnresolved) { + // Mirror of AppIsLocatedOnReturnsMissingItemWhenHostComponentUnresolved: surface a broken + // parent reference instead of silently returning items=[]. Lets HATEOAS clients tell + // 'app has no parent area' from 'manifest broken, component gone'. + auto & cache = const_cast(suite_node_->get_thread_safe_cache()); + auto apps = cache.get_apps(); + ASSERT_FALSE(apps.empty()); + + bool updated = false; + for (auto & app : apps) { + if (app.id == "mapper") { + app.component_id = "ghost_component"; + updated = true; + break; + } + } + ASSERT_TRUE(updated); + cache.update_apps(apps); + + auto req = make_request_with_match("/api/v1/apps/mapper/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + auto body = parse_json(res); + ASSERT_EQ(body["items"].size(), 1); + EXPECT_EQ(body["items"][0]["x-medkit"]["missing"], true); + EXPECT_EQ(body["items"][0]["x-medkit"]["unresolved_component"], "ghost_component"); + EXPECT_EQ(body["items"][0]["href"], ""); + EXPECT_EQ(body["x-medkit"]["total_count"], 1); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToReturnsEmptyWhenComponentHasNoArea) { + // Strip the parent component's area assignment so the chain App->Component->Area breaks. + auto & cache = const_cast(suite_node_->get_thread_safe_cache()); + auto components = cache.get_components(); + bool updated = false; + for (auto & comp : components) { + if (comp.id == "lidar_unit") { + comp.area.clear(); + updated = true; + break; + } + } + ASSERT_TRUE(updated); + cache.update_components(components); + + auto req = make_request_with_match("/api/v1/apps/mapper/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + auto body = parse_json(res); + ASSERT_TRUE(body["items"].empty()); + EXPECT_EQ(body["x-medkit"]["total_count"], 0); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToReturnsMissingItemWhenAreaUnresolved) { + // Reassign the parent component to an area id that does not exist in the catalogue. + auto & cache = const_cast(suite_node_->get_thread_safe_cache()); + auto components = cache.get_components(); + bool updated = false; + for (auto & comp : components) { + if (comp.id == "lidar_unit") { + comp.area = "ghost_area"; + updated = true; + break; + } + } + ASSERT_TRUE(updated); + cache.update_components(components); + + auto req = make_request_with_match("/api/v1/apps/mapper/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + auto body = parse_json(res); + ASSERT_EQ(body["items"].size(), 1); + EXPECT_EQ(body["items"][0]["id"], "ghost_area"); + EXPECT_EQ(body["items"][0]["name"], "ghost_area"); + EXPECT_EQ(body["items"][0]["href"], "/api/v1/areas/ghost_area"); + EXPECT_EQ(body["items"][0]["x-medkit"]["missing"], true); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersValidationTest, AppBelongsToInvalidIdReturns400) { + auto req = make_request_with_match("/api/v1/apps/bad/id/belongs-to", R"(/api/v1/apps/(.+)/belongs-to)"); + httplib::Response res; + + handlers_.handle_app_belongs_to(req, res); + + EXPECT_EQ(res.status, 400); +} + +// @verifies REQ_INTEROP_106 +TEST_F(DiscoveryHandlersFixtureTest, AppBelongsToUnknownAppReturns404) { + auto req = make_request_with_match("/api/v1/apps/unknown/belongs-to", R"(/api/v1/apps/([^/]+)/belongs-to)"); + httplib::Response res; + + handlers_->handle_app_belongs_to(req, res); + + EXPECT_EQ(res.status, 404); +} + // @verifies REQ_INTEROP_009 TEST_F(DiscoveryHandlersFixtureTest, AppDependsOnReturnsResolvedAndMissingDependencies) { auto req = make_request_with_match("/api/v1/apps/mapper/depends-on", R"(/api/v1/apps/([^/]+)/depends-on)"); diff --git a/src/ros2_medkit_gateway/test/test_discovery_models.cpp b/src/ros2_medkit_gateway/test/test_discovery_models.cpp index 06fe0bb2..0d1c600d 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_models.cpp +++ b/src/ros2_medkit_gateway/test/test_discovery_models.cpp @@ -250,6 +250,33 @@ TEST_F(AppModelTest, ToJson_ExternalWhenTrue) { EXPECT_EQ(j["x-medkit"]["external"], true); } +// @verifies REQ_INTEROP_105 REQ_INTEROP_106 +TEST_F(AppModelTest, ToCapabilities_ContainsRelationshipLinks) { + json j = app_.to_capabilities("http://localhost:8080/api/v1"); + + // Both is-located-on and belongs-to are gated on the same component_id + // condition - handler emits the same set in handle_get_app, so + // to_capabilities must stay in sync for notification and snapshot + // consumers that go through the model. + EXPECT_EQ(j["is-located-on"], "http://localhost:8080/api/v1/components/navigation_server"); + EXPECT_EQ(j["belongs-to"], "http://localhost:8080/api/v1/apps/nav2/belongs-to"); + EXPECT_EQ(j["depends-on"], "http://localhost:8080/api/v1/apps/nav2/depends-on"); +} + +TEST_F(AppModelTest, ToCapabilities_OmitsRelationshipLinksWhenAppIsStandalone) { + App standalone; + standalone.id = "standalone"; + standalone.name = "Standalone"; + standalone.source = "manifest"; + // No component_id, no depends_on - mirrors discovery_handlers.cpp gating. + + json j = standalone.to_capabilities("http://localhost:8080/api/v1"); + + EXPECT_FALSE(j.contains("is-located-on")); + EXPECT_FALSE(j.contains("belongs-to")); + EXPECT_FALSE(j.contains("depends-on")); +} + TEST_F(AppModelTest, ToJson_OmitsEmptyOptionalFields) { App minimal; minimal.id = "test"; diff --git a/src/ros2_medkit_gateway/test/test_entity_resource_model.cpp b/src/ros2_medkit_gateway/test/test_entity_resource_model.cpp index 1f31b13a..73bb6754 100644 --- a/src/ros2_medkit_gateway/test/test_entity_resource_model.cpp +++ b/src/ros2_medkit_gateway/test/test_entity_resource_model.cpp @@ -175,6 +175,11 @@ TEST(EntityCapabilities, AppSupportsIsLocatedOn) { EXPECT_FALSE(caps.supports_resource("hosts")); // Apps don't host anything } +TEST(EntityCapabilities, AppSupportsBelongsTo) { + auto caps = EntityCapabilities::for_type(SovdEntityType::APP); + EXPECT_TRUE(caps.supports_resource("belongs-to")); +} + TEST(EntityCapabilities, FunctionAggregatesCollections) { auto caps = EntityCapabilities::for_type(SovdEntityType::FUNCTION); // ros2_medkit extension: functions support additional collections via aggregation diff --git a/src/ros2_medkit_gateway/test/test_fault_manager.cpp b/src/ros2_medkit_gateway/test/test_fault_manager.cpp index de81aaaf..99569ae1 100644 --- a/src/ros2_medkit_gateway/test/test_fault_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_fault_manager.cpp @@ -158,6 +158,7 @@ TEST_F(FaultManagerTest, GetSnapshotsSuccessWithValidJson) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("MOTOR_OVERHEAT"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_TRUE(result.error_message.empty()); @@ -191,6 +192,7 @@ TEST_F(FaultManagerTest, GetSnapshotsUsesConfiguredFaultManagerNamespace) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("NAMESPACED_FAULT"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_EQ(result.data["fault_code"], "NAMESPACED_FAULT"); @@ -220,6 +222,7 @@ TEST_F(FaultManagerTest, InvalidFaultManagerNamespaceFallsBackToRootServicePath) FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("INVALID_NAMESPACE_FAULT"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_EQ(result.data["service_path"], "/fault_manager/get_snapshots"); @@ -241,6 +244,7 @@ TEST_F(FaultManagerTest, GetSnapshotsSuccessWithTopicFilter) { FaultManager fault_manager(std::make_shared(node_.get())); fault_manager.get_snapshots("TEST_FAULT", "/specific_topic"); + stop_spinning(); EXPECT_EQ(received_topic, "/specific_topic"); } @@ -258,6 +262,7 @@ TEST_F(FaultManagerTest, GetSnapshotsErrorResponse) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("NONEXISTENT_FAULT"); + stop_spinning(); EXPECT_FALSE(result.success); EXPECT_EQ(result.error_message, "Fault not found"); @@ -276,6 +281,7 @@ TEST_F(FaultManagerTest, GetSnapshotsInvalidJsonFallback) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("TEST_FAULT"); + stop_spinning(); EXPECT_TRUE(result.success); // When JSON parsing fails, raw_data should be returned @@ -296,6 +302,7 @@ TEST_F(FaultManagerTest, GetSnapshotsEmptyResponse) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_snapshots("TEST_FAULT"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_TRUE(result.data.is_object()); @@ -334,6 +341,7 @@ TEST_F(FaultManagerTest, GetRosbagSuccess) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_rosbag("TEST_ROSBAG_FAULT"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_TRUE(result.data.contains("file_path")); @@ -367,6 +375,7 @@ TEST_F(FaultManagerTest, GetRosbagUsesConfiguredFaultManagerNamespace) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_rosbag("NAMESPACED_ROSBAG"); + stop_spinning(); EXPECT_TRUE(result.success); EXPECT_EQ(result.data["file_path"], "/tmp/NAMESPACED_ROSBAG"); @@ -580,6 +589,7 @@ TEST_F(FaultManagerTest, GetRosbagNotFound) { FaultManager fault_manager(std::make_shared(node_.get())); auto result = fault_manager.get_rosbag("NONEXISTENT_FAULT"); + stop_spinning(); EXPECT_FALSE(result.success); EXPECT_EQ(result.error_message, "No rosbag file available for fault"); diff --git a/src/ros2_medkit_gateway/test/test_trigger_manager.cpp b/src/ros2_medkit_gateway/test/test_trigger_manager.cpp index 37ef6bbe..104bd661 100644 --- a/src/ros2_medkit_gateway/test/test_trigger_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_trigger_manager.cpp @@ -316,8 +316,16 @@ TEST_F(TriggerManagerTest, LifetimeExpiry) { EXPECT_TRUE(manager_->is_active(created->id)); - // Sleep past expiry with margin for CPU contention during parallel testing - std::this_thread::sleep_for(std::chrono::milliseconds(1500)); + // Poll for expiry: finishes quickly on a fast machine but tolerates + // scheduler stalls under parallel test load. Deadline is well past the + // 1 s lifetime so a busy CI runner does not race the check. + const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(5000); + while (std::chrono::steady_clock::now() < deadline) { + if (!manager_->is_active(created->id)) { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } EXPECT_FALSE(manager_->is_active(created->id)); } diff --git a/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/launch_helpers.py b/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/launch_helpers.py index 612afd8f..78c777da 100644 --- a/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/launch_helpers.py +++ b/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/launch_helpers.py @@ -106,6 +106,14 @@ def create_gateway_node(*, port=DEFAULT_PORT, extra_params=None, coverage=True): output='screen', parameters=[params], additional_env=get_coverage_env() if coverage else {}, + # Default SIGINT->SIGTERM escalation is 5s and SIGTERM->SIGKILL is 5s. + # Under TSan/ASan the gateway shutdown sequence (mdns stop, REST server + # stop, transport teardown, plugin shutdown) easily exceeds 5s on slower + # CI runners, causing launch_testing to escalate to SIGKILL and the + # TestShutdown.test_exit_codes check to report -9. The process still + # shuts down cleanly given enough time, so widen both windows. + sigterm_timeout='30', + sigkill_timeout='15', ) diff --git a/src/ros2_medkit_integration_tests/test/features/test_discovery_gap_fill.test.py b/src/ros2_medkit_integration_tests/test/features/test_discovery_gap_fill.test.py index 48070f53..bda3db88 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_discovery_gap_fill.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_discovery_gap_fill.test.py @@ -92,6 +92,8 @@ def test_only_manifest_areas_present(self): for area_id in area_ids: self.assertIn(area_id, [ 'powertrain', 'chassis', 'body', 'perception', + # HATEOAS edge-case fixture, manifest-only. + 'hateoas-edge-area', ], f"Unexpected area found in top-level listing: {area_id}") def test_only_manifest_components_present(self): @@ -109,6 +111,8 @@ def test_only_manifest_components_present(self): 'brake-ecu', 'brake-pressure-sensor-hw', 'brake-actuator-hw', 'door-sensor-hw', 'light-module', 'lidar-unit', + # HATEOAS edge-case fixture (component without area assignment). + 'hateoas-component-no-area', ] for comp_id in component_ids: self.assertIn( diff --git a/src/ros2_medkit_integration_tests/test/features/test_hateoas.test.py b/src/ros2_medkit_integration_tests/test/features/test_hateoas.test.py index acbec83a..549ee44c 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_hateoas.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_hateoas.test.py @@ -415,21 +415,20 @@ def test_depends_on_apps_nonexistent(self): data = response.json() self.assertIn('error_code', data) - self.assertEqual(data['message'], 'App not found') + # validate_entity_for_route emits the generic Entity not found shape + # so the gateway can also forward unknown apps to peers when running + # in an aggregation cluster. + self.assertEqual(data['message'], 'Entity not found') self.assertIn('parameters', data) - self.assertIn('app_id', data['parameters']) - self.assertEqual(data['parameters'].get('app_id'), 'nonexistent_app') + self.assertEqual(data['parameters'].get('entity_id'), 'nonexistent_app') def test_is_located_on_apps_has_href(self): """GET /apps/{id}/is-located-on returns 0-or-1 component hrefs.""" # @verifies REQ_INTEROP_105 - response = requests.get( - f'{self.BASE_URL}/apps/engine-temp-sensor/is-located-on', - timeout=10 - ) - self.assertEqual(response.status_code, 200) + # poll_endpoint adds retry under sanitizer/CI load - the gateway may + # take a moment to discover entities after launch. + data = self.poll_endpoint('/apps/engine-temp-sensor/is-located-on') - data = response.json() self.assertIn('items', data) self.assertIn('_links', data) self.assertEqual(data['_links']['self'], '/api/v1/apps/engine-temp-sensor/is-located-on') @@ -457,9 +456,87 @@ def test_is_located_on_apps_nonexistent(self): data = response.json() self.assertIn('error_code', data) - self.assertEqual(data['message'], 'App not found') + self.assertEqual(data['message'], 'Entity not found') self.assertIn('parameters', data) - self.assertEqual(data['parameters'].get('app_id'), 'nonexistent_app') + self.assertEqual(data['parameters'].get('entity_id'), 'nonexistent_app') + + def test_belongs_to_apps_returns_parent_area(self): + """GET /apps/{id}/belongs-to returns the parent area via component.""" + # @verifies REQ_INTEROP_106 + # poll_endpoint adds retry under sanitizer/CI load. + data = self.poll_endpoint('/apps/engine-temp-sensor/belongs-to') + + self.assertIn('items', data) + self.assertIn('_links', data) + self.assertEqual(data['_links']['self'], '/api/v1/apps/engine-temp-sensor/belongs-to') + self.assertEqual(data['_links']['app'], '/api/v1/apps/engine-temp-sensor') + self.assertLessEqual(len(data['items']), 1) + + # engine-temp-sensor -> temp-sensor-hw (component) -> engine (area) + self.assertEqual(len(data['items']), 1) + area = data['items'][0] + self.assertEqual(area['id'], 'engine') + self.assertIn('name', area) + self.assertEqual(area['href'], '/api/v1/areas/engine') + + def test_belongs_to_apps_nonexistent(self): + """GET /apps/{id}/belongs-to returns 404 for unknown app.""" + # @verifies REQ_INTEROP_106 + response = requests.get( + f'{self.BASE_URL}/apps/nonexistent_app/belongs-to', + timeout=10 + ) + self.assertEqual(response.status_code, 404) + + data = response.json() + self.assertIn('error_code', data) + self.assertEqual(data['message'], 'Entity not found') + self.assertIn('parameters', data) + self.assertEqual(data['parameters'].get('entity_id'), 'nonexistent_app') + + # ------------------------------------------------------------------ + # HATEOAS edge cases: standalone / broken-ref / missing-area apps. + # Fixtures defined in demo_nodes_manifest.yaml (hateoas-app-*). + # ------------------------------------------------------------------ + + def test_belongs_to_standalone_app_returns_empty(self): + """Standalone app (no parent component) returns 200 with empty items. + + @verifies REQ_INTEROP_106 + """ + data = self.poll_endpoint('/apps/hateoas-app-standalone/belongs-to') + self.assertEqual(data['items'], []) + self.assertEqual(data['x-medkit']['total_count'], 0) + + def test_belongs_to_app_on_area_less_component_returns_empty(self): + """Component without area assignment returns 200 with empty items. + + @verifies REQ_INTEROP_106 + """ + data = self.poll_endpoint('/apps/hateoas-app-on-area-less-component/belongs-to') + self.assertEqual(data['items'], []) + self.assertEqual(data['x-medkit']['total_count'], 0) + + # Note: the 'unresolved parent component' branch on /belongs-to and + # /is-located-on is covered by the unit tests + # AppBelongsToReturnsMissingItemWhenParentComponentUnresolved and + # AppIsLocatedOnReturnsMissingItemWhenHostComponentUnresolved instead - + # the manifest validator rejects an app that points at a non-existent + # component, so we can't seed that scenario as an integration fixture. + + # Note: the 'dangling area reference' branch is covered by unit test + # AppBelongsToReturnsMissingItemWhenAreaUnresolved - manifest validator + # R006 rejects a component that points at a non-existent area, so we + # can't seed that scenario as an integration fixture without disabling + # manifest-driven discovery for the whole suite. + + def test_is_located_on_standalone_app_returns_empty(self): + """Standalone app returns 200 with empty items on /is-located-on too. + + @verifies REQ_INTEROP_105 + """ + data = self.poll_endpoint('/apps/hateoas-app-standalone/is-located-on') + self.assertEqual(data['items'], []) # ------------------------------------------------------------------ # Functions (test_81) diff --git a/src/ros2_medkit_integration_tests/test/features/test_hybrid_suppression.test.py b/src/ros2_medkit_integration_tests/test/features/test_hybrid_suppression.test.py index 891f627b..fe9ceb47 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_hybrid_suppression.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_hybrid_suppression.test.py @@ -58,21 +58,34 @@ MANIFEST_ALL_AREAS = { 'powertrain', 'engine', 'chassis', 'brakes', 'body', 'door', 'front-left-door', 'lights', 'perception', 'lidar', + # HATEOAS edge-case fixture (manifest-only) + 'hateoas-edge-area', } # Only top-level areas appear in GET /areas; subareas are filtered MANIFEST_TOP_LEVEL_AREAS = { 'powertrain', 'chassis', 'body', 'perception', + 'hateoas-edge-area', } MANIFEST_SUBAREAS = MANIFEST_ALL_AREAS - MANIFEST_TOP_LEVEL_AREAS MANIFEST_COMPONENTS = { 'engine-ecu', 'temp-sensor-hw', 'rpm-sensor-hw', 'brake-ecu', 'brake-pressure-sensor-hw', 'brake-actuator-hw', 'door-sensor-hw', 'light-module', 'lidar-unit', + # HATEOAS edge-case fixture (component without area assignment) + 'hateoas-component-no-area', } MANIFEST_APPS = { 'engine-temp-sensor', 'engine-rpm-sensor', 'engine-calibration-service', 'engine-long-calibration', 'brake-pressure-sensor', 'brake-actuator', 'door-status-sensor', 'light-controller', 'lidar-sensor', + # HATEOAS edge-case fixtures (manifest-only, no ros_binding) + 'hateoas-app-standalone', 'hateoas-app-on-area-less-component', +} +# Subset that has a ros_binding and therefore can be is_online once the +# runtime nodes are launched. Manifest-only fixtures (no binding) are +# present in the cache but never reach online state. +MANIFEST_APPS_WITH_BINDING = MANIFEST_APPS - { + 'hateoas-app-standalone', 'hateoas-app-on-area-less-component', } MANIFEST_FUNCTIONS = { 'engine-monitoring', 'engine-calibration', 'brake-system', @@ -260,6 +273,11 @@ def test_manifest_apps_are_online(self): Even with allow_heuristic_apps=false (gap-fill blocking runtime apps from the entity tree), the linker must still receive the unfiltered runtime apps so it can bind manifest apps to live nodes. + + Apps in ``MANIFEST_APPS_WITH_BINDING`` must be online; manifest-only + fixtures (no ros_binding) are exempt - they are present in the cache + but never reach online state because no runtime node was launched + for them. """ # @verifies REQ_INTEROP_003 data = self.poll_endpoint_until( @@ -267,10 +285,13 @@ def test_manifest_apps_are_online(self): lambda d: d if all( a.get('x-medkit', {}).get('is_online', False) for a in d.get('items', []) + if a['id'] in MANIFEST_APPS_WITH_BINDING ) and len(d.get('items', [])) == len(MANIFEST_APPS) else None, timeout=30.0, ) for app in data['items']: + if app['id'] not in MANIFEST_APPS_WITH_BINDING: + continue x_medkit = app.get('x-medkit', {}) is_online = x_medkit.get('is_online', False) self.assertTrue( diff --git a/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_manifest.test.py b/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_manifest.test.py index 7aea73fe..2718ddc9 100644 --- a/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_manifest.test.py +++ b/src/ros2_medkit_integration_tests/test/scenarios/test_scenario_discovery_manifest.test.py @@ -382,6 +382,8 @@ def test_25_no_topic_components_in_manifest_only(self): 'engine-ecu', 'temp-sensor-hw', 'rpm-sensor-hw', 'brake-ecu', 'brake-pressure-sensor-hw', 'brake-actuator-hw', 'door-sensor-hw', 'light-module', 'lidar-unit', + # HATEOAS edge-case fixture (component without area assignment). + 'hateoas-component-no-area', } data = self.get_json('/components') actual_ids = {c['id'] for c in data['items']} diff --git a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/include/ros2_medkit_graph_provider/graph_provider_plugin.hpp b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/include/ros2_medkit_graph_provider/graph_provider_plugin.hpp index 3921fbd2..56f6b517 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/include/ros2_medkit_graph_provider/graph_provider_plugin.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/include/ros2_medkit_graph_provider/graph_provider_plugin.hpp @@ -55,7 +55,7 @@ class GraphProviderPlugin : public GatewayPlugin, public IntrospectionProvider { }; GraphProviderPlugin() = default; - ~GraphProviderPlugin() override; + ~GraphProviderPlugin() noexcept override; std::string name() const override; void configure(const nlohmann::json & config) override; diff --git a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/src/graph_provider_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/src/graph_provider_plugin.cpp index e93adbde..ad3cf0f4 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_graph_provider/src/graph_provider_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_graph_provider/src/graph_provider_plugin.cpp @@ -317,15 +317,28 @@ nlohmann::json build_graph_document_for_apps(const std::string & function_id, } // namespace -GraphProviderPlugin::~GraphProviderPlugin() { - shutdown(); +GraphProviderPlugin::~GraphProviderPlugin() noexcept { + // On Rolling, destroying rclcpp resources (Subscription, Node) after + // rclcpp::shutdown() has invalidated the context can throw + // graph_listener::NodeNotFoundError. An exception escaping a destructor + // calls std::terminate(), so swallow it here. + try { + shutdown(); + } catch (...) { + } } void GraphProviderPlugin::shutdown() { if (shutdown_requested_.exchange(true)) { return; } - diagnostics_sub_.reset(); + // ~rclcpp::Subscription can throw on Rolling when the rclcpp context + // was torn down before us; swallow so plugin_manager shutdown and the + // plugin destructor calling back into us do not abort the process. + try { + diagnostics_sub_.reset(); + } catch (...) { + } } std::string GraphProviderPlugin::name() const {