From 8b9abff8c6d1bbf09bd9ee3d0ca68f0927a18d1f Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:44:46 +0000 Subject: [PATCH 01/15] test: add integration tests for software updates endpoints (RED) Define OTA scenarios: no-plugin 501, CRUD lifecycle, prepare/execute workflow, automated mode, error cases. All tests expected to fail until implementation is complete. --- .../test/features/test_updates.test.py | 510 ++++++++++++++++++ 1 file changed, 510 insertions(+) create mode 100644 src/ros2_medkit_integration_tests/test/features/test_updates.test.py diff --git a/src/ros2_medkit_integration_tests/test/features/test_updates.test.py b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py new file mode 100644 index 00000000..3218b387 --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py @@ -0,0 +1,510 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Integration tests for Software Updates plugin system (Issue #206).""" + +import os +import time +import unittest + +from launch import LaunchDescription +from launch.actions import TimerAction +import launch_ros.actions +import launch_testing +import launch_testing.actions +import requests + +from ros2_medkit_test_utils.constants import API_BASE_PATH +from ros2_medkit_test_utils.gateway_test_case import GatewayTestCase +from ros2_medkit_test_utils.launch_helpers import get_coverage_env + + +PORT_NO_PLUGIN = 8088 +PORT_WITH_PLUGIN = 8089 + + +def _get_test_plugin_path(): + """Get path to test_update_backend.so demo plugin.""" + from ament_index_python.packages import get_package_prefix + + pkg_prefix = get_package_prefix('ros2_medkit_gateway') + return os.path.join( + pkg_prefix, 'lib', 'ros2_medkit_gateway', 'libtest_update_backend.so' + ) + + +def generate_test_description(): + """Launch two gateways: one without plugin (501 mode), one with demo plugin. + + Uses raw Node construction instead of create_gateway_node() because we need + two gateway instances with distinct ROS node names. + """ + coverage_env = get_coverage_env() + plugin_path = _get_test_plugin_path() + + gateway_no_plugin = launch_ros.actions.Node( + package='ros2_medkit_gateway', + executable='gateway_node', + name='gateway_no_plugin', + output='screen', + parameters=[{ + 'server.host': '127.0.0.1', + 'server.port': PORT_NO_PLUGIN, + 'refresh_interval_ms': 1000, + 'updates.enabled': True, + 'updates.backend': 'none', + }], + additional_env=coverage_env, + ) + + gateway_with_plugin = launch_ros.actions.Node( + package='ros2_medkit_gateway', + executable='gateway_node', + name='gateway_with_plugin', + output='screen', + parameters=[{ + 'server.host': '127.0.0.1', + 'server.port': PORT_WITH_PLUGIN, + 'refresh_interval_ms': 1000, + 'updates.enabled': True, + 'updates.backend': 'plugin', + 'updates.plugin_path': plugin_path, + }], + additional_env=coverage_env, + ) + + return LaunchDescription([ + gateway_no_plugin, + gateway_with_plugin, + TimerAction( + period=2.0, + actions=[launch_testing.actions.ReadyToTest()], + ), + ]) + + +class _UpdatesTestMixin: + """Shared helpers for update tests with plugin gateway. + + Must be mixed with GatewayTestCase (provides BASE_URL, poll_endpoint_until). + """ + + def register_package(self, pkg_id, automated=False, origins=None): + """Register a test package and schedule cleanup.""" + pkg = { + 'id': pkg_id, + 'update_name': f'Test {pkg_id}', + 'automated': automated, + 'origins': origins or ['proximity'], + } + r = requests.post(f'{self.BASE_URL}/updates', json=pkg, timeout=5) + self.assertEqual(r.status_code, 201) + self.addCleanup(self._cleanup_package, pkg_id) + return r + + def _cleanup_package(self, pkg_id): + """Best-effort cleanup: wait for idle, then delete.""" + deadline = time.monotonic() + 10.0 + while time.monotonic() < deadline: + r = requests.get( + f'{self.BASE_URL}/updates/{pkg_id}/status', timeout=5 + ) + if r.status_code == 404: + break + if r.status_code == 200 and r.json().get('status') in ( + 'completed', + 'failed', + ): + break + time.sleep(0.2) + requests.delete(f'{self.BASE_URL}/updates/{pkg_id}', timeout=5) + + def poll_update_status(self, pkg_id, target_status, timeout=30.0): + """Poll /updates/{id}/status until target_status reached.""" + return self.poll_endpoint_until( + f'/updates/{pkg_id}/status', + lambda d: d.get('status') == target_status, + timeout=timeout, + ) + + +class TestUpdatesNoPlugin(GatewayTestCase): + """Scenario 1: Without plugin, all /updates endpoints return 501.""" + + BASE_URL = f'http://127.0.0.1:{PORT_NO_PLUGIN}{API_BASE_PATH}' + MIN_EXPECTED_APPS = 0 + + # @verifies REQ_INTEROP_082 + def test_01_list_updates_returns_501(self): + """GET /updates returns 501 when no plugin loaded.""" + r = requests.get(f'{self.BASE_URL}/updates', timeout=5) + self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') + + # @verifies REQ_INTEROP_085 + def test_02_get_update_returns_501(self): + """GET /updates/{id} returns 501 when no plugin loaded.""" + r = requests.get(f'{self.BASE_URL}/updates/some-pkg', timeout=5) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_083 + def test_03_register_update_returns_501(self): + """POST /updates returns 501 when no plugin loaded.""" + r = requests.post( + f'{self.BASE_URL}/updates', + json={ + 'id': 'test', + 'update_name': 'Test', + 'automated': False, + 'origins': ['remote'], + }, + timeout=5, + ) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_091 + def test_04_prepare_returns_501(self): + """PUT /updates/{id}/prepare returns 501 when no plugin loaded.""" + r = requests.put( + f'{self.BASE_URL}/updates/some-pkg/prepare', timeout=5 + ) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_092 + def test_05_execute_returns_501(self): + """PUT /updates/{id}/execute returns 501 when no plugin loaded.""" + r = requests.put( + f'{self.BASE_URL}/updates/some-pkg/execute', timeout=5 + ) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_093 + def test_06_automated_returns_501(self): + """PUT /updates/{id}/automated returns 501 when no plugin loaded.""" + r = requests.put( + f'{self.BASE_URL}/updates/some-pkg/automated', timeout=5 + ) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_094 + def test_07_status_returns_501(self): + """GET /updates/{id}/status returns 501 when no plugin loaded.""" + r = requests.get( + f'{self.BASE_URL}/updates/some-pkg/status', timeout=5 + ) + self.assertEqual(r.status_code, 501) + + # @verifies REQ_INTEROP_084 + def test_08_delete_returns_501(self): + """DELETE /updates/{id} returns 501 when no plugin loaded.""" + r = requests.delete(f'{self.BASE_URL}/updates/some-pkg', timeout=5) + self.assertEqual(r.status_code, 501) + + +class TestUpdatesCRUD(_UpdatesTestMixin, GatewayTestCase): + """Scenario 2: CRUD lifecycle - register, list, get, delete.""" + + BASE_URL = f'http://127.0.0.1:{PORT_WITH_PLUGIN}{API_BASE_PATH}' + MIN_EXPECTED_APPS = 0 + + # @verifies REQ_INTEROP_082 + def test_01_list_updates_empty(self): + """GET /updates returns empty list initially.""" + data = self.get_json('/updates') + self.assertIn('items', data) + self.assertIsInstance(data['items'], list) + + # @verifies REQ_INTEROP_083 + def test_02_register_update(self): + """POST /updates registers a new package.""" + r = self.register_package('test-crud-pkg') + self.assertIn('Location', r.headers) + + # @verifies REQ_INTEROP_082 + def test_03_list_after_register(self): + """GET /updates includes registered package.""" + self.register_package('test-list-pkg', origins=['remote']) + data = self.get_json('/updates') + self.assertIn('test-list-pkg', data['items']) + + # @verifies REQ_INTEROP_085 + def test_04_get_update_metadata(self): + """GET /updates/{id} returns full metadata.""" + pkg = { + 'id': 'test-meta-pkg', + 'update_name': 'Metadata Test', + 'automated': True, + 'origins': ['remote', 'proximity'], + 'duration': 300, + 'size': 1024, + 'updated_components': ['comp_a'], + 'affected_components': ['comp_a', 'comp_b'], + } + requests.post(f'{self.BASE_URL}/updates', json=pkg, timeout=5) + self.addCleanup(self._cleanup_package, 'test-meta-pkg') + + data = self.get_json('/updates/test-meta-pkg') + self.assertEqual(data['id'], 'test-meta-pkg') + self.assertEqual(data['update_name'], 'Metadata Test') + self.assertTrue(data['automated']) + self.assertEqual(data['origins'], ['remote', 'proximity']) + self.assertEqual(data['duration'], 300) + self.assertEqual(data['size'], 1024) + self.assertEqual(data['updated_components'], ['comp_a']) + self.assertEqual(data['affected_components'], ['comp_a', 'comp_b']) + + # @verifies REQ_INTEROP_085 + def test_05_get_nonexistent_returns_404(self): + """GET /updates/{id} returns 404 for unknown package.""" + r = requests.get( + f'{self.BASE_URL}/updates/nonexistent', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + # @verifies REQ_INTEROP_084 + def test_06_delete_update(self): + """DELETE /updates/{id} removes the package.""" + self.register_package('test-delete-pkg') + + r = requests.delete( + f'{self.BASE_URL}/updates/test-delete-pkg', timeout=5 + ) + self.assertEqual(r.status_code, 204) + + # Verify deleted + r = requests.get( + f'{self.BASE_URL}/updates/test-delete-pkg', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + # @verifies REQ_INTEROP_084 + def test_07_delete_nonexistent_returns_404(self): + """DELETE /updates/{id} returns 404 for unknown package.""" + r = requests.delete( + f'{self.BASE_URL}/updates/nonexistent', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + # @verifies REQ_INTEROP_083 + def test_08_register_duplicate_returns_400(self): + """POST /updates returns 400 when ID already exists.""" + self.register_package('test-dup-pkg', origins=['remote']) + r = requests.post( + f'{self.BASE_URL}/updates', + json={ + 'id': 'test-dup-pkg', + 'update_name': 'Dup', + 'automated': False, + 'origins': ['remote'], + }, + timeout=5, + ) + self.assertEqual(r.status_code, 400) + + # @verifies REQ_INTEROP_082 + def test_09_list_with_origin_filter(self): + """GET /updates?origin=remote filters by origin.""" + self.register_package('test-filter-remote', origins=['remote']) + self.register_package('test-filter-prox', origins=['proximity']) + + r = requests.get( + f'{self.BASE_URL}/updates?origin=remote', timeout=5 + ) + self.assertEqual(r.status_code, 200) + items = r.json()['items'] + self.assertIn('test-filter-remote', items) + self.assertNotIn('test-filter-prox', items) + + +class TestUpdatesPrepareExecute(_UpdatesTestMixin, GatewayTestCase): + """Scenario 3: Prepare + execute async workflow with status polling.""" + + BASE_URL = f'http://127.0.0.1:{PORT_WITH_PLUGIN}{API_BASE_PATH}' + MIN_EXPECTED_APPS = 0 + + # @verifies REQ_INTEROP_091 + def test_01_prepare_returns_202(self): + """PUT /updates/{id}/prepare returns 202 Accepted.""" + self.register_package('test-prep-202') + r = requests.put( + f'{self.BASE_URL}/updates/test-prep-202/prepare', timeout=5 + ) + self.assertEqual(r.status_code, 202) + self.assertIn('Location', r.headers) + self.assertIn('/status', r.headers['Location']) + + # @verifies REQ_INTEROP_091 + def test_02_prepare_completes(self): + """Prepare workflow completes successfully with status polling.""" + self.register_package('test-prep-complete') + requests.put( + f'{self.BASE_URL}/updates/test-prep-complete/prepare', timeout=5 + ) + data = self.poll_update_status('test-prep-complete', 'completed') + self.assertEqual(data['status'], 'completed') + + # @verifies REQ_INTEROP_092 + def test_03_execute_after_prepare(self): + """Execute succeeds after prepare completes.""" + self.register_package('test-exec-after-prep') + requests.put( + f'{self.BASE_URL}/updates/test-exec-after-prep/prepare', + timeout=5, + ) + self.poll_update_status('test-exec-after-prep', 'completed') + + r = requests.put( + f'{self.BASE_URL}/updates/test-exec-after-prep/execute', + timeout=5, + ) + self.assertEqual(r.status_code, 202) + data = self.poll_update_status('test-exec-after-prep', 'completed') + self.assertEqual(data['status'], 'completed') + + # @verifies REQ_INTEROP_092 + def test_04_execute_without_prepare_returns_400(self): + """Execute before prepare returns 400.""" + self.register_package('test-exec-no-prep') + r = requests.put( + f'{self.BASE_URL}/updates/test-exec-no-prep/execute', timeout=5 + ) + self.assertEqual(r.status_code, 400) + + # @verifies REQ_INTEROP_094 + def test_05_status_shows_progress(self): + """Status endpoint shows progress during prepare.""" + self.register_package('test-progress') + requests.put( + f'{self.BASE_URL}/updates/test-progress/prepare', timeout=5 + ) + + saw_in_progress = False + deadline = time.monotonic() + 30.0 + while time.monotonic() < deadline: + r = requests.get( + f'{self.BASE_URL}/updates/test-progress/status', timeout=5 + ) + if r.status_code == 200: + data = r.json() + if data.get('status') == 'inProgress': + saw_in_progress = True + if 'progress' in data: + self.assertIsInstance(data['progress'], int) + self.assertGreaterEqual(data['progress'], 0) + self.assertLessEqual(data['progress'], 100) + if data.get('status') == 'completed': + break + time.sleep(0.1) + + self.assertTrue(saw_in_progress, 'Never saw inProgress status') + + # @verifies REQ_INTEROP_094 + def test_06_status_not_found_for_unknown(self): + """GET /status for unknown package returns 404.""" + r = requests.get( + f'{self.BASE_URL}/updates/unknown/status', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + +class TestUpdatesAutomated(_UpdatesTestMixin, GatewayTestCase): + """Scenario 4: Automated (prepare + execute in one step).""" + + BASE_URL = f'http://127.0.0.1:{PORT_WITH_PLUGIN}{API_BASE_PATH}' + MIN_EXPECTED_APPS = 0 + + # @verifies REQ_INTEROP_093 + def test_01_automated_completes(self): + """Automated update runs prepare + execute and completes.""" + self.register_package( + 'test-auto-complete', automated=True, origins=['remote'] + ) + r = requests.put( + f'{self.BASE_URL}/updates/test-auto-complete/automated', + timeout=5, + ) + self.assertEqual(r.status_code, 202) + self.assertIn('Location', r.headers) + + data = self.poll_update_status('test-auto-complete', 'completed') + self.assertEqual(data['status'], 'completed') + + # @verifies REQ_INTEROP_093 + def test_02_automated_on_non_automated_returns_400(self): + """Automated on non-automated package returns 400.""" + self.register_package('test-auto-non', automated=False) + r = requests.put( + f'{self.BASE_URL}/updates/test-auto-non/automated', timeout=5 + ) + self.assertEqual(r.status_code, 400) + + +class TestUpdatesErrorCases(_UpdatesTestMixin, GatewayTestCase): + """Scenario 5: Various error conditions.""" + + BASE_URL = f'http://127.0.0.1:{PORT_WITH_PLUGIN}{API_BASE_PATH}' + MIN_EXPECTED_APPS = 0 + + # @verifies REQ_INTEROP_084 + def test_01_delete_during_prepare_returns_405(self): + """Cannot delete a package while prepare is in progress.""" + self.register_package('test-del-inprog') + requests.put( + f'{self.BASE_URL}/updates/test-del-inprog/prepare', timeout=5 + ) + + r = requests.delete( + f'{self.BASE_URL}/updates/test-del-inprog', timeout=5 + ) + self.assertEqual(r.status_code, 405) + + # @verifies REQ_INTEROP_091 + def test_02_prepare_nonexistent_returns_404(self): + """Cannot prepare a package that doesn't exist.""" + r = requests.put( + f'{self.BASE_URL}/updates/ghost-pkg/prepare', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + # @verifies REQ_INTEROP_092 + def test_03_execute_nonexistent_returns_404(self): + """Cannot execute a package that doesn't exist.""" + r = requests.put( + f'{self.BASE_URL}/updates/ghost-pkg/execute', timeout=5 + ) + self.assertEqual(r.status_code, 404) + + # @verifies REQ_INTEROP_083 + def test_04_register_missing_required_fields(self): + """POST /updates with missing required fields returns 400.""" + r = requests.post( + f'{self.BASE_URL}/updates', + json={'notes': 'missing id and other required fields'}, + timeout=5, + ) + self.assertEqual(r.status_code, 400) + + +@launch_testing.post_shutdown_test() +class TestShutdown(unittest.TestCase): + """Verify both gateways exit cleanly.""" + + def test_exit_codes(self, proc_info): + for info in proc_info: + self.assertIn( + info.returncode, + {0, -2, -15}, + f'Process {info.process_name} exited with {info.returncode}', + ) From 8dd0448032d6b5a9997583458e3bbc6032c5cef6 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:46:01 +0000 Subject: [PATCH 02/15] feat: add UpdateBackend plugin interface, requirements, and error codes Define abstract UpdateBackend class with CRUD + async operations, UpdateProgressReporter for optional progress reporting, new requirements REQ_INTEROP_091-094 for async update endpoints, and vendor-specific error codes for update operations. --- docs/requirements/specs/updates.rst | 27 ++++ .../ros2_medkit_gateway/http/error_codes.hpp | 15 ++ .../updates/update_backend.hpp | 128 ++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp diff --git a/docs/requirements/specs/updates.rst b/docs/requirements/specs/updates.rst index 2c732de8..93ec0435 100644 --- a/docs/requirements/specs/updates.rst +++ b/docs/requirements/specs/updates.rst @@ -29,3 +29,30 @@ Updates The endpoint shall return the details of the addressed software update package. +.. req:: PUT /updates/{id}/prepare + :id: REQ_INTEROP_091 + :status: open + :tags: Updates + + The endpoint shall trigger preparation of the addressed software update package (download, integrity check, dependency validation) and return 202 Accepted with a Location header pointing to the status resource. + +.. req:: PUT /updates/{id}/execute + :id: REQ_INTEROP_092 + :status: open + :tags: Updates + + The endpoint shall begin the actual installation of a previously prepared software update package and return 202 Accepted with a Location header pointing to the status resource. + +.. req:: PUT /updates/{id}/automated + :id: REQ_INTEROP_093 + :status: open + :tags: Updates + + The endpoint shall trigger a fully automated update (prepare + execute) for packages where automated mode is supported and return 202 Accepted with a Location header pointing to the status resource. + +.. req:: GET /updates/{id}/status + :id: REQ_INTEROP_094 + :status: open + :tags: Updates + + The endpoint shall return the current progress and status of an update operation, including status value, progress percentage, and optional sub-progress details. diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/error_codes.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/error_codes.hpp index 97b75a22..4d511d74 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/error_codes.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/error_codes.hpp @@ -99,6 +99,21 @@ constexpr const char * ERR_X_MEDKIT_ROS2_TOPIC_UNAVAILABLE = "x-medkit-ros2-topi /// ROS 2 action server not available constexpr const char * ERR_X_MEDKIT_ROS2_ACTION_UNAVAILABLE = "x-medkit-ros2-action-unavailable"; +/// Software update package not found +constexpr const char * ERR_X_MEDKIT_UPDATE_NOT_FOUND = "x-medkit-update-not-found"; + +/// Duplicate update package ID on registration +constexpr const char * ERR_X_MEDKIT_UPDATE_ALREADY_EXISTS = "x-medkit-update-already-exists"; + +/// Cannot modify/delete update while operation is in progress +constexpr const char * ERR_X_MEDKIT_UPDATE_IN_PROGRESS = "x-medkit-update-in-progress"; + +/// Execute called before prepare completed +constexpr const char * ERR_X_MEDKIT_UPDATE_NOT_PREPARED = "x-medkit-update-not-prepared"; + +/// Automated mode not supported for this package +constexpr const char * ERR_X_MEDKIT_UPDATE_NOT_AUTOMATED = "x-medkit-update-not-automated"; + /** * @brief Check if an error code is a vendor-specific code * @param error_code Error code to check diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp new file mode 100644 index 00000000..174a51f7 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp @@ -0,0 +1,128 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include +#include +#include + +#include +#include + +namespace ros2_medkit_gateway { + +/// Filter criteria for listing update packages +struct UpdateFilter { + std::optional origin; // "remote" | "proximity" + std::optional target_version; // Filter by target version +}; + +/// Status of an update operation +enum class UpdateStatus { Pending, InProgress, Completed, Failed }; + +/// Detailed progress for a sub-step of an update operation +struct UpdateSubProgress { + std::string name; + int progress; // 0-100 +}; + +/// Full status information for an update operation +struct UpdateStatusInfo { + UpdateStatus status = UpdateStatus::Pending; + std::optional progress; // 0-100 + std::optional> sub_progress; // Detailed per-step progress + std::optional error_message; // Set when status == Failed +}; + +/// Internal phase tracking for update lifecycle +enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed }; + +/** + * @brief Thread-safe reporter for update progress. + * + * Passed to UpdateBackend::prepare/execute. The plugin MAY use it to report + * fine-grained progress. If not used, UpdateManager still tracks base status + * (Pending -> InProgress -> Completed/Failed) automatically. + */ +class UpdateProgressReporter { + public: + UpdateProgressReporter(UpdateStatusInfo & status, std::mutex & mutex) : status_(status), mutex_(mutex) { + } + + void set_progress(int percent) { + std::lock_guard lock(mutex_); + status_.progress = percent; + } + + void set_sub_progress(std::vector steps) { + std::lock_guard lock(mutex_); + status_.sub_progress = std::move(steps); + } + + private: + UpdateStatusInfo & status_; + std::mutex & mutex_; +}; + +/** + * @brief Abstract base class for software update backends (plugin interface). + * + * Implementations handle the actual update logic: metadata storage, package + * preparation, and execution. The gateway's UpdateManager handles async + * lifecycle and status tracking. + * + * Plugins can be loaded at compile-time (subclass and pass to UpdateManager) + * or at runtime (.so loaded via dlopen with extern "C" factory function). + * + * For runtime loading, the .so must export: + * extern "C" UpdateBackend* create_update_backend(); + */ +class UpdateBackend { + public: + virtual ~UpdateBackend() = default; + + // ---- CRUD (plugin owns all metadata storage) ---- + + /// List all registered update package IDs, optionally filtered + virtual tl::expected, std::string> list_updates(const UpdateFilter & filter) = 0; + + /// Get full metadata for a specific update package as JSON + virtual tl::expected get_update(const std::string & id) = 0; + + /// Register a new update package from JSON metadata + virtual tl::expected register_update(const nlohmann::json & metadata) = 0; + + /// Delete an update package + virtual tl::expected delete_update(const std::string & id) = 0; + + // ---- Async operations (called in background thread by UpdateManager) ---- + + /// Prepare an update (download, verify, check dependencies). + /// Reporter is optional - plugin may call reporter.set_progress() etc. + virtual tl::expected prepare(const std::string & id, UpdateProgressReporter & reporter) = 0; + + /// Execute an update (install). Only called after prepare succeeds. + virtual tl::expected execute(const std::string & id, UpdateProgressReporter & reporter) = 0; + + /// Check whether a package supports automated mode (prepare + execute) + virtual tl::expected supports_automated(const std::string & id) = 0; + + /// Optional: receive plugin-specific configuration from YAML + virtual void configure(const nlohmann::json & /* config */) { + } +}; + +} // namespace ros2_medkit_gateway From ed226189045e27da23339ac8c32b7dc561b87db8 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:46:50 +0000 Subject: [PATCH 03/15] test: add in-memory demo update backend plugin Stores metadata in memory, simulates prepare/execute with 4 steps (100ms each) and progress reporting. Used as test fixture for integration tests. --- .../test/demo_nodes/test_update_backend.cpp | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp new file mode 100644 index 00000000..93df4d8b --- /dev/null +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp @@ -0,0 +1,178 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include +#include +#include + +#include + +#include "ros2_medkit_gateway/updates/update_backend.hpp" + +using json = nlohmann::json; +using namespace ros2_medkit_gateway; + +/** + * @brief In-memory update backend for integration testing. + * + * Stores package metadata in a map. Simulates prepare/execute with + * short delays (100ms per step, 4 steps) and progress reporting. + */ +class TestUpdateBackend : public UpdateBackend { + public: + tl::expected, std::string> list_updates(const UpdateFilter & filter) override { + std::lock_guard lock(mutex_); + std::vector ids; + for (const auto & [id, meta] : packages_) { + if (filter.origin.has_value()) { + auto origins = meta.value("origins", std::vector{}); + bool found = false; + for (const auto & o : origins) { + if (o == *filter.origin) { + found = true; + break; + } + } + if (!found) { + continue; + } + } + if (filter.target_version.has_value()) { + auto tv = meta.value("target_version", std::string{}); + if (tv != *filter.target_version) { + continue; + } + } + ids.push_back(id); + } + return ids; + } + + tl::expected get_update(const std::string & id) override { + std::lock_guard lock(mutex_); + auto it = packages_.find(id); + if (it == packages_.end()) { + return tl::make_unexpected("Update package '" + id + "' not found"); + } + return it->second; + } + + tl::expected register_update(const json & metadata) override { + if (!metadata.contains("id") || !metadata["id"].is_string()) { + return tl::make_unexpected("Missing required field: id"); + } + if (!metadata.contains("update_name") || !metadata["update_name"].is_string()) { + return tl::make_unexpected("Missing required field: update_name"); + } + if (!metadata.contains("automated") || !metadata["automated"].is_boolean()) { + return tl::make_unexpected("Missing required field: automated"); + } + if (!metadata.contains("origins") || !metadata["origins"].is_array()) { + return tl::make_unexpected("Missing required field: origins"); + } + + std::string id = metadata["id"].get(); + std::lock_guard lock(mutex_); + if (packages_.count(id) > 0) { + return tl::make_unexpected("Update package '" + id + "' already exists"); + } + packages_[id] = metadata; + return {}; + } + + tl::expected delete_update(const std::string & id) override { + std::lock_guard lock(mutex_); + auto it = packages_.find(id); + if (it == packages_.end()) { + return tl::make_unexpected("Update package '" + id + "' not found"); + } + packages_.erase(it); + return {}; + } + + tl::expected prepare(const std::string & id, UpdateProgressReporter & reporter) override { + { + std::lock_guard lock(mutex_); + if (packages_.find(id) == packages_.end()) { + return tl::make_unexpected("Update package '" + id + "' not found"); + } + } + + // Simulate preparation with 4 steps + std::vector steps = {"Downloading package", "Verifying checksum", "Checking dependencies", + "Ready to install"}; + for (int i = 0; i < 4; ++i) { + int pct = (i + 1) * 25; + reporter.set_progress(pct); + std::vector sub; + for (int j = 0; j <= i; ++j) { + sub.push_back({steps[static_cast(j)], 100}); + } + if (i < 3) { + sub.push_back({steps[static_cast(i + 1)], 0}); + } + reporter.set_sub_progress(std::move(sub)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + return {}; + } + + tl::expected execute(const std::string & id, UpdateProgressReporter & reporter) override { + { + std::lock_guard lock(mutex_); + if (packages_.find(id) == packages_.end()) { + return tl::make_unexpected("Update package '" + id + "' not found"); + } + } + + // Simulate execution with 4 steps + std::vector steps = {"Stopping services", "Installing update", "Migrating data", + "Restarting services"}; + for (int i = 0; i < 4; ++i) { + int pct = (i + 1) * 25; + reporter.set_progress(pct); + std::vector sub; + for (int j = 0; j <= i; ++j) { + sub.push_back({steps[static_cast(j)], 100}); + } + if (i < 3) { + sub.push_back({steps[static_cast(i + 1)], 0}); + } + reporter.set_sub_progress(std::move(sub)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + return {}; + } + + tl::expected supports_automated(const std::string & id) override { + std::lock_guard lock(mutex_); + auto it = packages_.find(id); + if (it == packages_.end()) { + return tl::make_unexpected("Update package '" + id + "' not found"); + } + return it->second.value("automated", false); + } + + private: + std::mutex mutex_; + std::unordered_map packages_; +}; + +// Plugin factory function - exported for dlopen/dlsym loading +extern "C" UpdateBackend * create_update_backend() { + return new TestUpdateBackend(); +} From 31639aa777f36c961d2271371cd09f59d525a854 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:48:07 +0000 Subject: [PATCH 04/15] feat: implement UpdateManager with async lifecycle tracking Background threads for prepare/execute/automated. Automatic status tracking (Pending->InProgress->Completed/Failed) with optional progress enrichment via UpdateProgressReporter. Uses unique_ptr for PackageState pointer stability. --- .../updates/update_manager.hpp | 94 ++++++ .../src/updates/update_manager.cpp | 280 ++++++++++++++++++ 2 files changed, 374 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp create mode 100644 src/ros2_medkit_gateway/src/updates/update_manager.cpp diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp new file mode 100644 index 00000000..e7a45f21 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp @@ -0,0 +1,94 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "ros2_medkit_gateway/updates/update_backend.hpp" + +namespace ros2_medkit_gateway { + +/** + * @brief Manages software update lifecycle with pluggable backend. + * + * Handles async operations (prepare/execute) in background threads, + * tracks status automatically, and delegates to UpdateBackend for + * actual work. Without a backend, all operations return errors. + */ +class UpdateManager { + public: + /// Construct with optional backend. Pass nullptr for 501 mode. + explicit UpdateManager(std::unique_ptr backend, void * plugin_handle = nullptr); + ~UpdateManager(); + + // Prevent copy/move (owns async tasks) + UpdateManager(const UpdateManager &) = delete; + UpdateManager & operator=(const UpdateManager &) = delete; + + /// Check if a backend is loaded + bool has_backend() const; + + // ---- CRUD (direct delegation to backend) ---- + tl::expected, std::string> list_updates(const UpdateFilter & filter); + tl::expected get_update(const std::string & id); + tl::expected register_update(const nlohmann::json & metadata); + tl::expected delete_update(const std::string & id); + + // ---- Async operations ---- + tl::expected start_prepare(const std::string & id); + tl::expected start_execute(const std::string & id); + tl::expected start_automated(const std::string & id); + + // ---- Status ---- + tl::expected get_status(const std::string & id); + + private: + std::unique_ptr backend_; + void * plugin_handle_ = nullptr; // dlopen handle, closed in destructor + + struct PackageState { + UpdatePhase phase = UpdatePhase::None; + UpdateStatusInfo status; + std::future active_task; + }; + + // unique_ptr for pointer stability: references to PackageState (held by + // UpdateProgressReporter in background threads) remain valid even if the + // map rehashes due to concurrent insertions. + std::unordered_map> states_; + mutable std::mutex mutex_; + + /// Check if a task is still running + bool is_task_active(const std::string & id) const; + + /// Run prepare in background thread + void run_prepare(const std::string & id); + + /// Run execute in background thread + void run_execute(const std::string & id); + + /// Run automated (prepare + execute) in background thread + void run_automated(const std::string & id); +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp new file mode 100644 index 00000000..aa52cf29 --- /dev/null +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -0,0 +1,280 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ros2_medkit_gateway/updates/update_manager.hpp" + +#include + +namespace ros2_medkit_gateway { + +UpdateManager::UpdateManager(std::unique_ptr backend, void * plugin_handle) + : backend_(std::move(backend)), plugin_handle_(plugin_handle) { +} + +UpdateManager::~UpdateManager() { + // Wait for all active tasks to finish + { + std::lock_guard lock(mutex_); + for (auto & [id, state] : states_) { + if (state->active_task.valid()) { + state->active_task.wait(); + } + } + } + // Destroy backend before closing plugin handle + backend_.reset(); + if (plugin_handle_) { + dlclose(plugin_handle_); + } +} + +bool UpdateManager::has_backend() const { + return backend_ != nullptr; +} + +tl::expected, std::string> UpdateManager::list_updates(const UpdateFilter & filter) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + return backend_->list_updates(filter); +} + +tl::expected UpdateManager::get_update(const std::string & id) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + return backend_->get_update(id); +} + +tl::expected UpdateManager::register_update(const nlohmann::json & metadata) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + return backend_->register_update(metadata); +} + +tl::expected UpdateManager::delete_update(const std::string & id) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + + // Check if an operation is in progress + { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && is_task_active(id)) { + return tl::make_unexpected("Cannot delete update while operation is in progress"); + } + // Wait for completed task before erasing (clean up future) + if (it != states_.end() && it->second->active_task.valid()) { + it->second->active_task.wait(); + } + } + + auto result = backend_->delete_update(id); + if (result) { + std::lock_guard lock(mutex_); + states_.erase(id); + } + return result; +} + +tl::expected UpdateManager::start_prepare(const std::string & id) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + + // Verify package exists + auto pkg = backend_->get_update(id); + if (!pkg) { + return tl::make_unexpected(pkg.error()); + } + + std::lock_guard lock(mutex_); + if (is_task_active(id)) { + return tl::make_unexpected("An operation is already in progress for this package"); + } + + auto & state_ptr = states_[id]; + if (!state_ptr) { + state_ptr = std::make_unique(); + } + state_ptr->phase = UpdatePhase::Preparing; + state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt}; + state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_prepare, this, id); + return {}; +} + +tl::expected UpdateManager::start_execute(const std::string & id) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + + // Verify package exists + auto pkg = backend_->get_update(id); + if (!pkg) { + return tl::make_unexpected(pkg.error()); + } + + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it == states_.end() || !it->second || it->second->phase != UpdatePhase::Prepared) { + return tl::make_unexpected("Package must be prepared before execution"); + } + if (is_task_active(id)) { + return tl::make_unexpected("An operation is already in progress for this package"); + } + + auto & state = *it->second; + state.phase = UpdatePhase::Executing; + state.status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt}; + state.active_task = std::async(std::launch::async, &UpdateManager::run_execute, this, id); + return {}; +} + +tl::expected UpdateManager::start_automated(const std::string & id) { + if (!backend_) { + return tl::make_unexpected("No update backend loaded"); + } + + // Verify package exists and supports automated + auto supported = backend_->supports_automated(id); + if (!supported) { + return tl::make_unexpected(supported.error()); + } + if (!*supported) { + return tl::make_unexpected("Package does not support automated updates"); + } + + std::lock_guard lock(mutex_); + if (is_task_active(id)) { + return tl::make_unexpected("An operation is already in progress for this package"); + } + + auto & state_ptr = states_[id]; + if (!state_ptr) { + state_ptr = std::make_unique(); + } + state_ptr->phase = UpdatePhase::Preparing; + state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt}; + state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_automated, this, id); + return {}; +} + +tl::expected UpdateManager::get_status(const std::string & id) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it == states_.end() || !it->second) { + return tl::make_unexpected("No status available for package '" + id + "'"); + } + return it->second->status; +} + +bool UpdateManager::is_task_active(const std::string & id) const { + auto it = states_.find(id); + if (it == states_.end() || !it->second) { + return false; + } + if (!it->second->active_task.valid()) { + return false; + } + return it->second->active_task.wait_for(std::chrono::seconds(0)) != std::future_status::ready; +} + +void UpdateManager::run_prepare(const std::string & id) { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); // stable pointer (unique_ptr) + state->status.status = UpdateStatus::InProgress; + } + + UpdateProgressReporter reporter(state->status, mutex_); + auto result = backend_->prepare(id, reporter); + + std::lock_guard lock(mutex_); + if (result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Prepared; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = result.error(); + state->phase = UpdatePhase::Failed; + } +} + +void UpdateManager::run_execute(const std::string & id) { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); + state->status.status = UpdateStatus::InProgress; + } + + UpdateProgressReporter reporter(state->status, mutex_); + auto result = backend_->execute(id, reporter); + + std::lock_guard lock(mutex_); + if (result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Executed; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = result.error(); + state->phase = UpdatePhase::Failed; + } +} + +void UpdateManager::run_automated(const std::string & id) { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); + state->status.status = UpdateStatus::InProgress; + } + + // Phase 1: Prepare + UpdateProgressReporter reporter(state->status, mutex_); + auto prep_result = backend_->prepare(id, reporter); + + if (!prep_result) { + std::lock_guard lock(mutex_); + state->status.status = UpdateStatus::Failed; + state->status.error_message = prep_result.error(); + state->phase = UpdatePhase::Failed; + return; + } + + // Phase 2: Execute (reset progress for execute phase) + { + std::lock_guard lock(mutex_); + state->phase = UpdatePhase::Executing; + state->status.progress = std::nullopt; + state->status.sub_progress = std::nullopt; + } + + auto exec_result = backend_->execute(id, reporter); + + std::lock_guard lock(mutex_); + if (exec_result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Executed; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = exec_result.error(); + state->phase = UpdatePhase::Failed; + } +} + +} // namespace ros2_medkit_gateway From 84c880d86ccc90cb296134e74a4942cd8faa41f1 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:48:44 +0000 Subject: [PATCH 05/15] feat: implement UpdatePluginLoader for runtime .so loading Uses dlopen/dlsym to load extern C create_update_backend() factory. Returns backend + handle pair for UpdateManager ownership. --- .../updates/plugin_loader.hpp | 43 +++++++++++++++ .../src/updates/plugin_loader.cpp | 52 +++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp create mode 100644 src/ros2_medkit_gateway/src/updates/plugin_loader.cpp diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp new file mode 100644 index 00000000..38bdde68 --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/plugin_loader.hpp @@ -0,0 +1,43 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include + +#include + +#include "ros2_medkit_gateway/updates/update_backend.hpp" + +namespace ros2_medkit_gateway { + +/// Result of loading a plugin: backend + dlopen handle (for cleanup) +struct PluginLoadResult { + std::unique_ptr backend; + void * handle = nullptr; // dlopen handle, pass to UpdateManager +}; + +/** + * @brief Loads an UpdateBackend plugin from a shared library (.so). + * + * The .so must export: extern "C" UpdateBackend* create_update_backend(); + */ +class UpdatePluginLoader { + public: + /// Load plugin from .so path. Returns backend + handle. + static tl::expected load(const std::string & plugin_path); +}; + +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp b/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp new file mode 100644 index 00000000..5d1f6ffa --- /dev/null +++ b/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp @@ -0,0 +1,52 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ros2_medkit_gateway/updates/plugin_loader.hpp" + +#include + +namespace ros2_medkit_gateway { + +tl::expected UpdatePluginLoader::load(const std::string & plugin_path) { + void * handle = dlopen(plugin_path.c_str(), RTLD_LAZY); + if (!handle) { + return tl::make_unexpected("Failed to load plugin '" + plugin_path + "': " + std::string(dlerror())); + } + + // Clear any existing error + dlerror(); + + using FactoryFn = UpdateBackend * (*)(); + auto factory = reinterpret_cast(dlsym(handle, "create_update_backend")); + + const char * error = dlerror(); + if (error) { + dlclose(handle); + return tl::make_unexpected("Failed to find 'create_update_backend' in '" + plugin_path + + "': " + std::string(error)); + } + + UpdateBackend * raw_backend = factory(); + if (!raw_backend) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_update_backend' returned null in '" + plugin_path + "'"); + } + + PluginLoadResult result; + result.backend = std::unique_ptr(raw_backend); + result.handle = handle; + return result; +} + +} // namespace ros2_medkit_gateway From cb8d39689ccf57535be71462673e4738694a4b16 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:51:32 +0000 Subject: [PATCH 06/15] feat: implement UpdateHandlers for 8 /updates endpoints HTTP handler class for all SOVD software update REST endpoints: - GET /updates (list with origin/target-version filters) - POST /updates (register new package) - GET /updates/{id} (get package details) - DELETE /updates/{id} (remove package) - PUT /updates/{id}/prepare (start prepare phase) - PUT /updates/{id}/execute (start execute phase) - PUT /updates/{id}/automated (prepare+execute in one) - GET /updates/{id}/status (poll async operation status) Includes check_backend() guard (501 if no plugin loaded) and status_to_json() converter for UpdateStatusInfo. --- .../http/handlers/update_handlers.hpp | 56 ++++ .../src/http/handlers/update_handlers.cpp | 267 ++++++++++++++++++ 2 files changed, 323 insertions(+) create mode 100644 src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/update_handlers.hpp create mode 100644 src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/update_handlers.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/update_handlers.hpp new file mode 100644 index 00000000..236cb6ac --- /dev/null +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/update_handlers.hpp @@ -0,0 +1,56 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include + +#include "ros2_medkit_gateway/http/handlers/handler_context.hpp" +#include "ros2_medkit_gateway/updates/update_manager.hpp" + +namespace ros2_medkit_gateway { +namespace handlers { + +/** + * @brief HTTP handlers for software update endpoints (/updates). + * + * All endpoints are server-level (no entity path). Without a loaded + * backend plugin, all endpoints return 501 Not Implemented. + */ +class UpdateHandlers { + public: + UpdateHandlers(HandlerContext & ctx, UpdateManager * update_manager); + + void handle_list_updates(const httplib::Request & req, httplib::Response & res); + void handle_get_update(const httplib::Request & req, httplib::Response & res); + void handle_register_update(const httplib::Request & req, httplib::Response & res); + void handle_delete_update(const httplib::Request & req, httplib::Response & res); + void handle_prepare(const httplib::Request & req, httplib::Response & res); + void handle_execute(const httplib::Request & req, httplib::Response & res); + void handle_automated(const httplib::Request & req, httplib::Response & res); + void handle_get_status(const httplib::Request & req, httplib::Response & res); + + private: + HandlerContext & ctx_; + UpdateManager * update_mgr_; + + /// Check backend loaded, send 501 if not. Returns true if OK. + bool check_backend(httplib::Response & res); + + /// Convert UpdateStatusInfo to JSON + static nlohmann::json status_to_json(const UpdateStatusInfo & status); +}; + +} // namespace handlers +} // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp new file mode 100644 index 00000000..ad2aa163 --- /dev/null +++ b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp @@ -0,0 +1,267 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "ros2_medkit_gateway/http/handlers/update_handlers.hpp" + +#include "ros2_medkit_gateway/http/error_codes.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" + +using json = nlohmann::json; + +namespace ros2_medkit_gateway { +namespace handlers { + +UpdateHandlers::UpdateHandlers(HandlerContext & ctx, UpdateManager * update_manager) + : ctx_(ctx), update_mgr_(update_manager) { +} + +bool UpdateHandlers::check_backend(httplib::Response & res) { + if (!update_mgr_ || !update_mgr_->has_backend()) { + HandlerContext::send_error(res, 501, ERR_NOT_IMPLEMENTED, "Software updates backend not configured"); + return false; + } + return true; +} + +json UpdateHandlers::status_to_json(const UpdateStatusInfo & status) { + json j; + switch (status.status) { + case UpdateStatus::Pending: + j["status"] = "pending"; + break; + case UpdateStatus::InProgress: + j["status"] = "inProgress"; + break; + case UpdateStatus::Completed: + j["status"] = "completed"; + break; + case UpdateStatus::Failed: + j["status"] = "failed"; + break; + } + if (status.progress.has_value()) { + j["progress"] = *status.progress; + } + if (status.sub_progress.has_value()) { + j["sub_progress"] = json::array(); + for (const auto & sp : *status.sub_progress) { + j["sub_progress"].push_back({{"name", sp.name}, {"progress", sp.progress}}); + } + } + if (status.error_message.has_value()) { + json err; + err["error_code"] = ERR_INTERNAL_ERROR; + err["message"] = *status.error_message; + j["error"] = err; + } + return j; +} + +void UpdateHandlers::handle_list_updates(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + UpdateFilter filter; + if (req.has_param("origin")) { + filter.origin = req.get_param_value("origin"); + } + if (req.has_param("target-version")) { + filter.target_version = req.get_param_value("target-version"); + } + + auto result = update_mgr_->list_updates(filter); + if (!result) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error()); + return; + } + + json response; + response["items"] = *result; + HandlerContext::send_json(res, response); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_get_update(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->get_update(id); + if (!result) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + return; + } + HandlerContext::send_json(res, *result); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_register_update(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + json body; + try { + body = json::parse(req.body); + } catch (const json::parse_error &) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Invalid JSON body"); + return; + } + + auto result = update_mgr_->register_update(body); + if (!result) { + // Distinguish "already exists" from "missing fields" + if (result.error().find("already exists") != std::string::npos) { + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_ALREADY_EXISTS, result.error()); + } else { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + } + return; + } + + auto id = body.value("id", std::string{}); + res.status = 201; + res.set_header("Location", api_path("/updates/" + id)); + res.set_header("Content-Type", "application/json"); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_delete_update(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->delete_update(id); + if (!result) { + if (result.error().find("in progress") != std::string::npos) { + HandlerContext::send_error(res, 405, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error()); + } else if (result.error().find("not found") != std::string::npos) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + } else { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error()); + } + return; + } + res.status = 204; + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_prepare(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->start_prepare(id); + if (!result) { + if (result.error().find("not found") != std::string::npos) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + } else { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + } + return; + } + res.status = 202; + res.set_header("Location", api_path("/updates/" + id + "/status")); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_execute(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->start_execute(id); + if (!result) { + if (result.error().find("not found") != std::string::npos) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + } else if (result.error().find("must be prepared") != std::string::npos) { + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_PREPARED, result.error()); + } else { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + } + return; + } + res.status = 202; + res.set_header("Location", api_path("/updates/" + id + "/status")); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_automated(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->start_automated(id); + if (!result) { + if (result.error().find("not found") != std::string::npos) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + } else if (result.error().find("does not support") != std::string::npos) { + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_AUTOMATED, result.error()); + } else { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + } + return; + } + res.status = 202; + res.set_header("Location", api_path("/updates/" + id + "/status")); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +void UpdateHandlers::handle_get_status(const httplib::Request & req, httplib::Response & res) { + if (!check_backend(res)) { + return; + } + + try { + auto id = req.matches[1].str(); + auto result = update_mgr_->get_status(id); + if (!result) { + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + return; + } + HandlerContext::send_json(res, status_to_json(*result)); + } catch (const std::exception & e) { + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); + } +} + +} // namespace handlers +} // namespace ros2_medkit_gateway From e9acf7a6f910192cc743823880de90f0014df041 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:53:38 +0000 Subject: [PATCH 07/15] feat: wire UpdateManager and UpdateHandlers into gateway Load updates config from parameters (updates.enabled, updates.backend, updates.plugin_path), instantiate UpdateManager with plugin backend (or nullptr for 501 mode), register 8 routes in REST server. --- .../ros2_medkit_gateway/gateway_node.hpp | 8 ++++ .../http/handlers/handlers.hpp | 1 + .../ros2_medkit_gateway/http/rest_server.hpp | 1 + src/ros2_medkit_gateway/src/gateway_node.cpp | 31 +++++++++++++++ .../src/http/rest_server.cpp | 38 +++++++++++++++++++ 5 files changed, 79 insertions(+) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index 86cd60c1..b77ab3a1 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -35,6 +35,7 @@ #include "ros2_medkit_gateway/models/thread_safe_entity_cache.hpp" #include "ros2_medkit_gateway/operation_manager.hpp" #include "ros2_medkit_gateway/subscription_manager.hpp" +#include "ros2_medkit_gateway/updates/update_manager.hpp" namespace ros2_medkit_gateway { @@ -93,6 +94,12 @@ class GatewayNode : public rclcpp::Node { */ SubscriptionManager * get_subscription_manager() const; + /** + * @brief Get the UpdateManager instance + * @return Raw pointer to UpdateManager (valid for lifetime of GatewayNode), or nullptr if disabled + */ + UpdateManager * get_update_manager() const; + private: void refresh_cache(); void start_rest_server(); @@ -115,6 +122,7 @@ class GatewayNode : public rclcpp::Node { std::unique_ptr fault_mgr_; std::unique_ptr bulk_data_store_; std::unique_ptr subscription_mgr_; + std::unique_ptr update_mgr_; std::unique_ptr rest_server_; // Cache with thread safety diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp index e64b0872..26c5900c 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp @@ -37,3 +37,4 @@ #include "ros2_medkit_gateway/http/handlers/health_handlers.hpp" #include "ros2_medkit_gateway/http/handlers/operation_handlers.hpp" #include "ros2_medkit_gateway/http/handlers/sse_fault_handler.hpp" +#include "ros2_medkit_gateway/http/handlers/update_handlers.hpp" diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp index 1348ac0c..d9eac3e2 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp @@ -99,6 +99,7 @@ class RESTServer { std::unique_ptr sse_fault_handler_; std::unique_ptr bulkdata_handlers_; std::unique_ptr cyclic_sub_handlers_; + std::unique_ptr update_handlers_; }; } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index 72d4f9af..bd555079 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -16,6 +16,8 @@ #include +#include "ros2_medkit_gateway/updates/plugin_loader.hpp" + using namespace std::chrono_literals; namespace ros2_medkit_gateway { @@ -70,6 +72,11 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { declare_parameter("manifest_path", ""); declare_parameter("manifest_strict_validation", true); + // Software updates plugin parameters + declare_parameter("updates.enabled", false); + declare_parameter("updates.backend", std::string("none")); + declare_parameter("updates.plugin_path", std::string("")); + // Bulk data storage parameters declare_parameter("bulk_data.storage_dir", "/tmp/ros2_medkit_bulk_data"); declare_parameter("bulk_data.max_upload_size", 104857600); // 100MB @@ -316,6 +323,26 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { subscription_mgr_ = std::make_unique(max_subscriptions); RCLCPP_INFO(get_logger(), "Subscription manager: max_subscriptions=%zu", max_subscriptions); + // Initialize update manager + auto updates_enabled = get_parameter("updates.enabled").as_bool(); + if (updates_enabled) { + auto backend_type = get_parameter("updates.backend").as_string(); + if (backend_type == "plugin") { + auto plugin_path = get_parameter("updates.plugin_path").as_string(); + auto load_result = UpdatePluginLoader::load(plugin_path); + if (load_result) { + RCLCPP_INFO(get_logger(), "Loaded update plugin: %s", plugin_path.c_str()); + update_mgr_ = std::make_unique(std::move(load_result->backend), load_result->handle); + } else { + RCLCPP_ERROR(get_logger(), "Failed to load update plugin: %s", load_result.error().c_str()); + update_mgr_ = std::make_unique(nullptr); + } + } else { + // backend: "none" - endpoints exist but return 501 + update_mgr_ = std::make_unique(nullptr); + } + } + // Connect topic sampler to discovery manager for component-topic mapping discovery_mgr_->set_topic_sampler(data_access_mgr_->get_native_sampler()); @@ -390,6 +417,10 @@ SubscriptionManager * GatewayNode::get_subscription_manager() const { return subscription_mgr_.get(); } +UpdateManager * GatewayNode::get_update_manager() const { + return update_mgr_.get(); +} + void GatewayNode::refresh_cache() { RCLCPP_DEBUG(get_logger(), "Refreshing entity cache..."); diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index 6bf49fad..a45ca1da 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -127,6 +127,10 @@ RESTServer::RESTServer(GatewayNode * node, const std::string & host, int port, c cyclic_sub_handlers_ = std::make_unique( *handler_ctx_, *node_->get_subscription_manager(), sse_client_tracker_); + if (node_->get_update_manager()) { + update_handlers_ = std::make_unique(*handler_ctx_, node_->get_update_manager()); + } + // Set up global error handlers for SOVD GenericError compliance setup_global_error_handlers(); // Set up pre-routing handler for CORS and Authentication @@ -983,6 +987,40 @@ void RESTServer::setup_routes() { cyclic_sub_handlers_->handle_delete(req, res); }); + // === Software Updates (server-level endpoints, REQ_INTEROP_082-085, 091-094) === + if (update_handlers_) { + srv->Get(api_path("/updates"), [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_list_updates(req, res); + }); + srv->Post(api_path("/updates"), [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_register_update(req, res); + }); + // Specific routes before generic /{id} + srv->Get((api_path("/updates") + R"(/([^/]+)/status$)"), + [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_get_status(req, res); + }); + srv->Put((api_path("/updates") + R"(/([^/]+)/prepare$)"), + [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_prepare(req, res); + }); + srv->Put((api_path("/updates") + R"(/([^/]+)/execute$)"), + [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_execute(req, res); + }); + srv->Put((api_path("/updates") + R"(/([^/]+)/automated$)"), + [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_automated(req, res); + }); + // Generic /{id} routes last + srv->Get((api_path("/updates") + R"(/([^/]+)$)"), [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_get_update(req, res); + }); + srv->Delete((api_path("/updates") + R"(/([^/]+)$)"), [this](const httplib::Request & req, httplib::Response & res) { + update_handlers_->handle_delete_update(req, res); + }); + } + // Authentication endpoints (REQ_INTEROP_086, REQ_INTEROP_087) // POST /auth/authorize - Authenticate and get tokens (client_credentials grant) srv->Post(api_path("/auth/authorize"), [this](const httplib::Request & req, httplib::Response & res) { From e2fda5c3e24c9ef81cfeb585a01f71f5710dd5e1 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 11:54:31 +0000 Subject: [PATCH 08/15] build: add update module sources, demo plugin, and config Add update_manager.cpp, plugin_loader.cpp, update_handlers.cpp to gateway_lib. Build test_update_backend.so MODULE for integration tests. Link dl library. Add updates config section to params YAML. --- src/ros2_medkit_gateway/CMakeLists.txt | 26 +++++++++++++++++++ .../config/gateway_params.yaml | 21 +++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/ros2_medkit_gateway/CMakeLists.txt b/src/ros2_medkit_gateway/CMakeLists.txt index 17b8476e..984c4424 100644 --- a/src/ros2_medkit_gateway/CMakeLists.txt +++ b/src/ros2_medkit_gateway/CMakeLists.txt @@ -128,6 +128,11 @@ add_library(gateway_lib STATIC src/auth/auth_manager.cpp src/auth/auth_middleware.cpp src/auth/auth_requirement_policy.cpp + # Updates module + src/updates/update_manager.cpp + src/updates/plugin_loader.cpp + # HTTP handlers - updates + src/http/handlers/update_handlers.cpp ) ament_target_dependencies(gateway_lib @@ -151,6 +156,7 @@ target_link_libraries(gateway_lib OpenSSL::Crypto tl::expected jwt-cpp::jwt-cpp + ${CMAKE_DL_LIBS} ) # Gateway node executable @@ -356,6 +362,25 @@ if(BUILD_TESTING) ament_add_gtest(test_cyclic_subscription_handlers test/test_cyclic_subscription_handlers.cpp) target_link_libraries(test_cyclic_subscription_handlers gateway_lib) + # Add update manager tests + ament_add_gtest(test_update_manager test/test_update_manager.cpp) + target_link_libraries(test_update_manager gateway_lib) + + # Demo update backend plugin (.so for integration tests) + add_library(test_update_backend MODULE + test/demo_nodes/test_update_backend.cpp + ) + target_include_directories(test_update_backend PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/include + ) + target_link_libraries(test_update_backend + nlohmann_json::nlohmann_json + tl::expected + ) + install(TARGETS test_update_backend + LIBRARY DESTINATION lib/${PROJECT_NAME} + ) + # Apply coverage flags to test targets if(ENABLE_COVERAGE) set(_test_targets @@ -382,6 +407,7 @@ if(BUILD_TESTING) test_bulkdata_handlers test_subscription_manager test_cyclic_subscription_handlers + test_update_manager ) foreach(_target ${_test_targets}) target_compile_options(${_target} PRIVATE --coverage -O0 -g) diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index 062d0cbc..1fc50522 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -236,6 +236,27 @@ ros2_medkit_gateway: # Example: ["calibration", "firmware", "comlogs", "snapshots"] categories: [] + # Software Updates Plugin System + # Enables /updates endpoints for SOVD-compliant software update management + updates: + # Enable/disable /updates endpoints + # When false, update routes are not registered + # Default: false + enabled: false + + # Backend type + # Options: + # - "none": Endpoints registered but return 501 Not Implemented + # - "plugin": Load update backend from shared library (.so) + # Default: "none" + backend: "none" + + # Path to update backend .so plugin + # Only used when backend: "plugin" + # The .so must export: extern "C" UpdateBackend* create_update_backend(); + # Example: "/opt/ros2_medkit/lib/libmy_update_backend.so" + plugin_path: "" + # Rate Limiting Configuration # Token-bucket-based rate limiting for API requests. # Disabled by default for backward compatibility. From 73fad95bba1e74facc052d33a130a2358503f8ef Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 12:21:07 +0000 Subject: [PATCH 09/15] feat: add UpdateManager unit tests and fix destructor deadlock Unit tests cover CRUD, prepare/execute workflow, automated mode, status polling, and error cases (no backend, not prepared, duplicate, delete during operation). Fix deadlock in ~UpdateManager: collect futures under lock, then wait outside lock to avoid blocking async tasks that also need the mutex. --- .../src/updates/update_manager.cpp | 11 +- .../test/test_update_manager.cpp | 278 ++++++++++++++++++ 2 files changed, 286 insertions(+), 3 deletions(-) create mode 100644 src/ros2_medkit_gateway/test/test_update_manager.cpp diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp index aa52cf29..6f2bdb5e 100644 --- a/src/ros2_medkit_gateway/src/updates/update_manager.cpp +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -23,15 +23,20 @@ UpdateManager::UpdateManager(std::unique_ptr backend, void * plug } UpdateManager::~UpdateManager() { - // Wait for all active tasks to finish + // Collect all valid futures, then wait OUTSIDE the lock to avoid + // deadlock (async tasks also acquire mutex_ during execution). + std::vector> futures; { std::lock_guard lock(mutex_); for (auto & [id, state] : states_) { - if (state->active_task.valid()) { - state->active_task.wait(); + if (state && state->active_task.valid()) { + futures.push_back(std::move(state->active_task)); } } } + for (auto & f : futures) { + f.wait(); + } // Destroy backend before closing plugin handle backend_.reset(); if (plugin_handle_) { diff --git a/src/ros2_medkit_gateway/test/test_update_manager.cpp b/src/ros2_medkit_gateway/test/test_update_manager.cpp new file mode 100644 index 00000000..90867cee --- /dev/null +++ b/src/ros2_medkit_gateway/test/test_update_manager.cpp @@ -0,0 +1,278 @@ +// Copyright 2026 bburda +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include +#include + +#include "ros2_medkit_gateway/updates/update_manager.hpp" + +using namespace ros2_medkit_gateway; +using json = nlohmann::json; + +/// Mock backend for unit testing +class MockUpdateBackend : public UpdateBackend { + public: + tl::expected, std::string> list_updates(const UpdateFilter &) override { + std::lock_guard lock(mutex_); + std::vector ids; + for (const auto & [id, _] : packages_) { + ids.push_back(id); + } + return ids; + } + + tl::expected get_update(const std::string & id) override { + std::lock_guard lock(mutex_); + auto it = packages_.find(id); + if (it == packages_.end()) { + return tl::make_unexpected("not found"); + } + return it->second; + } + + tl::expected register_update(const json & metadata) override { + auto id = metadata.value("id", std::string{}); + if (id.empty()) { + return tl::make_unexpected("missing id"); + } + std::lock_guard lock(mutex_); + if (packages_.count(id)) { + return tl::make_unexpected("already exists"); + } + packages_[id] = metadata; + return {}; + } + + tl::expected delete_update(const std::string & id) override { + std::lock_guard lock(mutex_); + if (packages_.erase(id) == 0) { + return tl::make_unexpected("not found"); + } + return {}; + } + + tl::expected prepare(const std::string &, UpdateProgressReporter & reporter) override { + reporter.set_progress(50); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + reporter.set_progress(100); + return {}; + } + + tl::expected execute(const std::string &, UpdateProgressReporter & reporter) override { + reporter.set_progress(100); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + return {}; + } + + tl::expected supports_automated(const std::string & id) override { + std::lock_guard lock(mutex_); + auto it = packages_.find(id); + if (it == packages_.end()) { + return tl::make_unexpected("not found"); + } + return it->second.value("automated", false); + } + + private: + std::mutex mutex_; + std::unordered_map packages_; +}; + +class UpdateManagerTest : public ::testing::Test { + protected: + void SetUp() override { + auto backend = std::make_unique(); + manager_ = std::make_unique(std::move(backend)); + } + + std::unique_ptr manager_; +}; + +// @verifies REQ_INTEROP_082 +TEST_F(UpdateManagerTest, HasBackend) { + EXPECT_TRUE(manager_->has_backend()); +} + +// @verifies REQ_INTEROP_082 +TEST_F(UpdateManagerTest, NoBackendMode) { + UpdateManager no_backend(nullptr); + EXPECT_FALSE(no_backend.has_backend()); + auto result = no_backend.list_updates({}); + EXPECT_FALSE(result.has_value()); +} + +// @verifies REQ_INTEROP_082 +TEST_F(UpdateManagerTest, RegisterAndList) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + auto reg = manager_->register_update(pkg); + EXPECT_TRUE(reg.has_value()); + + auto list = manager_->list_updates({}); + ASSERT_TRUE(list.has_value()); + EXPECT_EQ(list->size(), 1u); + EXPECT_EQ((*list)[0], "test-pkg"); +} + +// @verifies REQ_INTEROP_085 +TEST_F(UpdateManagerTest, GetUpdate) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + auto result = manager_->get_update("test-pkg"); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ((*result)["id"], "test-pkg"); +} + +// @verifies REQ_INTEROP_085 +TEST_F(UpdateManagerTest, GetUpdateNotFound) { + auto result = manager_->get_update("nonexistent"); + EXPECT_FALSE(result.has_value()); +} + +// @verifies REQ_INTEROP_084 +TEST_F(UpdateManagerTest, DeleteUpdate) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + auto del = manager_->delete_update("test-pkg"); + EXPECT_TRUE(del.has_value()); + + auto get = manager_->get_update("test-pkg"); + EXPECT_FALSE(get.has_value()); +} + +// @verifies REQ_INTEROP_091 +TEST_F(UpdateManagerTest, PrepareAndPollStatus) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + auto prep = manager_->start_prepare("test-pkg"); + ASSERT_TRUE(prep.has_value()); + + // Poll until completed + UpdateStatusInfo status; + for (int i = 0; i < 100; ++i) { + auto s = manager_->get_status("test-pkg"); + ASSERT_TRUE(s.has_value()); + status = *s; + if (status.status == UpdateStatus::Completed) { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + EXPECT_EQ(status.status, UpdateStatus::Completed); +} + +// @verifies REQ_INTEROP_092 +TEST_F(UpdateManagerTest, ExecuteRequiresPrepare) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + auto exec = manager_->start_execute("test-pkg"); + EXPECT_FALSE(exec.has_value()); + EXPECT_NE(exec.error().find("must be prepared"), std::string::npos); +} + +// @verifies REQ_INTEROP_092 +TEST_F(UpdateManagerTest, ExecuteAfterPrepare) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + (void)manager_->start_prepare("test-pkg"); + // Wait for prepare to complete + for (int i = 0; i < 100; ++i) { + auto s = manager_->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Completed) { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + + auto exec = manager_->start_execute("test-pkg"); + ASSERT_TRUE(exec.has_value()); + + // Wait for execute to complete + UpdateStatusInfo status; + for (int i = 0; i < 100; ++i) { + auto s = manager_->get_status("test-pkg"); + ASSERT_TRUE(s.has_value()); + status = *s; + if (status.status == UpdateStatus::Completed) { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + EXPECT_EQ(status.status, UpdateStatus::Completed); +} + +// @verifies REQ_INTEROP_093 +TEST_F(UpdateManagerTest, AutomatedCompletes) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", true}}; + (void)manager_->register_update(pkg); + + auto result = manager_->start_automated("test-pkg"); + ASSERT_TRUE(result.has_value()); + + UpdateStatusInfo status; + for (int i = 0; i < 200; ++i) { + auto s = manager_->get_status("test-pkg"); + ASSERT_TRUE(s.has_value()); + status = *s; + if (status.status == UpdateStatus::Completed) { + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + EXPECT_EQ(status.status, UpdateStatus::Completed); +} + +// @verifies REQ_INTEROP_093 +TEST_F(UpdateManagerTest, AutomatedRejectsNonAutomated) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + auto result = manager_->start_automated("test-pkg"); + EXPECT_FALSE(result.has_value()); + EXPECT_NE(result.error().find("does not support"), std::string::npos); +} + +// @verifies REQ_INTEROP_084 +TEST_F(UpdateManagerTest, DeleteDuringOperationFails) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + (void)manager_->register_update(pkg); + + (void)manager_->start_prepare("test-pkg"); + + auto del = manager_->delete_update("test-pkg"); + EXPECT_FALSE(del.has_value()); + EXPECT_NE(del.error().find("in progress"), std::string::npos); +} + +// @verifies REQ_INTEROP_094 +TEST_F(UpdateManagerTest, StatusNotFoundForUnknown) { + auto result = manager_->get_status("unknown"); + EXPECT_FALSE(result.has_value()); +} + +// @verifies REQ_INTEROP_083 +TEST_F(UpdateManagerTest, DuplicateRegistration) { + json pkg = {{"id", "test-pkg"}, {"update_name", "Test"}, {"automated", false}}; + auto first = manager_->register_update(pkg); + EXPECT_TRUE(first.has_value()); + + auto second = manager_->register_update(pkg); + EXPECT_FALSE(second.has_value()); +} From c609a25f5fa5050ef8ed0a18199f65b60a90ab92 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 15:03:10 +0000 Subject: [PATCH 10/15] fix: harden software updates with typed errors, thread safety, and exception handling Replace string-based error matching with typed UpdateErrorCode enum throughout UpdateManager, handlers, and tests. Fix TOCTOU races by adding Deleting sentinel phase and moving backend validation inside mutex. Wrap async prepare/execute/automated in try-catch for exception safety. Add plugin loader exception safety to prevent dlopen handle leaks. Validate entity IDs in all handlers, fix register response to return JSON body with 201, and change state-conflict errors from 405 to 409. Add 3 new unit tests (backend failure, exception, concurrent reject), integration tests for malformed JSON and target-version filter, and complete REST API docs, changelog, and config reference for software updates. --- README.md | 1 + docs/api/rest.rst | 140 ++++++++ docs/changelog.rst | 8 +- docs/config/server.rst | 37 +++ docs/requirements/specs/updates.rst | 16 +- docs/roadmap.rst | 2 +- .../updates/update_backend.hpp | 16 +- .../updates/update_manager.hpp | 35 +- src/ros2_medkit_gateway/src/gateway_node.cpp | 17 +- .../src/http/handlers/update_handlers.cpp | 135 ++++++-- .../src/updates/plugin_loader.cpp | 11 +- .../src/updates/update_manager.cpp | 312 ++++++++++++------ .../test/test_update_manager.cpp | 145 +++++++- .../test/features/test_updates.test.py | 63 +++- 14 files changed, 761 insertions(+), 177 deletions(-) diff --git a/README.md b/README.md index 2d2a49f0..72223438 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ For more examples, see our [Postman collection](postman/). - **๐Ÿ“Š Health Modeling** โ€” Track health state per entity for fleet-level observability - **๐Ÿ”ง Easy Integration** โ€” Works with existing ROS 2 nodes out of the box (Jazzy, Humble & Rolling) - **๐Ÿ“ฆ Bulk Data Management** โ€” Upload, download, list, and delete bulk data files (calibration, firmware, etc.) +- **๐Ÿ”„ Software Updates** โ€” Manage update packages with async prepare/execute lifecycle and progress tracking via pluggable backends ## ๐Ÿ“– Overview diff --git a/docs/api/rest.rst b/docs/api/rest.rst index ad211ada..43359cfc 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -672,6 +672,145 @@ fault manager and cannot be deleted via this endpoint. - **404 Not Found**: Entity, category, or bulk-data ID not found - **405 Method Not Allowed**: Delete attempted on areas or functions +Software Updates +---------------- + +Manage software update packages with an async prepare/execute lifecycle. +The updates feature requires a backend plugin to be loaded (see :doc:`/config/server`). +Without a plugin, all endpoints return ``501 Not Implemented``. + +``GET /api/v1/updates`` + List all registered update packages. + + **Query Parameters:** + + - ``origin`` (optional): Filter by origin (``remote`` or ``proximity``) + - ``target-version`` (optional): Filter by target version + + **Example Response (200 OK):** + + .. code-block:: json + + { + "items": ["firmware-v2.1", "calibration-update-3"] + } + +``POST /api/v1/updates`` + Register a new update package. + + **Request Body:** + + .. code-block:: json + + { + "id": "firmware-v2.1", + "update_name": "Firmware Update v2.1", + "automated": true, + "origins": ["remote"], + "duration": 600, + "size": 52428800, + "updated_components": ["ecu_main"], + "affected_components": ["ecu_main", "ecu_secondary"] + } + + **Response (201 Created):** + + .. code-block:: json + + { + "id": "firmware-v2.1" + } + + **Response Headers:** + + - ``Location``: ``/api/v1/updates/firmware-v2.1`` + +``GET /api/v1/updates/{id}`` + Get full metadata for a specific update package. + + **Response (200 OK):** + + Returns the JSON metadata as registered. + + - **404 Not Found:** Package does not exist + +``DELETE /api/v1/updates/{id}`` + Delete an update package. + + - **204 No Content:** Package deleted + - **404 Not Found:** Package does not exist + - **409 Conflict:** Operation in progress for this package + +``PUT /api/v1/updates/{id}/prepare`` + Trigger preparation of an update (download, verify, check dependencies). + Runs asynchronously - poll the status endpoint for progress. + + - **202 Accepted:** Preparation started + - **404 Not Found:** Package does not exist + - **409 Conflict:** Operation already in progress + + **Response Headers:** + + - ``Location``: ``/api/v1/updates/{id}/status`` + +``PUT /api/v1/updates/{id}/execute`` + Trigger execution of a prepared update (install). Only succeeds after + prepare has completed. + + - **202 Accepted:** Execution started + - **400 Bad Request:** Package not prepared + - **404 Not Found:** Package does not exist + - **409 Conflict:** Operation already in progress + + **Response Headers:** + + - ``Location``: ``/api/v1/updates/{id}/status`` + +``PUT /api/v1/updates/{id}/automated`` + Trigger automated update (prepare + execute in one step). Only works + for packages that support automated mode. + + - **202 Accepted:** Automated update started + - **400 Bad Request:** Package does not support automated mode + - **404 Not Found:** Package does not exist + - **409 Conflict:** Operation already in progress + + **Response Headers:** + + - ``Location``: ``/api/v1/updates/{id}/status`` + +``GET /api/v1/updates/{id}/status`` + Get the current status and progress of an update operation. + + **Example Response (200 OK):** + + .. code-block:: json + + { + "status": "inProgress", + "progress": 65, + "sub_progress": [ + {"name": "download", "progress": 100}, + {"name": "verify", "progress": 30} + ] + } + + **Status values:** ``pending``, ``inProgress``, ``completed``, ``failed`` + + When ``status`` is ``failed``, an ``error`` object is included: + + .. code-block:: json + + { + "status": "failed", + "error": { + "error_code": "internal-error", + "message": "Download failed: connection timeout" + } + } + + - **404 Not Found:** No status available (package not found or no operation started) + Cyclic Subscriptions -------------------- @@ -940,6 +1079,7 @@ The gateway implements a subset of the SOVD (Service-Oriented Vehicle Diagnostic - Configurations (``/configurations``) - Faults (``/faults``) with ``environment_data`` and SOVD status object - Bulk Data (``/bulk-data``) for binary data downloads (rosbags, logs) +- Software Updates (``/updates``) with async prepare/execute lifecycle - Cyclic Subscriptions (``/cyclic-subscriptions``) with SSE-based periodic data delivery **ros2_medkit Extensions:** diff --git a/docs/changelog.rst b/docs/changelog.rst index 54140c6a..847b56b2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,13 @@ and this project adheres to `Semantic Versioning `_ diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp index 174a51f7..324f1476 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp @@ -48,7 +48,7 @@ struct UpdateStatusInfo { }; /// Internal phase tracking for update lifecycle -enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed }; +enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting }; /** * @brief Thread-safe reporter for update progress. @@ -89,6 +89,20 @@ class UpdateProgressReporter { * * For runtime loading, the .so must export: * extern "C" UpdateBackend* create_update_backend(); + * + * @par Thread Safety + * - CRUD methods (list_updates, get_update, register_update, delete_update) + * are called while UpdateManager holds its mutex. They will not be called + * concurrently with each other, but may overlap with prepare/execute running + * in a background thread. If the backend shares state between CRUD and + * async methods, it must provide its own synchronization. + * - prepare() and execute() run in a background std::async thread. They may + * run concurrently with CRUD calls from the HTTP thread. + * - The UpdateProgressReporter passed to prepare/execute is already + * thread-safe - plugins may call set_progress/set_sub_progress freely. + * - Exceptions thrown from prepare/execute are caught by UpdateManager and + * mapped to Failed status. Plugins should prefer returning + * tl::make_unexpected() for expected errors. */ class UpdateBackend { public: diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp index e7a45f21..16080c98 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp @@ -28,6 +28,25 @@ namespace ros2_medkit_gateway { +/// Error codes for UpdateManager operations - replaces string matching +enum class UpdateErrorCode { + NotFound, // Package does not exist + AlreadyExists, // Duplicate package ID on registration + InProgress, // Operation already running for this package + NotPrepared, // Execute called before prepare completed + NotAutomated, // Package does not support automated mode + InvalidRequest, // Missing fields or invalid input + Deleting, // Package is being deleted + NoBackend, // No update backend loaded + Internal // Unexpected backend error +}; + +/// Typed error for UpdateManager operations +struct UpdateError { + UpdateErrorCode code; + std::string message; +}; + /** * @brief Manages software update lifecycle with pluggable backend. * @@ -49,18 +68,18 @@ class UpdateManager { bool has_backend() const; // ---- CRUD (direct delegation to backend) ---- - tl::expected, std::string> list_updates(const UpdateFilter & filter); - tl::expected get_update(const std::string & id); - tl::expected register_update(const nlohmann::json & metadata); - tl::expected delete_update(const std::string & id); + tl::expected, UpdateError> list_updates(const UpdateFilter & filter); + tl::expected get_update(const std::string & id); + tl::expected register_update(const nlohmann::json & metadata); + tl::expected delete_update(const std::string & id); // ---- Async operations ---- - tl::expected start_prepare(const std::string & id); - tl::expected start_execute(const std::string & id); - tl::expected start_automated(const std::string & id); + tl::expected start_prepare(const std::string & id); + tl::expected start_execute(const std::string & id); + tl::expected start_automated(const std::string & id); // ---- Status ---- - tl::expected get_status(const std::string & id); + tl::expected get_status(const std::string & id); private: std::unique_ptr backend_; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index bd555079..f0f99b37 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -329,13 +329,18 @@ GatewayNode::GatewayNode() : Node("ros2_medkit_gateway") { auto backend_type = get_parameter("updates.backend").as_string(); if (backend_type == "plugin") { auto plugin_path = get_parameter("updates.plugin_path").as_string(); - auto load_result = UpdatePluginLoader::load(plugin_path); - if (load_result) { - RCLCPP_INFO(get_logger(), "Loaded update plugin: %s", plugin_path.c_str()); - update_mgr_ = std::make_unique(std::move(load_result->backend), load_result->handle); - } else { - RCLCPP_ERROR(get_logger(), "Failed to load update plugin: %s", load_result.error().c_str()); + if (plugin_path.empty()) { + RCLCPP_ERROR(get_logger(), "updates.plugin_path is empty - cannot load plugin"); update_mgr_ = std::make_unique(nullptr); + } else { + auto load_result = UpdatePluginLoader::load(plugin_path); + if (load_result) { + RCLCPP_INFO(get_logger(), "Loaded update plugin: %s", plugin_path.c_str()); + update_mgr_ = std::make_unique(std::move(load_result->backend), load_result->handle); + } else { + RCLCPP_ERROR(get_logger(), "Failed to load update plugin: %s", load_result.error().c_str()); + update_mgr_ = std::make_unique(nullptr); + } } } else { // backend: "none" - endpoints exist but return 501 diff --git a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp index ad2aa163..1f96b909 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp @@ -84,7 +84,7 @@ void UpdateHandlers::handle_list_updates(const httplib::Request & req, httplib:: auto result = update_mgr_->list_updates(filter); if (!result) { - HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error()); + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error().message); return; } @@ -103,9 +103,15 @@ void UpdateHandlers::handle_get_update(const httplib::Request & req, httplib::Re try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->get_update(id); if (!result) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); return; } HandlerContext::send_json(res, *result); @@ -130,19 +136,27 @@ void UpdateHandlers::handle_register_update(const httplib::Request & req, httpli auto result = update_mgr_->register_update(body); if (!result) { - // Distinguish "already exists" from "missing fields" - if (result.error().find("already exists") != std::string::npos) { - HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_ALREADY_EXISTS, result.error()); - } else { - HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + switch (result.error().code) { + case UpdateErrorCode::AlreadyExists: + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_ALREADY_EXISTS, result.error().message); + break; + default: + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); + break; } return; } - auto id = body.value("id", std::string{}); + // Validate id field exists before using it for Location header + if (!body.contains("id") || !body["id"].is_string() || body["id"].get().empty()) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Missing required field: id"); + return; + } + auto id = body["id"].get(); + json response = {{"id", id}}; + HandlerContext::send_json(res, response); res.status = 201; res.set_header("Location", api_path("/updates/" + id)); - res.set_header("Content-Type", "application/json"); } catch (const std::exception & e) { HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, e.what()); } @@ -155,14 +169,24 @@ void UpdateHandlers::handle_delete_update(const httplib::Request & req, httplib: try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->delete_update(id); if (!result) { - if (result.error().find("in progress") != std::string::npos) { - HandlerContext::send_error(res, 405, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error()); - } else if (result.error().find("not found") != std::string::npos) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); - } else { - HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error()); + switch (result.error().code) { + case UpdateErrorCode::InProgress: + HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); + break; + case UpdateErrorCode::NotFound: + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); + break; + default: + HandlerContext::send_error(res, 500, ERR_INTERNAL_ERROR, result.error().message); + break; } return; } @@ -179,12 +203,25 @@ void UpdateHandlers::handle_prepare(const httplib::Request & req, httplib::Respo try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->start_prepare(id); if (!result) { - if (result.error().find("not found") != std::string::npos) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); - } else { - HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + switch (result.error().code) { + case UpdateErrorCode::NotFound: + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); + break; + case UpdateErrorCode::InProgress: + case UpdateErrorCode::Deleting: + HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); + break; + default: + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); + break; } return; } @@ -202,14 +239,28 @@ void UpdateHandlers::handle_execute(const httplib::Request & req, httplib::Respo try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->start_execute(id); if (!result) { - if (result.error().find("not found") != std::string::npos) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); - } else if (result.error().find("must be prepared") != std::string::npos) { - HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_PREPARED, result.error()); - } else { - HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + switch (result.error().code) { + case UpdateErrorCode::NotFound: + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); + break; + case UpdateErrorCode::NotPrepared: + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_PREPARED, result.error().message); + break; + case UpdateErrorCode::InProgress: + case UpdateErrorCode::Deleting: + HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); + break; + default: + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); + break; } return; } @@ -227,14 +278,28 @@ void UpdateHandlers::handle_automated(const httplib::Request & req, httplib::Res try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->start_automated(id); if (!result) { - if (result.error().find("not found") != std::string::npos) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); - } else if (result.error().find("does not support") != std::string::npos) { - HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_AUTOMATED, result.error()); - } else { - HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error()); + switch (result.error().code) { + case UpdateErrorCode::NotFound: + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); + break; + case UpdateErrorCode::NotAutomated: + HandlerContext::send_error(res, 400, ERR_X_MEDKIT_UPDATE_NOT_AUTOMATED, result.error().message); + break; + case UpdateErrorCode::InProgress: + case UpdateErrorCode::Deleting: + HandlerContext::send_error(res, 409, ERR_X_MEDKIT_UPDATE_IN_PROGRESS, result.error().message); + break; + default: + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, result.error().message); + break; } return; } @@ -252,9 +317,15 @@ void UpdateHandlers::handle_get_status(const httplib::Request & req, httplib::Re try { auto id = req.matches[1].str(); + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->get_status(id); if (!result) { - HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error()); + HandlerContext::send_error(res, 404, ERR_X_MEDKIT_UPDATE_NOT_FOUND, result.error().message); return; } HandlerContext::send_json(res, status_to_json(*result)); diff --git a/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp b/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp index 5d1f6ffa..f21f561c 100644 --- a/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp +++ b/src/ros2_medkit_gateway/src/updates/plugin_loader.cpp @@ -37,7 +37,16 @@ tl::expected UpdatePluginLoader::load(const std:: "': " + std::string(error)); } - UpdateBackend * raw_backend = factory(); + UpdateBackend * raw_backend = nullptr; + try { + raw_backend = factory(); + } catch (const std::exception & e) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_update_backend' threw exception in '" + plugin_path + "': " + e.what()); + } catch (...) { + dlclose(handle); + return tl::make_unexpected("Factory 'create_update_backend' threw unknown exception in '" + plugin_path + "'"); + } if (!raw_backend) { dlclose(handle); return tl::make_unexpected("Factory 'create_update_backend' returned null in '" + plugin_path + "'"); diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp index 6f2bdb5e..c115a1bb 100644 --- a/src/ros2_medkit_gateway/src/updates/update_manager.cpp +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -48,42 +48,61 @@ bool UpdateManager::has_backend() const { return backend_ != nullptr; } -tl::expected, std::string> UpdateManager::list_updates(const UpdateFilter & filter) { +tl::expected, UpdateError> UpdateManager::list_updates(const UpdateFilter & filter) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } - return backend_->list_updates(filter); + auto result = backend_->list_updates(filter); + if (!result) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, result.error()}); + } + return *result; } -tl::expected UpdateManager::get_update(const std::string & id) { +tl::expected UpdateManager::get_update(const std::string & id) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); + } + auto result = backend_->get_update(id); + if (!result) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, result.error()}); } - return backend_->get_update(id); + return *result; } -tl::expected UpdateManager::register_update(const nlohmann::json & metadata) { +tl::expected UpdateManager::register_update(const nlohmann::json & metadata) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); + } + auto result = backend_->register_update(metadata); + if (!result) { + if (result.error().find("already exists") != std::string::npos) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::AlreadyExists, result.error()}); + } + return tl::make_unexpected(UpdateError{UpdateErrorCode::InvalidRequest, result.error()}); } - return backend_->register_update(metadata); + return {}; } -tl::expected UpdateManager::delete_update(const std::string & id) { +tl::expected UpdateManager::delete_update(const std::string & id) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } - // Check if an operation is in progress { std::lock_guard lock(mutex_); auto it = states_.find(id); - if (it != states_.end() && is_task_active(id)) { - return tl::make_unexpected("Cannot delete update while operation is in progress"); - } - // Wait for completed task before erasing (clean up future) - if (it != states_.end() && it->second->active_task.valid()) { - it->second->active_task.wait(); + if (it != states_.end()) { + if (is_task_active(id)) { + return tl::make_unexpected( + UpdateError{UpdateErrorCode::InProgress, "Cannot delete update while operation is in progress"}); + } + // Wait for completed task before proceeding (clean up future) + if (it->second->active_task.valid()) { + it->second->active_task.wait(); + } + // Mark as deleting so no new operations can start on this package + it->second->phase = UpdatePhase::Deleting; } } @@ -91,54 +110,76 @@ tl::expected UpdateManager::delete_update(const std::string & if (result) { std::lock_guard lock(mutex_); states_.erase(id); + } else { + // Rollback sentinel on failure + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->phase = UpdatePhase::Failed; + } + if (result.error().find("not found") != std::string::npos) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, result.error()}); + } + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, result.error()}); } - return result; + return {}; } -tl::expected UpdateManager::start_prepare(const std::string & id) { +tl::expected UpdateManager::start_prepare(const std::string & id) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } - // Verify package exists + std::lock_guard lock(mutex_); + + // Verify package exists while holding lock to prevent concurrent deletion auto pkg = backend_->get_update(id); if (!pkg) { - return tl::make_unexpected(pkg.error()); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error()}); } - std::lock_guard lock(mutex_); if (is_task_active(id)) { - return tl::make_unexpected("An operation is already in progress for this package"); + return tl::make_unexpected( + UpdateError{UpdateErrorCode::InProgress, "An operation is already in progress for this package"}); } auto & state_ptr = states_[id]; if (!state_ptr) { state_ptr = std::make_unique(); } + + if (state_ptr->phase == UpdatePhase::Deleting) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"}); + } + state_ptr->phase = UpdatePhase::Preparing; state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt}; state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_prepare, this, id); return {}; } -tl::expected UpdateManager::start_execute(const std::string & id) { +tl::expected UpdateManager::start_execute(const std::string & id) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } - // Verify package exists + std::lock_guard lock(mutex_); + auto pkg = backend_->get_update(id); if (!pkg) { - return tl::make_unexpected(pkg.error()); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error()}); } - std::lock_guard lock(mutex_); auto it = states_.find(id); if (it == states_.end() || !it->second || it->second->phase != UpdatePhase::Prepared) { - return tl::make_unexpected("Package must be prepared before execution"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotPrepared, "Package must be prepared before execution"}); + } + if (it->second->phase == UpdatePhase::Deleting) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"}); } if (is_task_active(id)) { - return tl::make_unexpected("An operation is already in progress for this package"); + return tl::make_unexpected( + UpdateError{UpdateErrorCode::InProgress, "An operation is already in progress for this package"}); } auto & state = *it->second; @@ -148,40 +189,47 @@ tl::expected UpdateManager::start_execute(const std::string & return {}; } -tl::expected UpdateManager::start_automated(const std::string & id) { +tl::expected UpdateManager::start_automated(const std::string & id) { if (!backend_) { - return tl::make_unexpected("No update backend loaded"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } - // Verify package exists and supports automated + std::lock_guard lock(mutex_); + auto supported = backend_->supports_automated(id); if (!supported) { - return tl::make_unexpected(supported.error()); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, supported.error()}); } if (!*supported) { - return tl::make_unexpected("Package does not support automated updates"); + return tl::make_unexpected( + UpdateError{UpdateErrorCode::NotAutomated, "Package does not support automated updates"}); } - std::lock_guard lock(mutex_); if (is_task_active(id)) { - return tl::make_unexpected("An operation is already in progress for this package"); + return tl::make_unexpected( + UpdateError{UpdateErrorCode::InProgress, "An operation is already in progress for this package"}); } auto & state_ptr = states_[id]; if (!state_ptr) { state_ptr = std::make_unique(); } + + if (state_ptr->phase == UpdatePhase::Deleting) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"}); + } + state_ptr->phase = UpdatePhase::Preparing; state_ptr->status = UpdateStatusInfo{UpdateStatus::Pending, std::nullopt, std::nullopt, std::nullopt}; state_ptr->active_task = std::async(std::launch::async, &UpdateManager::run_automated, this, id); return {}; } -tl::expected UpdateManager::get_status(const std::string & id) { +tl::expected UpdateManager::get_status(const std::string & id) { std::lock_guard lock(mutex_); auto it = states_.find(id); if (it == states_.end() || !it->second) { - return tl::make_unexpected("No status available for package '" + id + "'"); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, "No status available for package '" + id + "'"}); } return it->second->status; } @@ -198,87 +246,141 @@ bool UpdateManager::is_task_active(const std::string & id) const { } void UpdateManager::run_prepare(const std::string & id) { - PackageState * state = nullptr; - { - std::lock_guard lock(mutex_); - state = states_[id].get(); // stable pointer (unique_ptr) - state->status.status = UpdateStatus::InProgress; - } + try { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); + state->status.status = UpdateStatus::InProgress; + } - UpdateProgressReporter reporter(state->status, mutex_); - auto result = backend_->prepare(id, reporter); + UpdateProgressReporter reporter(state->status, mutex_); + auto result = backend_->prepare(id, reporter); - std::lock_guard lock(mutex_); - if (result) { - state->status.status = UpdateStatus::Completed; - state->phase = UpdatePhase::Prepared; - } else { - state->status.status = UpdateStatus::Failed; - state->status.error_message = result.error(); - state->phase = UpdatePhase::Failed; + std::lock_guard lock(mutex_); + if (result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Prepared; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = result.error(); + state->phase = UpdatePhase::Failed; + } + } catch (const std::exception & e) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = std::string("Exception: ") + e.what(); + it->second->phase = UpdatePhase::Failed; + } + } catch (...) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = "Unknown exception during prepare"; + it->second->phase = UpdatePhase::Failed; + } } } void UpdateManager::run_execute(const std::string & id) { - PackageState * state = nullptr; - { - std::lock_guard lock(mutex_); - state = states_[id].get(); - state->status.status = UpdateStatus::InProgress; - } + try { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); + state->status.status = UpdateStatus::InProgress; + } - UpdateProgressReporter reporter(state->status, mutex_); - auto result = backend_->execute(id, reporter); + UpdateProgressReporter reporter(state->status, mutex_); + auto result = backend_->execute(id, reporter); - std::lock_guard lock(mutex_); - if (result) { - state->status.status = UpdateStatus::Completed; - state->phase = UpdatePhase::Executed; - } else { - state->status.status = UpdateStatus::Failed; - state->status.error_message = result.error(); - state->phase = UpdatePhase::Failed; + std::lock_guard lock(mutex_); + if (result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Executed; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = result.error(); + state->phase = UpdatePhase::Failed; + } + } catch (const std::exception & e) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = std::string("Exception: ") + e.what(); + it->second->phase = UpdatePhase::Failed; + } + } catch (...) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = "Unknown exception during execute"; + it->second->phase = UpdatePhase::Failed; + } } } void UpdateManager::run_automated(const std::string & id) { - PackageState * state = nullptr; - { - std::lock_guard lock(mutex_); - state = states_[id].get(); - state->status.status = UpdateStatus::InProgress; - } + try { + PackageState * state = nullptr; + { + std::lock_guard lock(mutex_); + state = states_[id].get(); + state->status.status = UpdateStatus::InProgress; + } - // Phase 1: Prepare - UpdateProgressReporter reporter(state->status, mutex_); - auto prep_result = backend_->prepare(id, reporter); + // Phase 1: Prepare + UpdateProgressReporter reporter(state->status, mutex_); + auto prep_result = backend_->prepare(id, reporter); - if (!prep_result) { - std::lock_guard lock(mutex_); - state->status.status = UpdateStatus::Failed; - state->status.error_message = prep_result.error(); - state->phase = UpdatePhase::Failed; - return; - } + if (!prep_result) { + std::lock_guard lock(mutex_); + state->status.status = UpdateStatus::Failed; + state->status.error_message = prep_result.error(); + state->phase = UpdatePhase::Failed; + return; + } - // Phase 2: Execute (reset progress for execute phase) - { - std::lock_guard lock(mutex_); - state->phase = UpdatePhase::Executing; - state->status.progress = std::nullopt; - state->status.sub_progress = std::nullopt; - } + // Phase 2: Execute (reset progress for execute phase) + { + std::lock_guard lock(mutex_); + state->phase = UpdatePhase::Executing; + state->status.progress = std::nullopt; + state->status.sub_progress = std::nullopt; + } - auto exec_result = backend_->execute(id, reporter); + auto exec_result = backend_->execute(id, reporter); - std::lock_guard lock(mutex_); - if (exec_result) { - state->status.status = UpdateStatus::Completed; - state->phase = UpdatePhase::Executed; - } else { - state->status.status = UpdateStatus::Failed; - state->status.error_message = exec_result.error(); - state->phase = UpdatePhase::Failed; + std::lock_guard lock(mutex_); + if (exec_result) { + state->status.status = UpdateStatus::Completed; + state->phase = UpdatePhase::Executed; + } else { + state->status.status = UpdateStatus::Failed; + state->status.error_message = exec_result.error(); + state->phase = UpdatePhase::Failed; + } + } catch (const std::exception & e) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = std::string("Exception: ") + e.what(); + it->second->phase = UpdatePhase::Failed; + } + } catch (...) { + std::lock_guard lock(mutex_); + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->status.status = UpdateStatus::Failed; + it->second->status.error_message = "Unknown exception during automated update"; + it->second->phase = UpdatePhase::Failed; + } } } diff --git a/src/ros2_medkit_gateway/test/test_update_manager.cpp b/src/ros2_medkit_gateway/test/test_update_manager.cpp index 90867cee..0ce2987f 100644 --- a/src/ros2_medkit_gateway/test/test_update_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_update_manager.cpp @@ -140,6 +140,7 @@ TEST_F(UpdateManagerTest, GetUpdate) { TEST_F(UpdateManagerTest, GetUpdateNotFound) { auto result = manager_->get_update("nonexistent"); EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code, UpdateErrorCode::NotFound); } // @verifies REQ_INTEROP_084 @@ -163,17 +164,21 @@ TEST_F(UpdateManagerTest, PrepareAndPollStatus) { ASSERT_TRUE(prep.has_value()); // Poll until completed + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); UpdateStatusInfo status; - for (int i = 0; i < 100; ++i) { + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { auto s = manager_->get_status("test-pkg"); - ASSERT_TRUE(s.has_value()); - status = *s; - if (status.status == UpdateStatus::Completed) { - break; + if (s) { + status = *s; + if (status.status == UpdateStatus::Completed) { + found = true; + break; + } } std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - EXPECT_EQ(status.status, UpdateStatus::Completed); + ASSERT_TRUE(found) << "Timed out waiting for status Completed"; } // @verifies REQ_INTEROP_092 @@ -183,7 +188,7 @@ TEST_F(UpdateManagerTest, ExecuteRequiresPrepare) { auto exec = manager_->start_execute("test-pkg"); EXPECT_FALSE(exec.has_value()); - EXPECT_NE(exec.error().find("must be prepared"), std::string::npos); + EXPECT_EQ(exec.error().code, UpdateErrorCode::NotPrepared); } // @verifies REQ_INTEROP_092 @@ -246,7 +251,7 @@ TEST_F(UpdateManagerTest, AutomatedRejectsNonAutomated) { auto result = manager_->start_automated("test-pkg"); EXPECT_FALSE(result.has_value()); - EXPECT_NE(result.error().find("does not support"), std::string::npos); + EXPECT_EQ(result.error().code, UpdateErrorCode::NotAutomated); } // @verifies REQ_INTEROP_084 @@ -258,13 +263,14 @@ TEST_F(UpdateManagerTest, DeleteDuringOperationFails) { auto del = manager_->delete_update("test-pkg"); EXPECT_FALSE(del.has_value()); - EXPECT_NE(del.error().find("in progress"), std::string::npos); + EXPECT_EQ(del.error().code, UpdateErrorCode::InProgress); } // @verifies REQ_INTEROP_094 TEST_F(UpdateManagerTest, StatusNotFoundForUnknown) { auto result = manager_->get_status("unknown"); EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code, UpdateErrorCode::NotFound); } // @verifies REQ_INTEROP_083 @@ -275,4 +281,125 @@ TEST_F(UpdateManagerTest, DuplicateRegistration) { auto second = manager_->register_update(pkg); EXPECT_FALSE(second.has_value()); + EXPECT_EQ(second.error().code, UpdateErrorCode::AlreadyExists); +} + +// @verifies REQ_INTEROP_091 +TEST_F(UpdateManagerTest, ConcurrentPrepareOnSamePackageRejected) { + json pkg = {{"id", "test-pkg"}}; + (void)manager_->register_update(pkg); + + auto result1 = manager_->start_prepare("test-pkg"); + EXPECT_TRUE(result1.has_value()); + + auto result2 = manager_->start_prepare("test-pkg"); + EXPECT_FALSE(result2.has_value()); + EXPECT_EQ(result2.error().code, UpdateErrorCode::InProgress); +} + +/// Mock backend that returns errors from prepare/execute +class MockFailingBackend : public UpdateBackend { + public: + tl::expected, std::string> list_updates(const UpdateFilter & /*filter*/) override { + return tl::make_unexpected("backend error"); + } + tl::expected get_update(const std::string & /*id*/) override { + return json{{"id", "pkg"}}; + } + tl::expected register_update(const json & metadata) override { + auto id = metadata.value("id", std::string{}); + if (id.empty()) { + return tl::make_unexpected("missing id"); + } + return {}; + } + tl::expected delete_update(const std::string & /*id*/) override { + return {}; + } + tl::expected prepare(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + return tl::make_unexpected("download failed"); + } + tl::expected execute(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + return tl::make_unexpected("install failed"); + } + tl::expected supports_automated(const std::string & /*id*/) override { + return true; + } +}; + +/// Mock backend that throws exceptions from prepare/execute +class MockThrowingBackend : public UpdateBackend { + public: + tl::expected, std::string> list_updates(const UpdateFilter & /*filter*/) override { + return std::vector{}; + } + tl::expected get_update(const std::string & /*id*/) override { + return json{{"id", "pkg"}}; + } + tl::expected register_update(const json & /*metadata*/) override { + return {}; + } + tl::expected delete_update(const std::string & /*id*/) override { + return {}; + } + tl::expected prepare(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + throw std::runtime_error("plugin crashed"); + } + tl::expected execute(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + throw std::runtime_error("plugin crashed"); + } + tl::expected supports_automated(const std::string & /*id*/) override { + return true; + } +}; + +// @verifies REQ_INTEROP_091 +TEST(UpdateManagerFailureTest, PrepareFailureSetsFailedStatus) { + auto backend = std::make_unique(); + auto manager = std::make_unique(std::move(backend)); + json pkg = {{"id", "test-pkg"}}; + (void)manager->register_update(pkg); + + auto result = manager->start_prepare("test-pkg"); + ASSERT_TRUE(result.has_value()); + + // Poll until status is no longer InProgress + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + UpdateStatusInfo status; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Failed) { + status = *s; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + EXPECT_EQ(status.status, UpdateStatus::Failed); + ASSERT_TRUE(status.error_message.has_value()); + EXPECT_NE(status.error_message->find("download failed"), std::string::npos); +} + +// @verifies REQ_INTEROP_091 +TEST(UpdateManagerFailureTest, PrepareExceptionSetsFailedStatus) { + auto backend = std::make_unique(); + auto manager = std::make_unique(std::move(backend)); + json pkg = {{"id", "test-pkg"}}; + (void)manager->register_update(pkg); + + auto result = manager->start_prepare("test-pkg"); + ASSERT_TRUE(result.has_value()); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + UpdateStatusInfo status; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Failed) { + status = *s; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + EXPECT_EQ(status.status, UpdateStatus::Failed); + ASSERT_TRUE(status.error_message.has_value()); + EXPECT_NE(status.error_message->find("Exception"), std::string::npos); } diff --git a/src/ros2_medkit_integration_tests/test/features/test_updates.test.py b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py index 3218b387..01163ac1 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_updates.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_updates.test.py @@ -26,7 +26,7 @@ import launch_testing.actions import requests -from ros2_medkit_test_utils.constants import API_BASE_PATH +from ros2_medkit_test_utils.constants import ALLOWED_EXIT_CODES, API_BASE_PATH from ros2_medkit_test_utils.gateway_test_case import GatewayTestCase from ros2_medkit_test_utils.launch_helpers import get_coverage_env @@ -135,7 +135,7 @@ def poll_update_status(self, pkg_id, target_status, timeout=30.0): """Poll /updates/{id}/status until target_status reached.""" return self.poll_endpoint_until( f'/updates/{pkg_id}/status', - lambda d: d.get('status') == target_status, + lambda d: d if d.get('status') == target_status else None, timeout=timeout, ) @@ -158,6 +158,7 @@ def test_02_get_update_returns_501(self): """GET /updates/{id} returns 501 when no plugin loaded.""" r = requests.get(f'{self.BASE_URL}/updates/some-pkg', timeout=5) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_083 def test_03_register_update_returns_501(self): @@ -173,6 +174,7 @@ def test_03_register_update_returns_501(self): timeout=5, ) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_091 def test_04_prepare_returns_501(self): @@ -181,6 +183,7 @@ def test_04_prepare_returns_501(self): f'{self.BASE_URL}/updates/some-pkg/prepare', timeout=5 ) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_092 def test_05_execute_returns_501(self): @@ -189,6 +192,7 @@ def test_05_execute_returns_501(self): f'{self.BASE_URL}/updates/some-pkg/execute', timeout=5 ) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_093 def test_06_automated_returns_501(self): @@ -197,6 +201,7 @@ def test_06_automated_returns_501(self): f'{self.BASE_URL}/updates/some-pkg/automated', timeout=5 ) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_094 def test_07_status_returns_501(self): @@ -205,12 +210,14 @@ def test_07_status_returns_501(self): f'{self.BASE_URL}/updates/some-pkg/status', timeout=5 ) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') # @verifies REQ_INTEROP_084 def test_08_delete_returns_501(self): """DELETE /updates/{id} returns 501 when no plugin loaded.""" r = requests.delete(f'{self.BASE_URL}/updates/some-pkg', timeout=5) self.assertEqual(r.status_code, 501) + self.assertEqual(r.json()['error_code'], 'not-implemented') class TestUpdatesCRUD(_UpdatesTestMixin, GatewayTestCase): @@ -327,6 +334,19 @@ def test_09_list_with_origin_filter(self): self.assertIn('test-filter-remote', items) self.assertNotIn('test-filter-prox', items) + # @verifies REQ_INTEROP_082 + def test_10_list_with_target_version_filter(self): + """GET /updates?target-version=X filters by target version.""" + self.register_package('ver-pkg-1') + self.register_package('ver-pkg-2') + r = requests.get( + f'{self.BASE_URL}/updates?target-version=nonexistent', + timeout=5, + ) + self.assertEqual(r.status_code, 200) + data = r.json() + self.assertIsInstance(data['items'], list) + class TestUpdatesPrepareExecute(_UpdatesTestMixin, GatewayTestCase): """Scenario 3: Prepare + execute async workflow with status polling.""" @@ -404,6 +424,14 @@ def test_05_status_shows_progress(self): self.assertIsInstance(data['progress'], int) self.assertGreaterEqual(data['progress'], 0) self.assertLessEqual(data['progress'], 100) + if 'sub_progress' in data: + self.assertIsInstance(data['sub_progress'], list) + for sp in data['sub_progress']: + self.assertIn('name', sp) + self.assertIn('progress', sp) + self.assertIsInstance(sp['progress'], int) + self.assertGreaterEqual(sp['progress'], 0) + self.assertLessEqual(sp['progress'], 100) if data.get('status') == 'completed': break time.sleep(0.1) @@ -458,7 +486,7 @@ class TestUpdatesErrorCases(_UpdatesTestMixin, GatewayTestCase): MIN_EXPECTED_APPS = 0 # @verifies REQ_INTEROP_084 - def test_01_delete_during_prepare_returns_405(self): + def test_01_delete_during_prepare_returns_409(self): """Cannot delete a package while prepare is in progress.""" self.register_package('test-del-inprog') requests.put( @@ -468,7 +496,10 @@ def test_01_delete_during_prepare_returns_405(self): r = requests.delete( f'{self.BASE_URL}/updates/test-del-inprog', timeout=5 ) - self.assertEqual(r.status_code, 405) + self.assertEqual(r.status_code, 409) + data = r.json() + self.assertEqual(data['error_code'], 'vendor-error') + self.assertEqual(data['vendor_code'], 'x-medkit-update-in-progress') # @verifies REQ_INTEROP_091 def test_02_prepare_nonexistent_returns_404(self): @@ -477,6 +508,9 @@ def test_02_prepare_nonexistent_returns_404(self): f'{self.BASE_URL}/updates/ghost-pkg/prepare', timeout=5 ) self.assertEqual(r.status_code, 404) + data = r.json() + self.assertEqual(data['error_code'], 'vendor-error') + self.assertEqual(data['vendor_code'], 'x-medkit-update-not-found') # @verifies REQ_INTEROP_092 def test_03_execute_nonexistent_returns_404(self): @@ -485,6 +519,9 @@ def test_03_execute_nonexistent_returns_404(self): f'{self.BASE_URL}/updates/ghost-pkg/execute', timeout=5 ) self.assertEqual(r.status_code, 404) + data = r.json() + self.assertEqual(data['error_code'], 'vendor-error') + self.assertEqual(data['vendor_code'], 'x-medkit-update-not-found') # @verifies REQ_INTEROP_083 def test_04_register_missing_required_fields(self): @@ -495,6 +532,22 @@ def test_04_register_missing_required_fields(self): timeout=5, ) self.assertEqual(r.status_code, 400) + data = r.json() + self.assertIn('error_code', data) + self.assertEqual(data['error_code'], 'invalid-request') + + # @verifies REQ_INTEROP_083 + def test_05_register_malformed_json(self): + """POST /updates with broken JSON returns 400.""" + r = requests.post( + f'{self.BASE_URL}/updates', + data='not{valid json', + headers={'Content-Type': 'application/json'}, + timeout=5, + ) + self.assertEqual(r.status_code, 400) + data = r.json() + self.assertIn('error_code', data) @launch_testing.post_shutdown_test() @@ -505,6 +558,6 @@ def test_exit_codes(self, proc_info): for info in proc_info: self.assertIn( info.returncode, - {0, -2, -15}, + ALLOWED_EXIT_CODES, f'Process {info.process_name} exited with {info.returncode}', ) From b25cefaaef8830588c0846b5342660819d31270a Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 15:22:38 +0000 Subject: [PATCH 11/15] docs: replace features bullet list with SOVD coverage table in README --- README.md | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 72223438..9b8acad4 100644 --- a/README.md +++ b/README.md @@ -49,14 +49,23 @@ For more examples, see our [Postman collection](postman/). ## โœจ Features -- **๐Ÿ” Runtime Discovery** โ€” Automatically discover what is actually running on your robot -- **๐Ÿ—๏ธ Entity Tree Model** โ€” Organize diagnostics as Area โ†’ Component โ†’ Function โ†’ App -- **๐Ÿ”— SOVD Compatible** โ€” Align with Service-Oriented Vehicle Diagnostics standards -- **๐ŸŒ REST API Gateway** โ€” HTTP interface for integration with external tools and UIs -- **๐Ÿ“Š Health Modeling** โ€” Track health state per entity for fleet-level observability -- **๐Ÿ”ง Easy Integration** โ€” Works with existing ROS 2 nodes out of the box (Jazzy, Humble & Rolling) -- **๐Ÿ“ฆ Bulk Data Management** โ€” Upload, download, list, and delete bulk data files (calibration, firmware, etc.) -- **๐Ÿ”„ Software Updates** โ€” Manage update packages with async prepare/execute lifecycle and progress tracking via pluggable backends +| Feature | Status | Description | +|---------|--------|-------------| +| ๐Ÿ” Discovery | **Available** | Automatically discover running nodes, topics, services, and actions | +| ๐Ÿ“Š Data | **Available** | Read and write topic data via REST | +| โš™๏ธ Operations | **Available** | Call services and actions with execution tracking | +| ๐Ÿ”ง Configurations | **Available** | Read, write, and reset node parameters | +| ๐Ÿšจ Faults | **Available** | Query, inspect, and clear faults with environment data and snapshots | +| ๐Ÿ“ฆ Bulk Data | **Available** | Upload, download, and manage files (calibration, firmware, rosbags) | +| ๐Ÿ“ก Subscriptions | **Available** | Stream live data and fault events via SSE | +| ๐Ÿ”„ Software Updates | **Available** | Async prepare/execute lifecycle with pluggable backends | +| ๐Ÿ”’ Authentication | **Available** | JWT-based RBAC (viewer, operator, configurator, admin) | +| ๐Ÿ“‹ Logs | Planned | Log sources, entries, and configuration | +| ๐Ÿ” Entity Lifecycle | Planned | Start, restart, shutdown control | +| ๐Ÿ” Modes & Locking | Planned | Target mode control and resource locking | +| ๐Ÿ“ Scripts | Planned | Diagnostic script upload and execution | +| ๐Ÿงน Clear Data | Planned | Clear cached and learned diagnostic data | +| ๐Ÿ“ž Communication Logs | Planned | Protocol-level communication logging | ## ๐Ÿ“– Overview From 372fe5248c7b2a5c4110678fbe9b35ff5c1bcdcb Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 15:40:29 +0000 Subject: [PATCH 12/15] fix: use verified status instead of closed for updates requirements --- docs/requirements/specs/updates.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/requirements/specs/updates.rst b/docs/requirements/specs/updates.rst index 9f5e4e89..2a774065 100644 --- a/docs/requirements/specs/updates.rst +++ b/docs/requirements/specs/updates.rst @@ -3,56 +3,56 @@ Updates .. req:: GET /updates :id: REQ_INTEROP_082 - :status: closed + :status: verified :tags: Updates The endpoint shall list software update packages known to the SOVD server. .. req:: POST /updates :id: REQ_INTEROP_083 - :status: closed + :status: verified :tags: Updates The endpoint shall register a new software update package at the SOVD server using the provided parameters. .. req:: DELETE /updates/{id} :id: REQ_INTEROP_084 - :status: closed + :status: verified :tags: Updates The endpoint shall delete the addressed software update package from the SOVD server, if permitted (e.g. if no update is currently in execution). .. req:: GET /updates/{id} :id: REQ_INTEROP_085 - :status: closed + :status: verified :tags: Updates The endpoint shall return the details of the addressed software update package. .. req:: PUT /updates/{id}/prepare :id: REQ_INTEROP_091 - :status: closed + :status: verified :tags: Updates The endpoint shall trigger preparation of the addressed software update package (download, integrity check, dependency validation) and return 202 Accepted with a Location header pointing to the status resource. .. req:: PUT /updates/{id}/execute :id: REQ_INTEROP_092 - :status: closed + :status: verified :tags: Updates The endpoint shall begin the actual installation of a previously prepared software update package and return 202 Accepted with a Location header pointing to the status resource. .. req:: PUT /updates/{id}/automated :id: REQ_INTEROP_093 - :status: closed + :status: verified :tags: Updates The endpoint shall trigger a fully automated update (prepare + execute) for packages where automated mode is supported and return 202 Accepted with a Location header pointing to the status resource. .. req:: GET /updates/{id}/status :id: REQ_INTEROP_094 - :status: closed + :status: verified :tags: Updates The endpoint shall return the current progress and status of an update operation, including status value, progress percentage, and optional sub-progress details. From 81384a6356aaa461e95ec2691490beb01792a382 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 16:13:05 +0000 Subject: [PATCH 13/15] fix: detect @verifies tags before test definitions and add GitHub links The verification script only scanned inside test bodies for @verifies tags, missing tags placed as comments before def/TEST lines. Now scans backwards from each test to find preceding @verifies comments. Also generates clickable GitHub links with line numbers in verification.rst. --- scripts/generate_verification.py | 50 ++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/scripts/generate_verification.py b/scripts/generate_verification.py index 7b6f3b85..d7030f63 100755 --- a/scripts/generate_verification.py +++ b/scripts/generate_verification.py @@ -25,6 +25,36 @@ SRC_DIR = WORKSPACE_DIR / "src" OUTPUT_FILE = WORKSPACE_DIR / "docs/requirements/verification.rst" REQUIREMENTS_SPECS_DIR = WORKSPACE_DIR / "docs/requirements/specs" +GITHUB_BASE_URL = "https://github.com/selfpatch/ros2_medkit/blob/main" + + +def _extract_verifies_before(content, match_start, comment_prefix): + """Extract @verifies tags from comment lines immediately before a position. + + Scans backwards from match_start through contiguous comment lines + that contain @verifies (or Links to: / Verifies:) tags. + """ + before_text = content[:match_start] + before_lines = before_text.rstrip().split("\n") + + verifies_reqs = [] + for line in reversed(before_lines): + stripped = line.strip() + if not stripped: + continue + if not stripped.startswith(comment_prefix): + break + comment_content = stripped[len(comment_prefix):].strip() + tag_match = re.match( + r"(?:@verifies|Links to:|Verifies:)\s*(.*)", comment_content + ) + if tag_match: + reqs_text = tag_match.group(1) + reqs = re.findall(r"(REQ_\w+)", reqs_text) + verifies_reqs.extend(reqs) + else: + break + return verifies_reqs def parse_cpp_file(file_path): @@ -50,10 +80,16 @@ def parse_cpp_file(file_path): # Auto-generate ID and Title, including suite to avoid collisions test_id = f"TEST_{suite_name}_{test_name}" test_title = test_name + line_number = content[:match.start()].count('\n') + 1 verifies_reqs = [] description = [] + # Check comments before the TEST macro + verifies_reqs.extend( + _extract_verifies_before(content, match.start(), "//") + ) + for line in lines: line = line.strip() if not line: @@ -92,6 +128,7 @@ def parse_cpp_file(file_path): "description": "\n ".join(description), "file": str(file_path.relative_to(WORKSPACE_DIR)), "test_func": test_name, + "line": line_number, } ) @@ -141,10 +178,16 @@ def _find_class(pos): else: test_id = "TEST_" + test_name test_title = test_name + line_number = content[:match.start()].count('\n') + 1 verifies_reqs = [] description = [] + # Check comments before the def line + verifies_reqs.extend( + _extract_verifies_before(content, match.start(), "#") + ) + in_docstring = False docstring_lines = [] @@ -222,6 +265,7 @@ def _find_class(pos): "description": "\n ".join(description), "file": str(file_path.relative_to(WORKSPACE_DIR)), "test_func": test_name, + "line": line_number, } ) @@ -250,9 +294,11 @@ def generate_rst(tests): lines.append(" " + test["description"]) lines.append("") + github_url = f'{GITHUB_BASE_URL}/{test["file"]}#L{test["line"]}' lines.append( - " **Implementation:** ``" + test["file"] + "`` " - "(Test: ``" + test["test_func"] + "``)" + f" **Implementation:** `{test['file']}#L{test['line']}" + f" <{github_url}>`_" + f" (Test: ``{test['test_func']}``)" ) lines.append("") lines.append("") From 9fc6f1801ac292f074b953d0b9819ab4d2e6faaf Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 16:36:56 +0000 Subject: [PATCH 14/15] fix: skip linkcheck anchor verification for GitHub line number links GitHub generates line anchors (#L123) dynamically via JavaScript, so Sphinx linkcheck reports them as broken. Use linkcheck_anchors_ignore_for_url to skip anchor checks while still verifying the pages exist. --- docs/conf.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/conf.py b/docs/conf.py index a9bfb288..7e3169c9 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -158,4 +158,8 @@ linkcheck_ignore = [ r"http://localhost:\d+", # Ignore localhost URLs r"http://127\.0\.0\.1:\d+", + # Auto-generated line-number links in verification.rst; skipped to + # avoid 395+ GitHub requests (rate-limiting) and anchor mismatches + # on branches where files differ from main. + r"https://github\.com/selfpatch/ros2_medkit/blob/.+#L\d+", ] From 9b9fbdf103c31a720625a3afca47edf53760fdd5 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Sun, 22 Feb 2026 20:52:49 +0000 Subject: [PATCH 15/15] fix: address review feedback on update subsystem thread safety and validation Replace string-based backend error matching with typed UpdateBackendError enum and UpdateBackendErrorInfo struct across UpdateBackend interface, UpdateManager, and all mock/test backends. Fix TOCTOU race in delete_update by creating Deleting sentinel when no PackageState exists. Move id validation and entity_id format check before backend call in register handler. Remove dead Deleting phase check in start_execute. Add stopped_ atomic flag to prevent new operations during shutdown. Add execute failure/exception tests with dedicated mock backends. Fix all polling loops to use deadline-based waits with explicit timeout assertions. --- .../updates/update_backend.hpp | 41 ++- .../updates/update_manager.hpp | 2 + .../src/http/handlers/update_handlers.cpp | 25 +- .../src/updates/update_manager.cpp | 70 +++-- .../test/demo_nodes/test_update_backend.cpp | 46 ++-- .../test/test_update_manager.cpp | 249 ++++++++++++++---- 6 files changed, 329 insertions(+), 104 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp index 324f1476..3e47ce35 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_backend.hpp @@ -50,6 +50,20 @@ struct UpdateStatusInfo { /// Internal phase tracking for update lifecycle enum class UpdatePhase { None, Preparing, Prepared, Executing, Executed, Failed, Deleting }; +/// Error codes for backend return values +enum class UpdateBackendError { + NotFound, // Package does not exist + AlreadyExists, // Duplicate ID on registration + InvalidInput, // Malformed metadata + Internal // Unexpected error +}; + +/// Typed error for backend return values +struct UpdateBackendErrorInfo { + UpdateBackendError code; + std::string message; +}; + /** * @brief Thread-safe reporter for update progress. * @@ -92,10 +106,13 @@ class UpdateProgressReporter { * * @par Thread Safety * - CRUD methods (list_updates, get_update, register_update, delete_update) - * are called while UpdateManager holds its mutex. They will not be called - * concurrently with each other, but may overlap with prepare/execute running - * in a background thread. If the backend shares state between CRUD and - * async methods, it must provide its own synchronization. + * are called WITHOUT holding UpdateManager's mutex. They may be called + * concurrently with each other and with prepare/execute running in a + * background thread. If the backend shares state, it must provide its own + * synchronization. + * Note: delete_update has a partial guard - UpdateManager checks/sets a + * Deleting sentinel under lock before calling delete_update, but the + * backend call itself runs outside the lock. * - prepare() and execute() run in a background std::async thread. They may * run concurrently with CRUD calls from the HTTP thread. * - The UpdateProgressReporter passed to prepare/execute is already @@ -111,28 +128,30 @@ class UpdateBackend { // ---- CRUD (plugin owns all metadata storage) ---- /// List all registered update package IDs, optionally filtered - virtual tl::expected, std::string> list_updates(const UpdateFilter & filter) = 0; + virtual tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) = 0; /// Get full metadata for a specific update package as JSON - virtual tl::expected get_update(const std::string & id) = 0; + virtual tl::expected get_update(const std::string & id) = 0; /// Register a new update package from JSON metadata - virtual tl::expected register_update(const nlohmann::json & metadata) = 0; + virtual tl::expected register_update(const nlohmann::json & metadata) = 0; /// Delete an update package - virtual tl::expected delete_update(const std::string & id) = 0; + virtual tl::expected delete_update(const std::string & id) = 0; // ---- Async operations (called in background thread by UpdateManager) ---- /// Prepare an update (download, verify, check dependencies). /// Reporter is optional - plugin may call reporter.set_progress() etc. - virtual tl::expected prepare(const std::string & id, UpdateProgressReporter & reporter) = 0; + virtual tl::expected prepare(const std::string & id, + UpdateProgressReporter & reporter) = 0; /// Execute an update (install). Only called after prepare succeeds. - virtual tl::expected execute(const std::string & id, UpdateProgressReporter & reporter) = 0; + virtual tl::expected execute(const std::string & id, + UpdateProgressReporter & reporter) = 0; /// Check whether a package supports automated mode (prepare + execute) - virtual tl::expected supports_automated(const std::string & id) = 0; + virtual tl::expected supports_automated(const std::string & id) = 0; /// Optional: receive plugin-specific configuration from YAML virtual void configure(const nlohmann::json & /* config */) { diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp index 16080c98..6fe22bb9 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/updates/update_manager.hpp @@ -14,6 +14,7 @@ #pragma once +#include #include #include #include @@ -96,6 +97,7 @@ class UpdateManager { // map rehashes due to concurrent insertions. std::unordered_map> states_; mutable std::mutex mutex_; + std::atomic stopped_{false}; /// Check if a task is still running bool is_task_active(const std::string & id) const; diff --git a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp index 1f96b909..1f9f81a5 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/update_handlers.cpp @@ -60,10 +60,7 @@ json UpdateHandlers::status_to_json(const UpdateStatusInfo & status) { } } if (status.error_message.has_value()) { - json err; - err["error_code"] = ERR_INTERNAL_ERROR; - err["message"] = *status.error_message; - j["error"] = err; + j["error"] = *status.error_message; } return j; } @@ -134,6 +131,20 @@ void UpdateHandlers::handle_register_update(const httplib::Request & req, httpli return; } + // Validate id field exists before calling backend + if (!body.contains("id") || !body["id"].is_string() || body["id"].get().empty()) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Missing required field: id"); + return; + } + auto id = body["id"].get(); + + // Validate id format to prevent CRLF injection in Location header + auto id_validation = ctx_.validate_entity_id(id); + if (!id_validation) { + HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, id_validation.error()); + return; + } + auto result = update_mgr_->register_update(body); if (!result) { switch (result.error().code) { @@ -147,12 +158,6 @@ void UpdateHandlers::handle_register_update(const httplib::Request & req, httpli return; } - // Validate id field exists before using it for Location header - if (!body.contains("id") || !body["id"].is_string() || body["id"].get().empty()) { - HandlerContext::send_error(res, 400, ERR_INVALID_REQUEST, "Missing required field: id"); - return; - } - auto id = body["id"].get(); json response = {{"id", id}}; HandlerContext::send_json(res, response); res.status = 201; diff --git a/src/ros2_medkit_gateway/src/updates/update_manager.cpp b/src/ros2_medkit_gateway/src/updates/update_manager.cpp index c115a1bb..7ecc28a1 100644 --- a/src/ros2_medkit_gateway/src/updates/update_manager.cpp +++ b/src/ros2_medkit_gateway/src/updates/update_manager.cpp @@ -23,6 +23,9 @@ UpdateManager::UpdateManager(std::unique_ptr backend, void * plug } UpdateManager::~UpdateManager() { + // Signal background tasks to stop accepting new work + stopped_ = true; + // Collect all valid futures, then wait OUTSIDE the lock to avoid // deadlock (async tasks also acquire mutex_ during execution). std::vector> futures; @@ -54,7 +57,7 @@ tl::expected, UpdateError> UpdateManager::list_updates( } auto result = backend_->list_updates(filter); if (!result) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, result.error()}); + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, result.error().message}); } return *result; } @@ -65,7 +68,7 @@ tl::expected UpdateManager::get_update(const std::s } auto result = backend_->get_update(id); if (!result) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, result.error()}); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, result.error().message}); } return *result; } @@ -76,10 +79,15 @@ tl::expected UpdateManager::register_update(const nlohmann::j } auto result = backend_->register_update(metadata); if (!result) { - if (result.error().find("already exists") != std::string::npos) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::AlreadyExists, result.error()}); + const auto & err = result.error(); + switch (err.code) { + case UpdateBackendError::AlreadyExists: + return tl::make_unexpected(UpdateError{UpdateErrorCode::AlreadyExists, err.message}); + case UpdateBackendError::InvalidInput: + return tl::make_unexpected(UpdateError{UpdateErrorCode::InvalidRequest, err.message}); + default: + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, err.message}); } - return tl::make_unexpected(UpdateError{UpdateErrorCode::InvalidRequest, result.error()}); } return {}; } @@ -89,10 +97,12 @@ tl::expected UpdateManager::delete_update(const std::string & return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } + bool had_state = false; { std::lock_guard lock(mutex_); auto it = states_.find(id); if (it != states_.end()) { + had_state = true; if (is_task_active(id)) { return tl::make_unexpected( UpdateError{UpdateErrorCode::InProgress, "Cannot delete update while operation is in progress"}); @@ -103,6 +113,10 @@ tl::expected UpdateManager::delete_update(const std::string & } // Mark as deleting so no new operations can start on this package it->second->phase = UpdatePhase::Deleting; + } else { + // Create sentinel to prevent concurrent start_prepare + states_[id] = std::make_unique(); + states_[id]->phase = UpdatePhase::Deleting; } } @@ -113,14 +127,22 @@ tl::expected UpdateManager::delete_update(const std::string & } else { // Rollback sentinel on failure std::lock_guard lock(mutex_); - auto it = states_.find(id); - if (it != states_.end() && it->second) { - it->second->phase = UpdatePhase::Failed; + if (had_state) { + auto it = states_.find(id); + if (it != states_.end() && it->second) { + it->second->phase = UpdatePhase::Failed; + } + } else { + // Remove the sentinel we created - package never had state before + states_.erase(id); } - if (result.error().find("not found") != std::string::npos) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, result.error()}); + const auto & err = result.error(); + switch (err.code) { + case UpdateBackendError::NotFound: + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, err.message}); + default: + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, err.message}); } - return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, result.error()}); } return {}; } @@ -129,13 +151,16 @@ tl::expected UpdateManager::start_prepare(const std::string & if (!backend_) { return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } + if (stopped_) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, "UpdateManager is shutting down"}); + } std::lock_guard lock(mutex_); // Verify package exists while holding lock to prevent concurrent deletion auto pkg = backend_->get_update(id); if (!pkg) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error()}); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error().message}); } if (is_task_active(id)) { @@ -162,21 +187,21 @@ tl::expected UpdateManager::start_execute(const std::string & if (!backend_) { return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } + if (stopped_) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, "UpdateManager is shutting down"}); + } std::lock_guard lock(mutex_); auto pkg = backend_->get_update(id); if (!pkg) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error()}); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, pkg.error().message}); } auto it = states_.find(id); if (it == states_.end() || !it->second || it->second->phase != UpdatePhase::Prepared) { return tl::make_unexpected(UpdateError{UpdateErrorCode::NotPrepared, "Package must be prepared before execution"}); } - if (it->second->phase == UpdatePhase::Deleting) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::Deleting, "Package is being deleted"}); - } if (is_task_active(id)) { return tl::make_unexpected( UpdateError{UpdateErrorCode::InProgress, "An operation is already in progress for this package"}); @@ -193,12 +218,15 @@ tl::expected UpdateManager::start_automated(const std::string if (!backend_) { return tl::make_unexpected(UpdateError{UpdateErrorCode::NoBackend, "No update backend loaded"}); } + if (stopped_) { + return tl::make_unexpected(UpdateError{UpdateErrorCode::Internal, "UpdateManager is shutting down"}); + } std::lock_guard lock(mutex_); auto supported = backend_->supports_automated(id); if (!supported) { - return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, supported.error()}); + return tl::make_unexpected(UpdateError{UpdateErrorCode::NotFound, supported.error().message}); } if (!*supported) { return tl::make_unexpected( @@ -263,7 +291,7 @@ void UpdateManager::run_prepare(const std::string & id) { state->phase = UpdatePhase::Prepared; } else { state->status.status = UpdateStatus::Failed; - state->status.error_message = result.error(); + state->status.error_message = result.error().message; state->phase = UpdatePhase::Failed; } } catch (const std::exception & e) { @@ -303,7 +331,7 @@ void UpdateManager::run_execute(const std::string & id) { state->phase = UpdatePhase::Executed; } else { state->status.status = UpdateStatus::Failed; - state->status.error_message = result.error(); + state->status.error_message = result.error().message; state->phase = UpdatePhase::Failed; } } catch (const std::exception & e) { @@ -341,7 +369,7 @@ void UpdateManager::run_automated(const std::string & id) { if (!prep_result) { std::lock_guard lock(mutex_); state->status.status = UpdateStatus::Failed; - state->status.error_message = prep_result.error(); + state->status.error_message = prep_result.error().message; state->phase = UpdatePhase::Failed; return; } @@ -362,7 +390,7 @@ void UpdateManager::run_automated(const std::string & id) { state->phase = UpdatePhase::Executed; } else { state->status.status = UpdateStatus::Failed; - state->status.error_message = exec_result.error(); + state->status.error_message = exec_result.error().message; state->phase = UpdatePhase::Failed; } } catch (const std::exception & e) { diff --git a/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp index 93df4d8b..cdbb811c 100644 --- a/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp +++ b/src/ros2_medkit_gateway/test/demo_nodes/test_update_backend.cpp @@ -34,7 +34,7 @@ using namespace ros2_medkit_gateway; */ class TestUpdateBackend : public UpdateBackend { public: - tl::expected, std::string> list_updates(const UpdateFilter & filter) override { + tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter & filter) override { std::lock_guard lock(mutex_); std::vector ids; for (const auto & [id, meta] : packages_) { @@ -62,53 +62,62 @@ class TestUpdateBackend : public UpdateBackend { return ids; } - tl::expected get_update(const std::string & id) override { + tl::expected get_update(const std::string & id) override { std::lock_guard lock(mutex_); auto it = packages_.find(id); if (it == packages_.end()) { - return tl::make_unexpected("Update package '" + id + "' not found"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "Update package '" + id + "' not found"}); } return it->second; } - tl::expected register_update(const json & metadata) override { + tl::expected register_update(const json & metadata) override { if (!metadata.contains("id") || !metadata["id"].is_string()) { - return tl::make_unexpected("Missing required field: id"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "Missing required field: id"}); } if (!metadata.contains("update_name") || !metadata["update_name"].is_string()) { - return tl::make_unexpected("Missing required field: update_name"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "Missing required field: update_name"}); } if (!metadata.contains("automated") || !metadata["automated"].is_boolean()) { - return tl::make_unexpected("Missing required field: automated"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "Missing required field: automated"}); } if (!metadata.contains("origins") || !metadata["origins"].is_array()) { - return tl::make_unexpected("Missing required field: origins"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "Missing required field: origins"}); } std::string id = metadata["id"].get(); std::lock_guard lock(mutex_); if (packages_.count(id) > 0) { - return tl::make_unexpected("Update package '" + id + "' already exists"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::AlreadyExists, "Update package '" + id + "' already exists"}); } packages_[id] = metadata; return {}; } - tl::expected delete_update(const std::string & id) override { + tl::expected delete_update(const std::string & id) override { std::lock_guard lock(mutex_); auto it = packages_.find(id); if (it == packages_.end()) { - return tl::make_unexpected("Update package '" + id + "' not found"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "Update package '" + id + "' not found"}); } packages_.erase(it); return {}; } - tl::expected prepare(const std::string & id, UpdateProgressReporter & reporter) override { + tl::expected prepare(const std::string & id, + UpdateProgressReporter & reporter) override { { std::lock_guard lock(mutex_); if (packages_.find(id) == packages_.end()) { - return tl::make_unexpected("Update package '" + id + "' not found"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "Update package '" + id + "' not found"}); } } @@ -131,11 +140,13 @@ class TestUpdateBackend : public UpdateBackend { return {}; } - tl::expected execute(const std::string & id, UpdateProgressReporter & reporter) override { + tl::expected execute(const std::string & id, + UpdateProgressReporter & reporter) override { { std::lock_guard lock(mutex_); if (packages_.find(id) == packages_.end()) { - return tl::make_unexpected("Update package '" + id + "' not found"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "Update package '" + id + "' not found"}); } } @@ -158,11 +169,12 @@ class TestUpdateBackend : public UpdateBackend { return {}; } - tl::expected supports_automated(const std::string & id) override { + tl::expected supports_automated(const std::string & id) override { std::lock_guard lock(mutex_); auto it = packages_.find(id); if (it == packages_.end()) { - return tl::make_unexpected("Update package '" + id + "' not found"); + return tl::make_unexpected( + UpdateBackendErrorInfo{UpdateBackendError::NotFound, "Update package '" + id + "' not found"}); } return it->second.value("automated", false); } diff --git a/src/ros2_medkit_gateway/test/test_update_manager.cpp b/src/ros2_medkit_gateway/test/test_update_manager.cpp index 0ce2987f..7f0015b4 100644 --- a/src/ros2_medkit_gateway/test/test_update_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_update_manager.cpp @@ -25,7 +25,7 @@ using json = nlohmann::json; /// Mock backend for unit testing class MockUpdateBackend : public UpdateBackend { public: - tl::expected, std::string> list_updates(const UpdateFilter &) override { + tl::expected, UpdateBackendErrorInfo> list_updates(const UpdateFilter &) override { std::lock_guard lock(mutex_); std::vector ids; for (const auto & [id, _] : packages_) { @@ -34,54 +34,54 @@ class MockUpdateBackend : public UpdateBackend { return ids; } - tl::expected get_update(const std::string & id) override { + tl::expected get_update(const std::string & id) override { std::lock_guard lock(mutex_); auto it = packages_.find(id); if (it == packages_.end()) { - return tl::make_unexpected("not found"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::NotFound, "not found"}); } return it->second; } - tl::expected register_update(const json & metadata) override { + tl::expected register_update(const json & metadata) override { auto id = metadata.value("id", std::string{}); if (id.empty()) { - return tl::make_unexpected("missing id"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "missing id"}); } std::lock_guard lock(mutex_); if (packages_.count(id)) { - return tl::make_unexpected("already exists"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::AlreadyExists, "already exists"}); } packages_[id] = metadata; return {}; } - tl::expected delete_update(const std::string & id) override { + tl::expected delete_update(const std::string & id) override { std::lock_guard lock(mutex_); if (packages_.erase(id) == 0) { - return tl::make_unexpected("not found"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::NotFound, "not found"}); } return {}; } - tl::expected prepare(const std::string &, UpdateProgressReporter & reporter) override { + tl::expected prepare(const std::string &, UpdateProgressReporter & reporter) override { reporter.set_progress(50); std::this_thread::sleep_for(std::chrono::milliseconds(50)); reporter.set_progress(100); return {}; } - tl::expected execute(const std::string &, UpdateProgressReporter & reporter) override { + tl::expected execute(const std::string &, UpdateProgressReporter & reporter) override { reporter.set_progress(100); std::this_thread::sleep_for(std::chrono::milliseconds(50)); return {}; } - tl::expected supports_automated(const std::string & id) override { + tl::expected supports_automated(const std::string & id) override { std::lock_guard lock(mutex_); auto it = packages_.find(id); if (it == packages_.end()) { - return tl::make_unexpected("not found"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::NotFound, "not found"}); } return it->second.value("automated", false); } @@ -198,29 +198,41 @@ TEST_F(UpdateManagerTest, ExecuteAfterPrepare) { (void)manager_->start_prepare("test-pkg"); // Wait for prepare to complete - for (int i = 0; i < 100; ++i) { - auto s = manager_->get_status("test-pkg"); - if (s && s->status == UpdateStatus::Completed) { - break; + { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager_->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Completed) { + found = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + ASSERT_TRUE(found) << "Timed out waiting for prepare to complete"; } auto exec = manager_->start_execute("test-pkg"); ASSERT_TRUE(exec.has_value()); // Wait for execute to complete - UpdateStatusInfo status; - for (int i = 0; i < 100; ++i) { - auto s = manager_->get_status("test-pkg"); - ASSERT_TRUE(s.has_value()); - status = *s; - if (status.status == UpdateStatus::Completed) { - break; + { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + UpdateStatusInfo status; + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager_->get_status("test-pkg"); + ASSERT_TRUE(s.has_value()); + status = *s; + if (status.status == UpdateStatus::Completed) { + found = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + ASSERT_TRUE(found) << "Timed out waiting for execute to complete"; + EXPECT_EQ(status.status, UpdateStatus::Completed); } - EXPECT_EQ(status.status, UpdateStatus::Completed); } // @verifies REQ_INTEROP_093 @@ -231,16 +243,20 @@ TEST_F(UpdateManagerTest, AutomatedCompletes) { auto result = manager_->start_automated("test-pkg"); ASSERT_TRUE(result.has_value()); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); UpdateStatusInfo status; - for (int i = 0; i < 200; ++i) { + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { auto s = manager_->get_status("test-pkg"); ASSERT_TRUE(s.has_value()); status = *s; if (status.status == UpdateStatus::Completed) { + found = true; break; } std::this_thread::sleep_for(std::chrono::milliseconds(10)); } + ASSERT_TRUE(found) << "Timed out waiting for automated update to complete"; EXPECT_EQ(status.status, UpdateStatus::Completed); } @@ -300,29 +316,32 @@ TEST_F(UpdateManagerTest, ConcurrentPrepareOnSamePackageRejected) { /// Mock backend that returns errors from prepare/execute class MockFailingBackend : public UpdateBackend { public: - tl::expected, std::string> list_updates(const UpdateFilter & /*filter*/) override { - return tl::make_unexpected("backend error"); + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter & /*filter*/) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::Internal, "backend error"}); } - tl::expected get_update(const std::string & /*id*/) override { + tl::expected get_update(const std::string & /*id*/) override { return json{{"id", "pkg"}}; } - tl::expected register_update(const json & metadata) override { + tl::expected register_update(const json & metadata) override { auto id = metadata.value("id", std::string{}); if (id.empty()) { - return tl::make_unexpected("missing id"); + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::InvalidInput, "missing id"}); } return {}; } - tl::expected delete_update(const std::string & /*id*/) override { + tl::expected delete_update(const std::string & /*id*/) override { return {}; } - tl::expected prepare(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { - return tl::make_unexpected("download failed"); + tl::expected prepare(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::Internal, "download failed"}); } - tl::expected execute(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { - return tl::make_unexpected("install failed"); + tl::expected execute(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::Internal, "install failed"}); } - tl::expected supports_automated(const std::string & /*id*/) override { + tl::expected supports_automated(const std::string & /*id*/) override { return true; } }; @@ -330,25 +349,28 @@ class MockFailingBackend : public UpdateBackend { /// Mock backend that throws exceptions from prepare/execute class MockThrowingBackend : public UpdateBackend { public: - tl::expected, std::string> list_updates(const UpdateFilter & /*filter*/) override { + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter & /*filter*/) override { return std::vector{}; } - tl::expected get_update(const std::string & /*id*/) override { + tl::expected get_update(const std::string & /*id*/) override { return json{{"id", "pkg"}}; } - tl::expected register_update(const json & /*metadata*/) override { + tl::expected register_update(const json & /*metadata*/) override { return {}; } - tl::expected delete_update(const std::string & /*id*/) override { + tl::expected delete_update(const std::string & /*id*/) override { return {}; } - tl::expected prepare(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + tl::expected prepare(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { throw std::runtime_error("plugin crashed"); } - tl::expected execute(const std::string & /*id*/, UpdateProgressReporter & /*reporter*/) override { + tl::expected execute(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { throw std::runtime_error("plugin crashed"); } - tl::expected supports_automated(const std::string & /*id*/) override { + tl::expected supports_automated(const std::string & /*id*/) override { return true; } }; @@ -379,6 +401,66 @@ TEST(UpdateManagerFailureTest, PrepareFailureSetsFailedStatus) { EXPECT_NE(status.error_message->find("download failed"), std::string::npos); } +/// Mock backend with working prepare but failing execute +class MockExecuteFailingBackend : public UpdateBackend { + public: + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter & /*filter*/) override { + return std::vector{}; + } + tl::expected get_update(const std::string & /*id*/) override { + return json{{"id", "pkg"}}; + } + tl::expected register_update(const json & /*metadata*/) override { + return {}; + } + tl::expected delete_update(const std::string & /*id*/) override { + return {}; + } + tl::expected prepare(const std::string & /*id*/, + UpdateProgressReporter & reporter) override { + reporter.set_progress(100); + return {}; + } + tl::expected execute(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { + return tl::make_unexpected(UpdateBackendErrorInfo{UpdateBackendError::Internal, "install failed"}); + } + tl::expected supports_automated(const std::string & /*id*/) override { + return true; + } +}; + +/// Mock backend with working prepare but throwing execute +class MockExecuteThrowingBackend : public UpdateBackend { + public: + tl::expected, UpdateBackendErrorInfo> + list_updates(const UpdateFilter & /*filter*/) override { + return std::vector{}; + } + tl::expected get_update(const std::string & /*id*/) override { + return json{{"id", "pkg"}}; + } + tl::expected register_update(const json & /*metadata*/) override { + return {}; + } + tl::expected delete_update(const std::string & /*id*/) override { + return {}; + } + tl::expected prepare(const std::string & /*id*/, + UpdateProgressReporter & reporter) override { + reporter.set_progress(100); + return {}; + } + tl::expected execute(const std::string & /*id*/, + UpdateProgressReporter & /*reporter*/) override { + throw std::runtime_error("plugin crashed during install"); + } + tl::expected supports_automated(const std::string & /*id*/) override { + return true; + } +}; + // @verifies REQ_INTEROP_091 TEST(UpdateManagerFailureTest, PrepareExceptionSetsFailedStatus) { auto backend = std::make_unique(); @@ -403,3 +485,80 @@ TEST(UpdateManagerFailureTest, PrepareExceptionSetsFailedStatus) { ASSERT_TRUE(status.error_message.has_value()); EXPECT_NE(status.error_message->find("Exception"), std::string::npos); } + +// Helper: prepare a package and wait for completion +static bool prepare_and_wait(UpdateManager & manager, const std::string & id) { + auto prep = manager.start_prepare(id); + if (!prep) { + return false; + } + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager.get_status(id); + if (s && s->status == UpdateStatus::Completed) { + return true; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + return false; +} + +// @verifies REQ_INTEROP_092 +TEST(UpdateManagerFailureTest, ExecuteFailureSetsFailedStatus) { + auto backend = std::make_unique(); + auto manager = std::make_unique(std::move(backend)); + json pkg = {{"id", "test-pkg"}}; + (void)manager->register_update(pkg); + + ASSERT_TRUE(prepare_and_wait(*manager, "test-pkg")) << "Timed out waiting for prepare to complete"; + + auto exec = manager->start_execute("test-pkg"); + ASSERT_TRUE(exec.has_value()); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + UpdateStatusInfo status; + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Failed) { + status = *s; + found = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + ASSERT_TRUE(found) << "Timed out waiting for execute to fail"; + EXPECT_EQ(status.status, UpdateStatus::Failed); + ASSERT_TRUE(status.error_message.has_value()); + EXPECT_NE(status.error_message->find("install failed"), std::string::npos); +} + +// @verifies REQ_INTEROP_092 +TEST(UpdateManagerFailureTest, ExecuteExceptionSetsFailedStatus) { + auto backend = std::make_unique(); + auto manager = std::make_unique(std::move(backend)); + json pkg = {{"id", "test-pkg"}}; + (void)manager->register_update(pkg); + + ASSERT_TRUE(prepare_and_wait(*manager, "test-pkg")) << "Timed out waiting for prepare to complete"; + + auto exec = manager->start_execute("test-pkg"); + ASSERT_TRUE(exec.has_value()); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + UpdateStatusInfo status; + bool found = false; + while (std::chrono::steady_clock::now() < deadline) { + auto s = manager->get_status("test-pkg"); + if (s && s->status == UpdateStatus::Failed) { + status = *s; + found = true; + break; + } + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + ASSERT_TRUE(found) << "Timed out waiting for execute exception to be caught"; + EXPECT_EQ(status.status, UpdateStatus::Failed); + ASSERT_TRUE(status.error_message.has_value()); + EXPECT_NE(status.error_message->find("Exception"), std::string::npos); +}