From 466f643866cb72bd0fe3b9437548fd46706033d9 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 27 Jan 2026 16:33:29 +0000 Subject: [PATCH 1/3] fix(gateway): aggregate configurations for aggregate SOVD entities Add configuration aggregation for Areas, Components, and Functions by collecting parameters from all child App nodes. Implements pattern used by data/operations endpoints. Parameter IDs use "app_id:param_name" format for disambiguation. Updates endpoint documentation and adds smoke test. --- .../models/thread_safe_entity_cache.hpp | 73 +++ .../src/http/handlers/config_handlers.cpp | 485 +++++++++++++----- .../src/http/handlers/health_handlers.cpp | 84 ++- .../src/models/thread_safe_entity_cache.cpp | 170 ++++++ .../test/test_discovery_manifest.test.py | 16 +- .../test/test_integration.test.py | 93 +++- 6 files changed, 783 insertions(+), 138 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/models/thread_safe_entity_cache.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/models/thread_safe_entity_cache.hpp index 8c23f8b..32b5aa5 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/models/thread_safe_entity_cache.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/models/thread_safe_entity_cache.hpp @@ -82,6 +82,37 @@ struct AggregatedOperations { } }; +/** + * @brief Node info for configuration aggregation + */ +struct NodeConfigInfo { + std::string node_fqn; ///< Fully qualified node name + std::string app_id; ///< Source app ID + std::string entity_id; ///< Owning entity ID (app/component/area) +}; + +/** + * @brief Aggregated configurations (node FQNs) result from entity hierarchy + * + * Unlike data/operations which store actual values, configurations stores + * the list of ROS 2 nodes whose parameters should be queried. This is because + * parameters are owned by nodes, and aggregated entities need to iterate + * over all child nodes. + */ +struct AggregatedConfigurations { + std::vector nodes; ///< Nodes to query for parameters + std::vector source_ids; ///< Entity IDs that contributed + std::string aggregation_level; ///< "app" | "component" | "area" | "function" + bool is_aggregated{false}; ///< true if collected from sub-entities + + bool empty() const { + return nodes.empty(); + } + size_t node_count() const { + return nodes.size(); + } +}; + /** * @brief Cache statistics */ @@ -297,6 +328,48 @@ class ThreadSafeEntityCache { */ AggregatedData get_function_data(const std::string & function_id) const; + // ========================================================================= + // Configuration aggregation methods (collects node FQNs for parameter access) + // ========================================================================= + + /** + * @brief Aggregate configurations (node FQNs) for any entity by ID + * + * Unified method that works for all entity types. + * For Apps, returns the single bound node. + * For Components/Areas/Functions, aggregates all child app nodes. + * + * @param entity_id Entity ID to get configurations for + * @return AggregatedConfigurations with node FQNs, or empty if entity not found + */ + AggregatedConfigurations get_entity_configurations(const std::string & entity_id) const; + + /** + * @brief Get configurations for an App (returns its single bound node) + */ + AggregatedConfigurations get_app_configurations(const std::string & app_id) const; + + /** + * @brief Aggregate configurations for a Component + * + * Returns: All node FQNs from hosted Apps. + */ + AggregatedConfigurations get_component_configurations(const std::string & component_id) const; + + /** + * @brief Aggregate configurations for an Area + * + * Returns: All node FQNs from all Components in the Area. + */ + AggregatedConfigurations get_area_configurations(const std::string & area_id) const; + + /** + * @brief Aggregate configurations for a Function + * + * Returns: All node FQNs from all Apps implementing this Function. + */ + AggregatedConfigurations get_function_configurations(const std::string & function_id) const; + // ========================================================================= // Operation lookup (O(1) via operation index) // ========================================================================= diff --git a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp index 13a4fd5..9ec9b4e 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -49,45 +49,96 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht return; } - // Get node name for parameter access - std::string node_name = entity_info.fqn; - if (node_name.empty()) { - node_name = "/" + entity_id; + // Get aggregated configurations info for this entity + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto agg_configs = cache.get_entity_configurations(entity_id); + + // If no nodes to query, return empty result + if (agg_configs.nodes.empty()) { + json response; + response["items"] = json::array(); + + XMedkit ext; + ext.entity_id(entity_id).source("runtime"); + ext.add("aggregation_level", agg_configs.aggregation_level); + ext.add("is_aggregated", agg_configs.is_aggregated); + response["x-medkit"] = ext.build(); + + HandlerContext::send_json(res, response); + return; } auto config_mgr = ctx_.node()->get_configuration_manager(); - auto result = config_mgr->list_parameters(node_name); - - if (result.success) { - // SOVD format: items array with ConfigurationMetaData objects - json items = json::array(); - if (result.data.is_array()) { - for (const auto & param : result.data) { - json config_meta; - // SOVD required fields - std::string param_name = param.value("name", ""); - config_meta["id"] = param_name; - config_meta["name"] = param_name; - config_meta["type"] = "parameter"; // ROS2 parameters are always parameter type (not bulk) - items.push_back(config_meta); + json items = json::array(); + json all_parameters = json::array(); + std::vector queried_nodes; + bool any_success = false; + std::string first_error; + + // Query each node and aggregate results + for (const auto & node_info : agg_configs.nodes) { + auto result = config_mgr->list_parameters(node_info.node_fqn); + + if (result.success) { + any_success = true; + queried_nodes.push_back(node_info.node_fqn); + + if (result.data.is_array()) { + for (const auto & param : result.data) { + json config_meta; + std::string param_name = param.value("name", ""); + + // Create unique ID for aggregated configs: node_fqn:param_name + std::string unique_id = param_name; + if (agg_configs.is_aggregated) { + unique_id = node_info.app_id + ":" + param_name; + } + + config_meta["id"] = unique_id; + config_meta["name"] = param_name; + config_meta["type"] = "parameter"; + + // Add source info for aggregated configurations + if (agg_configs.is_aggregated) { + config_meta["x-medkit-source"] = node_info.app_id; + } + + items.push_back(config_meta); + + // Also track full parameter info + json param_with_source = param; + param_with_source["x-medkit-source"] = node_info.app_id; + param_with_source["x-medkit-node"] = node_info.node_fqn; + all_parameters.push_back(param_with_source); + } } + } else if (first_error.empty()) { + first_error = result.error_message; } + } - // Build x-medkit extension with ROS2-specific data - XMedkit ext; - ext.ros2_node(node_name).entity_id(entity_id).source("runtime"); - // Add original parameter details to x-medkit - ext.add("parameters", result.data); - - json response; - response["items"] = items; - response["x-medkit"] = ext.build(); - HandlerContext::send_json(res, response); - } else { + // If no successful queries, return error + if (!any_success) { HandlerContext::send_error(res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to list parameters", - {{"details", result.error_message}, {"node_name", node_name}}); + "Failed to list parameters from any node", + {{"details", first_error}, {"entity_id", entity_id}}); + return; } + + // Build x-medkit extension + XMedkit ext; + ext.entity_id(entity_id).source("runtime"); + ext.add("parameters", all_parameters); + ext.add("aggregation_level", agg_configs.aggregation_level); + ext.add("is_aggregated", agg_configs.is_aggregated); + ext.add("source_ids", agg_configs.source_ids); + ext.add("queried_nodes", queried_nodes); + + json response; + response["items"] = items; + response["x-medkit"] = ext.build(); + HandlerContext::send_json(res, response); + } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, "Failed to list configurations", {{"details", e.what()}, {"entity_id", entity_id}}); @@ -98,7 +149,7 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht void ConfigHandlers::handle_get_configuration(const httplib::Request & req, httplib::Response & res) { std::string entity_id; - std::string param_name; + std::string param_id; try { if (req.matches.size() < 3) { HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_REQUEST, "Invalid request"); @@ -106,7 +157,7 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http } entity_id = req.matches[1]; - param_name = req.matches[2]; + param_id = req.matches[2]; auto entity_validation = ctx_.validate_entity_id(entity_id); if (!entity_validation) { @@ -115,10 +166,10 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http return; } - // Parameter names may contain dots, so we use a more permissive validation - if (param_name.empty() || param_name.length() > 256) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter name", - {{"details", "Parameter name is empty or too long"}}); + // Parameter ID may be prefixed with app_id: for aggregated configs + if (param_id.empty() || param_id.length() > 512) { + HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter ID", + {{"details", "Parameter ID is empty or too long"}}); return; } @@ -130,59 +181,117 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http return; } - // Get node name for parameter access - std::string node_name = entity_info.fqn; - if (node_name.empty()) { - node_name = "/" + entity_id; + // Get aggregated configurations info + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto agg_configs = cache.get_entity_configurations(entity_id); + + if (agg_configs.nodes.empty()) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "No nodes available", + json{{"entity_id", entity_id}, {"id", param_id}}); + return; } auto config_mgr = ctx_.node()->get_configuration_manager(); - auto result = config_mgr->get_parameter(node_name, param_name); - if (result.success) { - // SOVD format: ReadConfigurations response with id and data - json response; - response["id"] = param_name; + // Check if param_id contains app_id prefix (for aggregated configs: "app_id:param_name") + std::string target_app_id; + std::string param_name = param_id; + auto colon_pos = param_id.find(':'); + if (colon_pos != std::string::npos && agg_configs.is_aggregated) { + target_app_id = param_id.substr(0, colon_pos); + param_name = param_id.substr(colon_pos + 1); + } - // Extract value from parameter data - if (result.data.contains("value")) { - response["data"] = result.data["value"]; - } else { - response["data"] = result.data; + // If targeting specific app in aggregated entity + if (!target_app_id.empty()) { + for (const auto & node_info : agg_configs.nodes) { + if (node_info.app_id == target_app_id) { + auto result = config_mgr->get_parameter(node_info.node_fqn, param_name); + + if (result.success) { + json response; + response["id"] = param_id; + + if (result.data.contains("value")) { + response["data"] = result.data["value"]; + } else { + response["data"] = result.data; + } + + XMedkit ext; + ext.ros2_node(node_info.node_fqn).entity_id(entity_id).source("runtime"); + ext.add("parameter", result.data); + ext.add("source_app", target_app_id); + response["x-medkit"] = ext.build(); + + HandlerContext::send_json(res, response); + return; + } else { + if (result.error_message.find("not found") != std::string::npos || + result.error_message.find("Parameter not found") != std::string::npos) { + HandlerContext::send_error( + res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + } else { + HandlerContext::send_error( + res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, + "Failed to get parameter", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + } + return; + } + } } + // App not found in this entity's children + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); + return; + } - // Build x-medkit extension with ROS2-specific data - XMedkit ext; - ext.ros2_node(node_name).entity_id(entity_id).source("runtime"); - // Add original parameter object to x-medkit for full type info - ext.add("parameter", result.data); - response["x-medkit"] = ext.build(); + // For non-aggregated or no prefix: search all nodes for the parameter + for (const auto & node_info : agg_configs.nodes) { + auto result = config_mgr->get_parameter(node_info.node_fqn, param_name); - HandlerContext::send_json(res, response); - } else { - // Check if it's a "not found" error - if (result.error_message.find("not found") != std::string::npos || - result.error_message.find("Parameter not found") != std::string::npos) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", - {{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_name}}); - } else { - HandlerContext::send_error(res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to get parameter", - {{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_name}}); + if (result.success) { + json response; + response["id"] = param_name; + + if (result.data.contains("value")) { + response["data"] = result.data["value"]; + } else { + response["data"] = result.data; + } + + XMedkit ext; + ext.ros2_node(node_info.node_fqn).entity_id(entity_id).source("runtime"); + ext.add("parameter", result.data); + if (agg_configs.is_aggregated) { + ext.add("source_app", node_info.app_id); + } + response["x-medkit"] = ext.build(); + + HandlerContext::send_json(res, response); + return; } } + + // Parameter not found in any node + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", + {{"entity_id", entity_id}, {"id", param_id}}); + } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, "Failed to get configuration", - {{"details", e.what()}, {"entity_id", entity_id}, {"param_name", param_name}}); + {{"details", e.what()}, {"entity_id", entity_id}, {"param_id", param_id}}); RCLCPP_ERROR(HandlerContext::logger(), "Error in handle_get_configuration for entity '%s', param '%s': %s", - entity_id.c_str(), param_name.c_str(), e.what()); + entity_id.c_str(), param_id.c_str(), e.what()); } } void ConfigHandlers::handle_set_configuration(const httplib::Request & req, httplib::Response & res) { std::string entity_id; - std::string param_name; + std::string param_id; try { if (req.matches.size() < 3) { HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_REQUEST, "Invalid request"); @@ -190,7 +299,7 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http } entity_id = req.matches[1]; - param_name = req.matches[2]; + param_id = req.matches[2]; auto entity_validation = ctx_.validate_entity_id(entity_id); if (!entity_validation) { @@ -199,9 +308,9 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - if (param_name.empty() || param_name.length() > 256) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter name", - {{"details", "Parameter name is empty or too long"}}); + if (param_id.empty() || param_id.length() > 512) { + HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter ID", + {{"details", "Parameter ID is empty or too long"}}); return; } @@ -235,36 +344,52 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - // Get node name for parameter access - std::string node_name = entity_info.fqn; - if (node_name.empty()) { - node_name = "/" + entity_id; + // Get aggregated configurations info + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto agg_configs = cache.get_entity_configurations(entity_id); + + if (agg_configs.nodes.empty()) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "No nodes available", + json{{"entity_id", entity_id}, {"id", param_id}}); + return; } auto config_mgr = ctx_.node()->get_configuration_manager(); - auto result = config_mgr->set_parameter(node_name, param_name, value); - if (result.success) { - // SOVD format: return updated configuration with id and data - json response; - response["id"] = param_name; + // Parse param_id for app_id prefix (for aggregated configs: "app_id:param_name") + std::string target_app_id; + std::string param_name = param_id; + auto colon_pos = param_id.find(':'); + if (colon_pos != std::string::npos && agg_configs.is_aggregated) { + target_app_id = param_id.substr(0, colon_pos); + param_name = param_id.substr(colon_pos + 1); + } - // Extract value from parameter data - if (result.data.contains("value")) { - response["data"] = result.data["value"]; - } else { - response["data"] = result.data; - } + // Helper to handle set result + auto handle_set_result = [&](const auto & result, const std::string & node_fqn, const std::string & app_id) { + if (result.success) { + json response; + response["id"] = param_id; - // Build x-medkit extension with ROS2-specific data - XMedkit ext; - ext.ros2_node(node_name).entity_id(entity_id).source("runtime"); - ext.add("parameter", result.data); - response["x-medkit"] = ext.build(); + if (result.data.contains("value")) { + response["data"] = result.data["value"]; + } else { + response["data"] = result.data; + } - HandlerContext::send_json(res, response); - } else { - // Check if it's a read-only, not found, or service unavailable error + XMedkit ext; + ext.ros2_node(node_fqn).entity_id(entity_id).source("runtime"); + ext.add("parameter", result.data); + if (agg_configs.is_aggregated) { + ext.add("source_app", app_id); + } + response["x-medkit"] = ext.build(); + + HandlerContext::send_json(res, response); + return true; + } + + // Determine error type std::string error_code; httplib::StatusCode status_code; if (result.error_message.find("read-only") != std::string::npos || @@ -285,20 +410,53 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http error_code = ERR_INVALID_REQUEST; } HandlerContext::send_error(res, status_code, error_code, "Failed to set parameter", - {{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_name}}); + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + return false; + }; + + // If targeting specific app in aggregated entity + if (!target_app_id.empty()) { + for (const auto & node_info : agg_configs.nodes) { + if (node_info.app_id == target_app_id) { + auto result = config_mgr->set_parameter(node_info.node_fqn, param_name, value); + handle_set_result(result, node_info.node_fqn, target_app_id); + return; + } + } + // App not found in this entity's children + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); + return; + } + + // For non-aggregated or no prefix: use first node (or find matching param) + if (!agg_configs.is_aggregated && !agg_configs.nodes.empty()) { + const auto & node_info = agg_configs.nodes[0]; + auto result = config_mgr->set_parameter(node_info.node_fqn, param_name, value); + handle_set_result(result, node_info.node_fqn, node_info.app_id); + return; } + + // For aggregated configs without prefix, we don't know which node to target + HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_REQUEST, + "Aggregated configuration requires app_id prefix", + {{"details", "Use format 'app_id:param_name' for aggregated configurations"}, + {"entity_id", entity_id}, + {"id", param_id}}); + } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, "Failed to set configuration", - {{"details", e.what()}, {"entity_id", entity_id}, {"param_name", param_name}}); + {{"details", e.what()}, {"entity_id", entity_id}, {"param_id", param_id}}); RCLCPP_ERROR(HandlerContext::logger(), "Error in handle_set_configuration for entity '%s', param '%s': %s", - entity_id.c_str(), param_name.c_str(), e.what()); + entity_id.c_str(), param_id.c_str(), e.what()); } } void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, httplib::Response & res) { std::string entity_id; - std::string param_name; + std::string param_id; try { if (req.matches.size() < 3) { @@ -307,7 +465,7 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h } entity_id = req.matches[1]; - param_name = req.matches[2]; + param_id = req.matches[2]; auto entity_validation = ctx_.validate_entity_id(entity_id); if (!entity_validation) { @@ -324,27 +482,75 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h return; } - // Get node name for parameter access - std::string node_name = entity_info.fqn; - if (node_name.empty()) { - node_name = "/" + entity_id; + // Get aggregated configurations info + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto agg_configs = cache.get_entity_configurations(entity_id); + + if (agg_configs.nodes.empty()) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "No nodes available", + json{{"entity_id", entity_id}, {"id", param_id}}); + return; } auto config_mgr = ctx_.node()->get_configuration_manager(); - auto result = config_mgr->reset_parameter(node_name, param_name); - if (result.success) { - // SOVD compliance: DELETE returns 204 No Content on success - res.status = StatusCode::NoContent_204; - } else { - HandlerContext::send_error( - res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, "Failed to reset parameter", - {{"details", result.error_message}, {"node_name", node_name}, {"param_name", param_name}}); + // Parse param_id for app_id prefix (for aggregated configs: "app_id:param_name") + std::string target_app_id; + std::string param_name = param_id; + auto colon_pos = param_id.find(':'); + if (colon_pos != std::string::npos && agg_configs.is_aggregated) { + target_app_id = param_id.substr(0, colon_pos); + param_name = param_id.substr(colon_pos + 1); + } + + // If targeting specific app in aggregated entity + if (!target_app_id.empty()) { + for (const auto & node_info : agg_configs.nodes) { + if (node_info.app_id == target_app_id) { + auto result = config_mgr->reset_parameter(node_info.node_fqn, param_name); + if (result.success) { + res.status = StatusCode::NoContent_204; + } else { + HandlerContext::send_error( + res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, + "Failed to reset parameter", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + } + return; + } + } + // App not found in this entity's children + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); + return; + } + + // For non-aggregated: use first node + if (!agg_configs.is_aggregated && !agg_configs.nodes.empty()) { + const auto & node_info = agg_configs.nodes[0]; + auto result = config_mgr->reset_parameter(node_info.node_fqn, param_name); + if (result.success) { + res.status = StatusCode::NoContent_204; + } else { + HandlerContext::send_error(res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, + "Failed to reset parameter", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + } + return; } + + // For aggregated configs without prefix, we don't know which node to target + HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_REQUEST, + "Aggregated configuration requires app_id prefix", + json{{"details", "Use format 'app_id:param_name' for aggregated configurations"}, + {"entity_id", entity_id}, + {"id", param_id}}); + } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, "Failed to reset configuration", - {{"details", e.what()}, {"entity_id", entity_id}, {"param_name", param_name}}); + {{"details", e.what()}, {"entity_id", entity_id}, {"param_id", param_id}}); RCLCPP_ERROR(HandlerContext::logger(), "Error in handle_delete_configuration: %s", e.what()); } } @@ -375,22 +581,53 @@ void ConfigHandlers::handle_delete_all_configurations(const httplib::Request & r return; } - // Get node name for parameter access - std::string node_name = entity_info.fqn; - if (node_name.empty()) { - node_name = "/" + entity_id; + // Get aggregated configurations info + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto agg_configs = cache.get_entity_configurations(entity_id); + + if (agg_configs.nodes.empty()) { + // No nodes means nothing to reset, success + res.status = StatusCode::NoContent_204; + return; } auto config_mgr = ctx_.node()->get_configuration_manager(); - auto result = config_mgr->reset_all_parameters(node_name); + bool all_success = true; + json multi_status = json::array(); + + // Reset all parameters on all nodes + for (const auto & node_info : agg_configs.nodes) { + auto result = config_mgr->reset_all_parameters(node_info.node_fqn); + if (!result.success) { + all_success = false; + json status_entry; + status_entry["node"] = node_info.node_fqn; + status_entry["app_id"] = node_info.app_id; + status_entry["success"] = false; + status_entry["error"] = result.error_message; + multi_status.push_back(status_entry); + } else { + json status_entry; + status_entry["node"] = node_info.node_fqn; + status_entry["app_id"] = node_info.app_id; + status_entry["success"] = true; + if (result.data.is_object() || result.data.is_array()) { + status_entry["details"] = result.data; + } + multi_status.push_back(status_entry); + } + } - if (result.success) { + if (all_success) { // SOVD compliance: DELETE returns 204 No Content on complete success res.status = StatusCode::NoContent_204; } else { - // Partial success - some parameters were reset, return 207 Multi-Status + // Partial success - return 207 Multi-Status + json response; + response["entity_id"] = entity_id; + response["results"] = multi_status; res.status = StatusCode::MultiStatus_207; - res.set_content(result.data.dump(2), "application/json"); + res.set_content(response.dump(2), "application/json"); } } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, diff --git a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp index 1f986f5..8a07b84 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp @@ -44,48 +44,116 @@ void HealthHandlers::handle_root(const httplib::Request & req, httplib::Response try { json endpoints = json::array({ + // Health & Discovery "GET /api/v1/health", "GET /api/v1/version-info", + // Areas "GET /api/v1/areas", "GET /api/v1/areas/{area_id}", "GET /api/v1/areas/{area_id}/subareas", "GET /api/v1/areas/{area_id}/components", "GET /api/v1/areas/{area_id}/contains", - "GET /api/v1/areas/{area_id}/related-components", + "GET /api/v1/areas/{area_id}/data", + "GET /api/v1/areas/{area_id}/data/{data_id}", + "PUT /api/v1/areas/{area_id}/data/{data_id}", + "GET /api/v1/areas/{area_id}/operations", + "GET /api/v1/areas/{area_id}/operations/{operation_id}", + "POST /api/v1/areas/{area_id}/operations/{operation_id}/executions", + "GET /api/v1/areas/{area_id}/operations/{operation_id}/executions", + "GET /api/v1/areas/{area_id}/operations/{operation_id}/executions/{execution_id}", + "PUT /api/v1/areas/{area_id}/operations/{operation_id}/executions/{execution_id}", + "DELETE /api/v1/areas/{area_id}/operations/{operation_id}/executions/{execution_id}", + "GET /api/v1/areas/{area_id}/configurations", + "GET /api/v1/areas/{area_id}/configurations/{param_name}", + "PUT /api/v1/areas/{area_id}/configurations/{param_name}", + "DELETE /api/v1/areas/{area_id}/configurations/{param_name}", + "DELETE /api/v1/areas/{area_id}/configurations", + "GET /api/v1/areas/{area_id}/faults", + "GET /api/v1/areas/{area_id}/faults/{fault_code}", + "DELETE /api/v1/areas/{area_id}/faults/{fault_code}", + "DELETE /api/v1/areas/{area_id}/faults", + "GET /api/v1/areas/{area_id}/faults/{fault_code}/snapshots", + // Components "GET /api/v1/components", "GET /api/v1/components/{component_id}", "GET /api/v1/components/{component_id}/subcomponents", "GET /api/v1/components/{component_id}/hosts", - "GET /api/v1/components/{component_id}/related-apps", "GET /api/v1/components/{component_id}/depends-on", "GET /api/v1/components/{component_id}/data", - "GET /api/v1/components/{component_id}/data/{topic_name}", - "PUT /api/v1/components/{component_id}/data/{topic_name}", + "GET /api/v1/components/{component_id}/data/{data_id}", + "PUT /api/v1/components/{component_id}/data/{data_id}", "GET /api/v1/components/{component_id}/operations", "GET /api/v1/components/{component_id}/operations/{operation_id}", "POST /api/v1/components/{component_id}/operations/{operation_id}/executions", "GET /api/v1/components/{component_id}/operations/{operation_id}/executions", "GET /api/v1/components/{component_id}/operations/{operation_id}/executions/{execution_id}", + "PUT /api/v1/components/{component_id}/operations/{operation_id}/executions/{execution_id}", "DELETE /api/v1/components/{component_id}/operations/{operation_id}/executions/{execution_id}", "GET /api/v1/components/{component_id}/configurations", "GET /api/v1/components/{component_id}/configurations/{param_name}", "PUT /api/v1/components/{component_id}/configurations/{param_name}", + "DELETE /api/v1/components/{component_id}/configurations/{param_name}", + "DELETE /api/v1/components/{component_id}/configurations", + "GET /api/v1/components/{component_id}/faults", + "GET /api/v1/components/{component_id}/faults/{fault_code}", + "DELETE /api/v1/components/{component_id}/faults/{fault_code}", + "DELETE /api/v1/components/{component_id}/faults", + "GET /api/v1/components/{component_id}/faults/{fault_code}/snapshots", + // Apps "GET /api/v1/apps", "GET /api/v1/apps/{app_id}", "GET /api/v1/apps/{app_id}/depends-on", "GET /api/v1/apps/{app_id}/data", + "GET /api/v1/apps/{app_id}/data/{data_id}", + "PUT /api/v1/apps/{app_id}/data/{data_id}", + "GET /api/v1/apps/{app_id}/data-categories", + "GET /api/v1/apps/{app_id}/data-groups", "GET /api/v1/apps/{app_id}/operations", + "GET /api/v1/apps/{app_id}/operations/{operation_id}", + "POST /api/v1/apps/{app_id}/operations/{operation_id}/executions", + "GET /api/v1/apps/{app_id}/operations/{operation_id}/executions", + "GET /api/v1/apps/{app_id}/operations/{operation_id}/executions/{execution_id}", + "PUT /api/v1/apps/{app_id}/operations/{operation_id}/executions/{execution_id}", + "DELETE /api/v1/apps/{app_id}/operations/{operation_id}/executions/{execution_id}", "GET /api/v1/apps/{app_id}/configurations", + "GET /api/v1/apps/{app_id}/configurations/{param_name}", + "PUT /api/v1/apps/{app_id}/configurations/{param_name}", + "DELETE /api/v1/apps/{app_id}/configurations/{param_name}", + "DELETE /api/v1/apps/{app_id}/configurations", + "GET /api/v1/apps/{app_id}/faults", + "GET /api/v1/apps/{app_id}/faults/{fault_code}", + "DELETE /api/v1/apps/{app_id}/faults/{fault_code}", + "DELETE /api/v1/apps/{app_id}/faults", + "GET /api/v1/apps/{app_id}/faults/{fault_code}/snapshots", + // Functions "GET /api/v1/functions", "GET /api/v1/functions/{function_id}", "GET /api/v1/functions/{function_id}/hosts", + "GET /api/v1/functions/{function_id}/data", + "GET /api/v1/functions/{function_id}/data/{data_id}", + "PUT /api/v1/functions/{function_id}/data/{data_id}", + "GET /api/v1/functions/{function_id}/operations", + "GET /api/v1/functions/{function_id}/operations/{operation_id}", + "POST /api/v1/functions/{function_id}/operations/{operation_id}/executions", + "GET /api/v1/functions/{function_id}/operations/{operation_id}/executions", + "GET /api/v1/functions/{function_id}/operations/{operation_id}/executions/{execution_id}", + "PUT /api/v1/functions/{function_id}/operations/{operation_id}/executions/{execution_id}", + "DELETE /api/v1/functions/{function_id}/operations/{operation_id}/executions/{execution_id}", + "GET /api/v1/functions/{function_id}/configurations", + "GET /api/v1/functions/{function_id}/configurations/{param_name}", + "PUT /api/v1/functions/{function_id}/configurations/{param_name}", + "DELETE /api/v1/functions/{function_id}/configurations/{param_name}", + "DELETE /api/v1/functions/{function_id}/configurations", + "GET /api/v1/functions/{function_id}/faults", + "GET /api/v1/functions/{function_id}/faults/{fault_code}", + "DELETE /api/v1/functions/{function_id}/faults/{fault_code}", + "DELETE /api/v1/functions/{function_id}/faults", + "GET /api/v1/functions/{function_id}/faults/{fault_code}/snapshots", + // Global Faults "GET /api/v1/faults", - "GET /api/v1/components/{component_id}/faults", - "GET /api/v1/components/{component_id}/faults/{fault_code}", - "DELETE /api/v1/components/{component_id}/faults/{fault_code}", + "GET /api/v1/faults/stream", "GET /api/v1/faults/{fault_code}/snapshots", "GET /api/v1/faults/{fault_code}/snapshots/bag", - "GET /api/v1/components/{component_id}/faults/{fault_code}/snapshots", }); const auto & auth_config = ctx_.auth_config(); diff --git a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp index d53c72f..f1c906d 100644 --- a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp +++ b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp @@ -380,6 +380,176 @@ AggregatedOperations ThreadSafeEntityCache::get_function_operations(const std::s return result; } +// ============================================================================ +// Configuration aggregation methods +// ============================================================================ + +AggregatedConfigurations ThreadSafeEntityCache::get_entity_configurations(const std::string & entity_id) const { + // Find entity by ID - O(1) lookups in each index + auto entity = find_entity(entity_id); + if (!entity) { + return {}; + } + + switch (entity->type) { + case SovdEntityType::APP: + return get_app_configurations(entity_id); + case SovdEntityType::COMPONENT: + return get_component_configurations(entity_id); + case SovdEntityType::AREA: + return get_area_configurations(entity_id); + case SovdEntityType::FUNCTION: + return get_function_configurations(entity_id); + default: + return {}; + } +} + +AggregatedConfigurations ThreadSafeEntityCache::get_app_configurations(const std::string & app_id) const { + std::shared_lock lock(mutex_); + AggregatedConfigurations result; + result.aggregation_level = "app"; + result.is_aggregated = false; + + auto it = app_index_.find(app_id); + if (it == app_index_.end() || it->second >= apps_.size()) { + return result; + } + + const auto & app = apps_[it->second]; + + // App must have a bound FQN to have parameters + if (app.bound_fqn.has_value() && !app.bound_fqn->empty()) { + NodeConfigInfo info; + info.node_fqn = *app.bound_fqn; + info.app_id = app_id; + info.entity_id = app_id; + result.nodes.push_back(info); + } + + result.source_ids.push_back(app_id); + return result; +} + +AggregatedConfigurations ThreadSafeEntityCache::get_component_configurations(const std::string & component_id) const { + std::shared_lock lock(mutex_); + AggregatedConfigurations result; + result.aggregation_level = "component"; + + auto comp_it = component_index_.find(component_id); + if (comp_it == component_index_.end() || comp_it->second >= components_.size()) { + return result; + } + + result.source_ids.push_back(component_id); + + // Collect node FQNs from hosted apps + auto apps_it = component_to_apps_.find(component_id); + if (apps_it != component_to_apps_.end()) { + for (size_t app_idx : apps_it->second) { + if (app_idx >= apps_.size()) { + continue; + } + const auto & app = apps_[app_idx]; + if (app.bound_fqn.has_value() && !app.bound_fqn->empty()) { + NodeConfigInfo info; + info.node_fqn = *app.bound_fqn; + info.app_id = app.id; + info.entity_id = component_id; + result.nodes.push_back(info); + result.source_ids.push_back(app.id); + } + } + if (!apps_it->second.empty()) { + result.is_aggregated = true; + } + } + + return result; +} + +AggregatedConfigurations ThreadSafeEntityCache::get_area_configurations(const std::string & area_id) const { + std::shared_lock lock(mutex_); + AggregatedConfigurations result; + result.aggregation_level = "area"; + result.is_aggregated = true; // Area configurations are always aggregated + + auto area_it = area_index_.find(area_id); + if (area_it == area_index_.end()) { + return result; + } + + result.source_ids.push_back(area_id); + + // Get all components in this area + auto comps_it = area_to_components_.find(area_id); + if (comps_it != area_to_components_.end()) { + for (size_t comp_idx : comps_it->second) { + if (comp_idx >= components_.size()) { + continue; + } + + const auto & comp = components_[comp_idx]; + result.source_ids.push_back(comp.id); + + // Add node FQNs from component's apps + auto apps_it = component_to_apps_.find(comp.id); + if (apps_it != component_to_apps_.end()) { + for (size_t app_idx : apps_it->second) { + if (app_idx >= apps_.size()) { + continue; + } + const auto & app = apps_[app_idx]; + if (app.bound_fqn.has_value() && !app.bound_fqn->empty()) { + NodeConfigInfo info; + info.node_fqn = *app.bound_fqn; + info.app_id = app.id; + info.entity_id = area_id; + result.nodes.push_back(info); + } + } + } + } + } + + return result; +} + +AggregatedConfigurations ThreadSafeEntityCache::get_function_configurations(const std::string & function_id) const { + std::shared_lock lock(mutex_); + AggregatedConfigurations result; + result.aggregation_level = "function"; + result.is_aggregated = true; // Function configurations are always aggregated + + auto func_it = function_index_.find(function_id); + if (func_it == function_index_.end()) { + return result; + } + + result.source_ids.push_back(function_id); + + // Get all apps implementing this function + auto apps_it = function_to_apps_.find(function_id); + if (apps_it != function_to_apps_.end()) { + for (size_t app_idx : apps_it->second) { + if (app_idx >= apps_.size()) { + continue; + } + const auto & app = apps_[app_idx]; + if (app.bound_fqn.has_value() && !app.bound_fqn->empty()) { + NodeConfigInfo info; + info.node_fqn = *app.bound_fqn; + info.app_id = app.id; + info.entity_id = function_id; + result.nodes.push_back(info); + result.source_ids.push_back(app.id); + } + } + } + + return result; +} + // ============================================================================ // Operation lookup // ============================================================================ diff --git a/src/ros2_medkit_gateway/test/test_discovery_manifest.test.py b/src/ros2_medkit_gateway/test/test_discovery_manifest.test.py index d95372a..f1d5c3d 100644 --- a/src/ros2_medkit_gateway/test/test_discovery_manifest.test.py +++ b/src/ros2_medkit_gateway/test/test_discovery_manifest.test.py @@ -394,15 +394,21 @@ def test_app_configurations_endpoint(self): """ Test GET /apps/{id}/configurations in manifest-only mode. - App is defined in manifest. Configurations require communication with - ROS 2 parameter service on the node. In manifest-only mode without - running nodes, returns 503 Service Unavailable. + App is defined in manifest with bound_fqn. Configurations are retrieved + from the ROS 2 parameter service on the node. When the node is running, + returns 200 with the list of parameters. + + @verifies REQ_INTEROP_003 """ response = requests.get( f'{self.BASE_URL}/apps/lidar-sensor/configurations', timeout=5 ) - # Configurations require node to be running - 503 in manifest-only mode - self.assertEqual(response.status_code, 503) + # App has bound_fqn and node is running - returns configurations + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertIn('items', data) + # x-medkit extension should include aggregation info + self.assertIn('x-medkit', data) def test_app_data_item_endpoint(self): """ diff --git a/src/ros2_medkit_gateway/test/test_integration.test.py b/src/ros2_medkit_gateway/test/test_integration.test.py index 57a9d61..8e45970 100644 --- a/src/ros2_medkit_gateway/test/test_integration.test.py +++ b/src/ros2_medkit_gateway/test/test_integration.test.py @@ -468,7 +468,7 @@ def test_01_root_endpoint(self): self.assertIn('GET /api/v1/areas', data['endpoints']) self.assertIn('GET /api/v1/components', data['endpoints']) self.assertIn( - 'PUT /api/v1/components/{component_id}/data/{topic_name}', data['endpoints'] + 'PUT /api/v1/components/{component_id}/data/{data_id}', data['endpoints'] ) # Verify api_base field @@ -504,6 +504,97 @@ def test_01b_version_info_endpoint(self): self.assertEqual(info['vendor_info']['name'], 'ros2_medkit') print('✓ Version info endpoint test passed') + def test_01c_endpoint_smoke_test(self): + """ + Smoke test: verify all advertised GET endpoints are implemented and don't return 5xx. + + This test ensures that: + 1. All endpoints listed in GET / are actually implemented + 2. No endpoint returns a server error (5xx) + 3. Documentation in handle_root matches actual implementation + + Only GET endpoints are tested (safe, read-only operations). + POST/PUT/DELETE endpoints are skipped as they modify state. + + @verifies REQ_INTEROP_010 + """ + # Get all advertised endpoints + data = self._get_json('/') + endpoints = data.get('endpoints', []) + self.assertIsInstance(endpoints, list) + self.assertGreater(len(endpoints), 0, 'No endpoints returned from /') + + # Test substitution values for path parameters + # These use known entities from demo_nodes.launch.py + substitutions = { + '{area_id}': 'powertrain', + '{component_id}': 'powertrain', + '{app_id}': 'temp_sensor', + '{function_id}': 'nonexistent_function', # Functions not used in demo + '{data_id}': 'temperature', + '{topic_name}': 'temperature', + '{operation_id}': 'nonexistent_op', # Will return 404, but not 5xx + '{execution_id}': 'nonexistent_exec', + '{param_name}': 'use_sim_time', + '{fault_code}': 'nonexistent_fault', + } + + # Filter for GET endpoints only (safe to call) + get_endpoints = [ep for ep in endpoints if ep.startswith('GET ')] + + tested_count = 0 + errors = [] + + for endpoint in get_endpoints: + # Parse method and path + parts = endpoint.split(' ', 1) + if len(parts) != 2: + continue + method, path = parts + + # Skip auth endpoints (require authentication) + if '/auth/' in path: + continue + + # Skip SSE stream endpoint (keeps connection open) + if path.endswith('/stream'): + continue + + # Substitute path parameters + test_path = path + for placeholder, value in substitutions.items(): + test_path = test_path.replace(placeholder, value) + + # Remove the /api/v1 prefix if present (BASE_URL already has it) + if test_path.startswith('/api/v1'): + test_path = test_path[7:] # len('/api/v1') = 7 + + try: + url = f'{self.BASE_URL}{test_path}' + response = requests.get(url, timeout=10) + + # Check for server errors (5xx) + # 501 Not Implemented is acceptable (intentional for unimplemented features) + if response.status_code >= 500: + # Skip known limitations + if response.status_code == 501: + pass # Intentional - feature not implemented for ROS 2 + else: + errors.append( + f'{endpoint} -> {test_path}: returned {response.status_code}' + ) + tested_count += 1 + + except requests.exceptions.RequestException as e: + errors.append(f'{endpoint} -> {test_path}: request failed - {e}') + + # Report results + print(f'✓ Smoke test: {tested_count} GET endpoints tested') + + if errors: + error_msg = '\n'.join(errors) + self.fail(f'Server errors detected:\n{error_msg}') + def test_02_list_areas(self): """ Test GET /areas returns all discovered areas. From f4b813530c93aa3cd48bbfcdd932010fd5a05b19 Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 27 Jan 2026 17:07:32 +0000 Subject: [PATCH 2/3] refactor(gateway): extract config handler helpers and fix error handling - Add helper functions to eliminate code duplication: - parse_aggregated_param_id(): parses 'app_id:param_name' format - find_node_for_app(): O(n) lookup replacing duplicated loops - classify_parameter_error(): consistent error classification (404/403/503/400) - Fix error handling in aggregated entity GET without prefix: - Track errors across all nodes instead of silently ignoring failures - Return 404 only when all nodes report 'not found' - Return appropriate error (503/400) when nodes have other failures - Fix get_area_configurations: add missing app IDs to source_ids Addresses code review feedback on inconsistent error semantics. --- .../src/http/handlers/config_handlers.cpp | 346 ++++++++++-------- .../src/models/thread_safe_entity_cache.cpp | 1 + 2 files changed, 203 insertions(+), 144 deletions(-) diff --git a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp index 9ec9b4e..2fda9d5 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -24,6 +24,109 @@ using httplib::StatusCode; namespace ros2_medkit_gateway { namespace handlers { +// ============================================================================ +// Helper structures and functions for configuration handlers +// ============================================================================ + +/** + * @brief Parsed parameter ID with optional app prefix + */ +struct ParsedParamId { + std::string app_id; ///< Target app ID (empty if not prefixed) + std::string param_name; ///< Parameter name + bool has_prefix{false}; ///< Whether the ID had an app prefix +}; + +/** + * @brief Parse param_id which may contain "app_id:param_name" format + * + * For aggregated configurations, parameter IDs are prefixed with the source + * app ID to disambiguate parameters with the same name across nodes. + * + * @param param_id The parameter ID to parse + * @param is_aggregated Whether the entity is aggregated + * @return ParsedParamId with separated app_id and param_name + */ +static ParsedParamId parse_aggregated_param_id(const std::string & param_id, bool is_aggregated) { + ParsedParamId result; + result.param_name = param_id; + + auto colon_pos = param_id.find(':'); + if (colon_pos != std::string::npos && is_aggregated) { + result.app_id = param_id.substr(0, colon_pos); + result.param_name = param_id.substr(colon_pos + 1); + result.has_prefix = true; + } + + return result; +} + +/** + * @brief Find node info for a specific app ID in aggregated configurations + * + * @param nodes Vector of NodeConfigInfo to search + * @param app_id Target app ID + * @return Pointer to NodeConfigInfo if found, nullptr otherwise + */ +static const NodeConfigInfo * find_node_for_app(const std::vector & nodes, const std::string & app_id) { + for (const auto & node : nodes) { + if (node.app_id == app_id) { + return &node; + } + } + return nullptr; +} + +/** + * @brief Error classification result for parameter operations + */ +struct ErrorClassification { + httplib::StatusCode status_code; + std::string error_code; +}; + +/** + * @brief Classify error message to appropriate HTTP status and error code + * + * Analyzes the error message to determine if it's: + * - 404 Not Found (parameter doesn't exist) + * - 403 Forbidden (read-only parameter) + * - 503 Service Unavailable (node/service not reachable) + * - 400 Bad Request (other errors) + * + * @param error_message The error message to classify + * @return ErrorClassification with status code and error code + */ +static ErrorClassification classify_parameter_error(const std::string & error_message) { + ErrorClassification result; + + if (error_message.find("not found") != std::string::npos || + error_message.find("Parameter not found") != std::string::npos) { + result.status_code = StatusCode::NotFound_404; + result.error_code = ERR_RESOURCE_NOT_FOUND; + } else if (error_message.find("read-only") != std::string::npos || + error_message.find("read only") != std::string::npos || + error_message.find("is read_only") != std::string::npos) { + result.status_code = StatusCode::Forbidden_403; + result.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY; + } else if (error_message.find("not available") != std::string::npos || + error_message.find("service not available") != std::string::npos || + error_message.find("timed out") != std::string::npos || + error_message.find("timeout") != std::string::npos) { + result.status_code = StatusCode::ServiceUnavailable_503; + result.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE; + } else { + result.status_code = StatusCode::BadRequest_400; + result.error_code = ERR_INVALID_REQUEST; + } + + return result; +} + +// ============================================================================ +// Handler implementations +// ============================================================================ + void ConfigHandlers::handle_list_configurations(const httplib::Request & req, httplib::Response & res) { std::string entity_id; try { @@ -193,75 +296,55 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http auto config_mgr = ctx_.node()->get_configuration_manager(); - // Check if param_id contains app_id prefix (for aggregated configs: "app_id:param_name") - std::string target_app_id; - std::string param_name = param_id; - auto colon_pos = param_id.find(':'); - if (colon_pos != std::string::npos && agg_configs.is_aggregated) { - target_app_id = param_id.substr(0, colon_pos); - param_name = param_id.substr(colon_pos + 1); - } + // Parse param_id for app_id prefix + auto parsed = parse_aggregated_param_id(param_id, agg_configs.is_aggregated); // If targeting specific app in aggregated entity - if (!target_app_id.empty()) { - for (const auto & node_info : agg_configs.nodes) { - if (node_info.app_id == target_app_id) { - auto result = config_mgr->get_parameter(node_info.node_fqn, param_name); - - if (result.success) { - json response; - response["id"] = param_id; - - if (result.data.contains("value")) { - response["data"] = result.data["value"]; - } else { - response["data"] = result.data; - } + if (parsed.has_prefix) { + const auto * node_info = find_node_for_app(agg_configs.nodes, parsed.app_id); + if (!node_info) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", parsed.app_id}}); + return; + } - XMedkit ext; - ext.ros2_node(node_info.node_fqn).entity_id(entity_id).source("runtime"); - ext.add("parameter", result.data); - ext.add("source_app", target_app_id); - response["x-medkit"] = ext.build(); - - HandlerContext::send_json(res, response); - return; - } else { - if (result.error_message.find("not found") != std::string::npos || - result.error_message.find("Parameter not found") != std::string::npos) { - HandlerContext::send_error( - res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); - } else { - HandlerContext::send_error( - res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to get parameter", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); - } - return; - } - } + auto result = config_mgr->get_parameter(node_info->node_fqn, parsed.param_name); + + if (result.success) { + json response; + response["id"] = param_id; + response["data"] = result.data.contains("value") ? result.data["value"] : result.data; + + XMedkit ext; + ext.ros2_node(node_info->node_fqn).entity_id(entity_id).source("runtime"); + ext.add("parameter", result.data); + ext.add("source_app", parsed.app_id); + response["x-medkit"] = ext.build(); + + HandlerContext::send_json(res, response); + } else { + auto err = classify_parameter_error(result.error_message); + HandlerContext::send_error(res, err.status_code, err.error_code, + err.status_code == StatusCode::NotFound_404 ? "Parameter not found" + : "Failed to get parameter", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); } - // App not found in this entity's children - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, - "Source app not found in entity", - json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); return; } // For non-aggregated or no prefix: search all nodes for the parameter + // Track errors to provide meaningful response if parameter not found anywhere + std::string last_error; + bool all_not_found = true; // Track if all failures are "not found" vs other errors + for (const auto & node_info : agg_configs.nodes) { - auto result = config_mgr->get_parameter(node_info.node_fqn, param_name); + auto result = config_mgr->get_parameter(node_info.node_fqn, parsed.param_name); if (result.success) { json response; - response["id"] = param_name; - - if (result.data.contains("value")) { - response["data"] = result.data["value"]; - } else { - response["data"] = result.data; - } + response["id"] = parsed.param_name; + response["data"] = result.data.contains("value") ? result.data["value"] : result.data; XMedkit ext; ext.ros2_node(node_info.node_fqn).entity_id(entity_id).source("runtime"); @@ -274,11 +357,25 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http HandlerContext::send_json(res, response); return; } + + // Track the error for later reporting + last_error = result.error_message; + auto err = classify_parameter_error(result.error_message); + if (err.status_code != StatusCode::NotFound_404) { + all_not_found = false; + } } - // Parameter not found in any node - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", - {{"entity_id", entity_id}, {"id", param_id}}); + // Parameter not found in any node - report appropriate error + if (all_not_found) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, "Parameter not found", + json{{"entity_id", entity_id}, {"id", param_id}}); + } else { + // Some nodes had non-"not found" errors (e.g., unavailable) - report 503 + auto err = classify_parameter_error(last_error); + HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to get parameter from any node", + json{{"details", last_error}, {"entity_id", entity_id}, {"id", param_id}}); + } } catch (const std::exception & e) { HandlerContext::send_error(res, StatusCode::InternalServerError_500, ERR_INTERNAL_ERROR, @@ -356,26 +453,15 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http auto config_mgr = ctx_.node()->get_configuration_manager(); - // Parse param_id for app_id prefix (for aggregated configs: "app_id:param_name") - std::string target_app_id; - std::string param_name = param_id; - auto colon_pos = param_id.find(':'); - if (colon_pos != std::string::npos && agg_configs.is_aggregated) { - target_app_id = param_id.substr(0, colon_pos); - param_name = param_id.substr(colon_pos + 1); - } + // Parse param_id for app_id prefix + auto parsed = parse_aggregated_param_id(param_id, agg_configs.is_aggregated); - // Helper to handle set result + // Helper to handle set result and send response auto handle_set_result = [&](const auto & result, const std::string & node_fqn, const std::string & app_id) { if (result.success) { json response; response["id"] = param_id; - - if (result.data.contains("value")) { - response["data"] = result.data["value"]; - } else { - response["data"] = result.data; - } + response["data"] = result.data.contains("value") ? result.data["value"] : result.data; XMedkit ext; ext.ros2_node(node_fqn).entity_id(entity_id).source("runtime"); @@ -389,51 +475,31 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return true; } - // Determine error type - std::string error_code; - httplib::StatusCode status_code; - if (result.error_message.find("read-only") != std::string::npos || - result.error_message.find("read only") != std::string::npos || - result.error_message.find("is read_only") != std::string::npos) { - status_code = StatusCode::Forbidden_403; - error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY; - } else if (result.error_message.find("not found") != std::string::npos || - result.error_message.find("Parameter not found") != std::string::npos) { - status_code = StatusCode::NotFound_404; - error_code = ERR_RESOURCE_NOT_FOUND; - } else if (result.error_message.find("not available") != std::string::npos || - result.error_message.find("service not available") != std::string::npos) { - status_code = StatusCode::ServiceUnavailable_503; - error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE; - } else { - status_code = StatusCode::BadRequest_400; - error_code = ERR_INVALID_REQUEST; - } - HandlerContext::send_error(res, status_code, error_code, "Failed to set parameter", + auto err = classify_parameter_error(result.error_message); + HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to set parameter", json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); return false; }; // If targeting specific app in aggregated entity - if (!target_app_id.empty()) { - for (const auto & node_info : agg_configs.nodes) { - if (node_info.app_id == target_app_id) { - auto result = config_mgr->set_parameter(node_info.node_fqn, param_name, value); - handle_set_result(result, node_info.node_fqn, target_app_id); - return; - } + if (parsed.has_prefix) { + const auto * node_info = find_node_for_app(agg_configs.nodes, parsed.app_id); + if (!node_info) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", parsed.app_id}}); + return; } - // App not found in this entity's children - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, - "Source app not found in entity", - json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); + + auto result = config_mgr->set_parameter(node_info->node_fqn, parsed.param_name, value); + handle_set_result(result, node_info->node_fqn, parsed.app_id); return; } - // For non-aggregated or no prefix: use first node (or find matching param) + // For non-aggregated: use the single node if (!agg_configs.is_aggregated && !agg_configs.nodes.empty()) { const auto & node_info = agg_configs.nodes[0]; - auto result = config_mgr->set_parameter(node_info.node_fqn, param_name, value); + auto result = config_mgr->set_parameter(node_info.node_fqn, parsed.param_name, value); handle_set_result(result, node_info.node_fqn, node_info.app_id); return; } @@ -494,49 +560,41 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h auto config_mgr = ctx_.node()->get_configuration_manager(); - // Parse param_id for app_id prefix (for aggregated configs: "app_id:param_name") - std::string target_app_id; - std::string param_name = param_id; - auto colon_pos = param_id.find(':'); - if (colon_pos != std::string::npos && agg_configs.is_aggregated) { - target_app_id = param_id.substr(0, colon_pos); - param_name = param_id.substr(colon_pos + 1); - } + // Parse param_id for app_id prefix + auto parsed = parse_aggregated_param_id(param_id, agg_configs.is_aggregated); + + // Helper to handle reset result + auto handle_reset_result = [&](const auto & result) { + if (result.success) { + res.status = StatusCode::NoContent_204; + return true; + } + auto err = classify_parameter_error(result.error_message); + HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to reset parameter", + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + return false; + }; // If targeting specific app in aggregated entity - if (!target_app_id.empty()) { - for (const auto & node_info : agg_configs.nodes) { - if (node_info.app_id == target_app_id) { - auto result = config_mgr->reset_parameter(node_info.node_fqn, param_name); - if (result.success) { - res.status = StatusCode::NoContent_204; - } else { - HandlerContext::send_error( - res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to reset parameter", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); - } - return; - } + if (parsed.has_prefix) { + const auto * node_info = find_node_for_app(agg_configs.nodes, parsed.app_id); + if (!node_info) { + HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, + "Source app not found in entity", + json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", parsed.app_id}}); + return; } - // App not found in this entity's children - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_RESOURCE_NOT_FOUND, - "Source app not found in entity", - json{{"entity_id", entity_id}, {"id", param_id}, {"source_app", target_app_id}}); + + auto result = config_mgr->reset_parameter(node_info->node_fqn, parsed.param_name); + handle_reset_result(result); return; } - // For non-aggregated: use first node + // For non-aggregated: use the single node if (!agg_configs.is_aggregated && !agg_configs.nodes.empty()) { const auto & node_info = agg_configs.nodes[0]; - auto result = config_mgr->reset_parameter(node_info.node_fqn, param_name); - if (result.success) { - res.status = StatusCode::NoContent_204; - } else { - HandlerContext::send_error(res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to reset parameter", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); - } + auto result = config_mgr->reset_parameter(node_info.node_fqn, parsed.param_name); + handle_reset_result(result); return; } diff --git a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp index f1c906d..fc4b477 100644 --- a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp +++ b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp @@ -506,6 +506,7 @@ AggregatedConfigurations ThreadSafeEntityCache::get_area_configurations(const st info.app_id = app.id; info.entity_id = area_id; result.nodes.push_back(info); + result.source_ids.push_back(app.id); } } } From 025e9af8fb1f894bb767d33277a570f5bed14bcc Mon Sep 17 00:00:00 2001 From: Bartosz Burda Date: Tue, 27 Jan 2026 19:04:55 +0000 Subject: [PATCH 3/3] refactor(gateway): structured error codes and parallel config queries - Add ParameterErrorCode enum for programmatic error handling - Parallelize list_configurations with std::async for large hierarchies - Fix is_aggregated: true only when multiple nodes exist - Unify entity lookup to cache.find_entity() pattern - Add send_parameter_error() helper and MAX_AGGREGATED_PARAM_ID_LENGTH constant --- .../configuration_manager.hpp | 14 ++ .../src/configuration_manager.cpp | 31 +++ .../src/http/handlers/config_handlers.cpp | 226 +++++++++++++----- .../src/models/thread_safe_entity_cache.cpp | 14 +- 4 files changed, 220 insertions(+), 65 deletions(-) diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/configuration_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/configuration_manager.hpp index d99d564..f55289b 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/configuration_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/configuration_manager.hpp @@ -26,11 +26,25 @@ namespace ros2_medkit_gateway { using json = nlohmann::json; +/// Error codes for parameter operations +enum class ParameterErrorCode { + NONE = 0, ///< No error (success) + NOT_FOUND, ///< Parameter does not exist + READ_ONLY, ///< Parameter is read-only and cannot be modified + SERVICE_UNAVAILABLE, ///< Parameter service not available (node unreachable) + TIMEOUT, ///< Operation timed out + TYPE_MISMATCH, ///< Value type doesn't match parameter type + INVALID_VALUE, ///< Invalid value for parameter + NO_DEFAULTS_CACHED, ///< No default values cached for reset operation + INTERNAL_ERROR ///< Internal/unexpected error +}; + /// Result of a parameter operation struct ParameterResult { bool success; json data; std::string error_message; + ParameterErrorCode error_code{ParameterErrorCode::NONE}; ///< Structured error code for programmatic handling }; /// Manager for ROS2 node parameters diff --git a/src/ros2_medkit_gateway/src/configuration_manager.cpp b/src/ros2_medkit_gateway/src/configuration_manager.cpp index dc4062b..a0332a4 100644 --- a/src/ros2_medkit_gateway/src/configuration_manager.cpp +++ b/src/ros2_medkit_gateway/src/configuration_manager.cpp @@ -74,6 +74,7 @@ ParameterResult ConfigurationManager::list_parameters(const std::string & node_n if (!client->wait_for_service(get_service_timeout())) { result.success = false; result.error_message = "Parameter service not available for node: " + node_name; + result.error_code = ParameterErrorCode::SERVICE_UNAVAILABLE; RCLCPP_WARN(node_->get_logger(), "Parameter service not available for node: '%s'", node_name.c_str()); return result; } @@ -129,6 +130,7 @@ ParameterResult ConfigurationManager::list_parameters(const std::string & node_n } catch (const std::exception & e) { result.success = false; result.error_message = std::string("Failed to list parameters: ") + e.what(); + result.error_code = ParameterErrorCode::INTERNAL_ERROR; RCLCPP_ERROR(node_->get_logger(), "Exception in list_parameters for node '%s': %s", node_name.c_str(), e.what()); } @@ -144,6 +146,7 @@ ParameterResult ConfigurationManager::get_parameter(const std::string & node_nam if (!client->wait_for_service(get_service_timeout())) { result.success = false; result.error_message = "Parameter service not available for node: " + node_name; + result.error_code = ParameterErrorCode::SERVICE_UNAVAILABLE; return result; } @@ -152,6 +155,7 @@ ParameterResult ConfigurationManager::get_parameter(const std::string & node_nam if (param_names.names.empty()) { result.success = false; result.error_message = "Parameter not found: " + param_name; + result.error_code = ParameterErrorCode::NOT_FOUND; return result; } @@ -160,6 +164,7 @@ ParameterResult ConfigurationManager::get_parameter(const std::string & node_nam if (parameters.empty()) { result.success = false; result.error_message = "Failed to get parameter: " + param_name; + result.error_code = ParameterErrorCode::INTERNAL_ERROR; return result; } @@ -183,6 +188,7 @@ ParameterResult ConfigurationManager::get_parameter(const std::string & node_nam } catch (const std::exception & e) { result.success = false; result.error_message = std::string("Failed to get parameter: ") + e.what(); + result.error_code = ParameterErrorCode::INTERNAL_ERROR; } return result; @@ -198,6 +204,7 @@ ParameterResult ConfigurationManager::set_parameter(const std::string & node_nam if (!client->wait_for_service(get_service_timeout())) { result.success = false; result.error_message = "Parameter service not available for node: " + node_name; + result.error_code = ParameterErrorCode::SERVICE_UNAVAILABLE; return result; } @@ -217,6 +224,20 @@ ParameterResult ConfigurationManager::set_parameter(const std::string & node_nam if (results.empty() || !results[0].successful) { result.success = false; result.error_message = results.empty() ? "Failed to set parameter" : results[0].reason; + // Classify error based on reason + if (!results.empty()) { + const auto & reason = results[0].reason; + if (reason.find("read-only") != std::string::npos || reason.find("read only") != std::string::npos || + reason.find("is read_only") != std::string::npos) { + result.error_code = ParameterErrorCode::READ_ONLY; + } else if (reason.find("type") != std::string::npos) { + result.error_code = ParameterErrorCode::TYPE_MISMATCH; + } else { + result.error_code = ParameterErrorCode::INVALID_VALUE; + } + } else { + result.error_code = ParameterErrorCode::INTERNAL_ERROR; + } return result; } @@ -231,6 +252,7 @@ ParameterResult ConfigurationManager::set_parameter(const std::string & node_nam } catch (const std::exception & e) { result.success = false; result.error_message = std::string("Failed to set parameter: ") + e.what(); + result.error_code = ParameterErrorCode::INTERNAL_ERROR; } return result; @@ -468,6 +490,7 @@ ParameterResult ConfigurationManager::reset_parameter(const std::string & node_n if (node_it == default_values_.end()) { result.success = false; result.error_message = "No default values cached for node: " + node_name; + result.error_code = ParameterErrorCode::NO_DEFAULTS_CACHED; return result; } @@ -475,6 +498,7 @@ ParameterResult ConfigurationManager::reset_parameter(const std::string & node_n if (param_it == node_it->second.end()) { result.success = false; result.error_message = "No default value for parameter: " + param_name; + result.error_code = ParameterErrorCode::NOT_FOUND; return result; } @@ -485,6 +509,7 @@ ParameterResult ConfigurationManager::reset_parameter(const std::string & node_n if (!client->wait_for_service(get_service_timeout())) { result.success = false; result.error_message = "Parameter service not available for node: " + node_name; + result.error_code = ParameterErrorCode::SERVICE_UNAVAILABLE; return result; } @@ -492,6 +517,7 @@ ParameterResult ConfigurationManager::reset_parameter(const std::string & node_n if (results.empty() || !results[0].successful) { result.success = false; result.error_message = results.empty() ? "Failed to reset parameter" : results[0].reason; + result.error_code = ParameterErrorCode::INTERNAL_ERROR; return result; } @@ -510,6 +536,7 @@ ParameterResult ConfigurationManager::reset_parameter(const std::string & node_n } catch (const std::exception & e) { result.success = false; result.error_message = std::string("Failed to reset parameter: ") + e.what(); + result.error_code = ParameterErrorCode::INTERNAL_ERROR; } return result; @@ -530,6 +557,7 @@ ParameterResult ConfigurationManager::reset_all_parameters(const std::string & n if (node_it == default_values_.end()) { result.success = false; result.error_message = "No default values cached for node: " + node_name; + result.error_code = ParameterErrorCode::NO_DEFAULTS_CACHED; return result; } @@ -537,6 +565,7 @@ ParameterResult ConfigurationManager::reset_all_parameters(const std::string & n if (!client->wait_for_service(get_service_timeout())) { result.success = false; result.error_message = "Parameter service not available for node: " + node_name; + result.error_code = ParameterErrorCode::SERVICE_UNAVAILABLE; return result; } @@ -580,6 +609,7 @@ ParameterResult ConfigurationManager::reset_all_parameters(const std::string & n if (failed_count > 0) { result.error_message = "Some parameters could not be reset"; + result.error_code = ParameterErrorCode::INTERNAL_ERROR; // Partial failure } RCLCPP_INFO(node_->get_logger(), "Reset %zu parameters on node '%s' (%zu failed)", reset_count, node_name.c_str(), @@ -587,6 +617,7 @@ ParameterResult ConfigurationManager::reset_all_parameters(const std::string & n } catch (const std::exception & e) { result.success = false; result.error_message = std::string("Failed to reset parameters: ") + e.what(); + result.error_code = ParameterErrorCode::INTERNAL_ERROR; } return result; diff --git a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp index 2fda9d5..08c71e3 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -14,6 +14,9 @@ #include "ros2_medkit_gateway/http/handlers/config_handlers.hpp" +#include +#include + #include "ros2_medkit_gateway/gateway_node.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" #include "ros2_medkit_gateway/http/x_medkit.hpp" @@ -24,6 +27,20 @@ using httplib::StatusCode; namespace ros2_medkit_gateway { namespace handlers { +// ============================================================================ +// Constants +// ============================================================================ + +/** + * @brief Maximum length for aggregated parameter IDs + * + * Aggregated parameter IDs use format "app_id:param_name", so max length is: + * - app_id: up to 256 characters (entity ID limit) + * - separator: 1 character (":") + * - param_name: up to 256 characters (ROS 2 parameter name limit) + */ +constexpr size_t MAX_AGGREGATED_PARAM_ID_LENGTH = 512; + // ============================================================================ // Helper structures and functions for configuration handlers // ============================================================================ @@ -86,41 +103,107 @@ struct ErrorClassification { }; /** - * @brief Classify error message to appropriate HTTP status and error code + * @brief Classify ParameterErrorCode to HTTP status and error code * - * Analyzes the error message to determine if it's: - * - 404 Not Found (parameter doesn't exist) - * - 403 Forbidden (read-only parameter) - * - 503 Service Unavailable (node/service not reachable) - * - 400 Bad Request (other errors) + * Maps structured error codes from ConfigurationManager to HTTP responses. * - * @param error_message The error message to classify - * @return ErrorClassification with status code and error code + * @param error_code The structured error code from ParameterResult + * @return ErrorClassification with HTTP status code and error code string */ -static ErrorClassification classify_parameter_error(const std::string & error_message) { +static ErrorClassification classify_error_code(ParameterErrorCode error_code) { ErrorClassification result; + switch (error_code) { + case ParameterErrorCode::NOT_FOUND: + case ParameterErrorCode::NO_DEFAULTS_CACHED: + result.status_code = StatusCode::NotFound_404; + result.error_code = ERR_RESOURCE_NOT_FOUND; + break; + case ParameterErrorCode::READ_ONLY: + result.status_code = StatusCode::Forbidden_403; + result.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY; + break; + case ParameterErrorCode::SERVICE_UNAVAILABLE: + case ParameterErrorCode::TIMEOUT: + result.status_code = StatusCode::ServiceUnavailable_503; + result.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE; + break; + case ParameterErrorCode::TYPE_MISMATCH: + case ParameterErrorCode::INVALID_VALUE: + result.status_code = StatusCode::BadRequest_400; + result.error_code = ERR_INVALID_PARAMETER; + break; + case ParameterErrorCode::INTERNAL_ERROR: + default: + result.status_code = StatusCode::InternalServerError_500; + result.error_code = ERR_INTERNAL_ERROR; + break; + } + + return result; +} + +/** + * @brief Classify error from ParameterResult to HTTP status and error code + * + * Uses structured error_code if available, falls back to string parsing for + * backward compatibility with older error messages. + * + * @param result The ParameterResult to classify + * @return ErrorClassification with HTTP status code and error code string + */ +static ErrorClassification classify_parameter_error(const ParameterResult & result) { + // Prefer structured error code + if (result.error_code != ParameterErrorCode::NONE) { + return classify_error_code(result.error_code); + } + + // Fallback to string parsing for backward compatibility + ErrorClassification classification; + const auto & error_message = result.error_message; + if (error_message.find("not found") != std::string::npos || error_message.find("Parameter not found") != std::string::npos) { - result.status_code = StatusCode::NotFound_404; - result.error_code = ERR_RESOURCE_NOT_FOUND; + classification.status_code = StatusCode::NotFound_404; + classification.error_code = ERR_RESOURCE_NOT_FOUND; } else if (error_message.find("read-only") != std::string::npos || error_message.find("read only") != std::string::npos || error_message.find("is read_only") != std::string::npos) { - result.status_code = StatusCode::Forbidden_403; - result.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY; + classification.status_code = StatusCode::Forbidden_403; + classification.error_code = ERR_X_MEDKIT_ROS2_PARAMETER_READ_ONLY; } else if (error_message.find("not available") != std::string::npos || error_message.find("service not available") != std::string::npos || error_message.find("timed out") != std::string::npos || error_message.find("timeout") != std::string::npos) { - result.status_code = StatusCode::ServiceUnavailable_503; - result.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE; + classification.status_code = StatusCode::ServiceUnavailable_503; + classification.error_code = ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE; } else { - result.status_code = StatusCode::BadRequest_400; - result.error_code = ERR_INVALID_REQUEST; + classification.status_code = StatusCode::BadRequest_400; + classification.error_code = ERR_INVALID_REQUEST; } - return result; + return classification; +} + +/** + * @brief Send error response for a failed parameter operation + * + * Common helper for set/get/reset parameter operations. Classifies the error + * and sends appropriate HTTP response. + * + * @param res HTTP response to send + * @param result ParameterResult containing error info + * @param operation_name Name of the operation (e.g., "set", "reset", "get") + * @param entity_id Entity ID for error context + * @param param_id Parameter ID for error context + */ +static void send_parameter_error(httplib::Response & res, const ParameterResult & result, + const std::string & operation_name, const std::string & entity_id, + const std::string & param_id) { + auto err = classify_parameter_error(result); + std::string message = "Failed to " + operation_name + " parameter"; + HandlerContext::send_error(res, err.status_code, err.error_code, message, + json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); } // ============================================================================ @@ -144,16 +227,16 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht return; } - // Use unified entity lookup - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { + // First, verify that the entity actually exists in the cache + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto entity_ref = cache.find_entity(entity_id); + if (!entity_ref) { HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; } // Get aggregated configurations info for this entity - const auto & cache = ctx_.node()->get_thread_safe_cache(); auto agg_configs = cache.get_entity_configurations(entity_id); // If no nodes to query, return empty result @@ -177,10 +260,33 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht std::vector queried_nodes; bool any_success = false; std::string first_error; + std::string first_error_node; // Track which node failed for better diagnostics + + // Query nodes in parallel for better performance with large hierarchies + // Each async task queries one node and returns its result + struct NodeQueryResult { + NodeConfigInfo node_info; + ParameterResult result; + }; + + std::vector> futures; + futures.reserve(agg_configs.nodes.size()); - // Query each node and aggregate results + // Launch parallel queries for (const auto & node_info : agg_configs.nodes) { - auto result = config_mgr->list_parameters(node_info.node_fqn); + futures.push_back(std::async(std::launch::async, [config_mgr, node_info]() { + NodeQueryResult query_result; + query_result.node_info = node_info; + query_result.result = config_mgr->list_parameters(node_info.node_fqn); + return query_result; + })); + } + + // Collect results + for (auto & future : futures) { + auto query_result = future.get(); + const auto & node_info = query_result.node_info; + const auto & result = query_result.result; if (result.success) { any_success = true; @@ -191,7 +297,8 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht json config_meta; std::string param_name = param.value("name", ""); - // Create unique ID for aggregated configs: node_fqn:param_name + // Create unique ID for aggregated configs: app_id:param_name + // This allows disambiguation when multiple apps have parameters with the same name std::string unique_id = param_name; if (agg_configs.is_aggregated) { unique_id = node_info.app_id + ":" + param_name; @@ -217,14 +324,16 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht } } else if (first_error.empty()) { first_error = result.error_message; + first_error_node = node_info.node_fqn; } } // If no successful queries, return error if (!any_success) { - HandlerContext::send_error(res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, - "Failed to list parameters from any node", - {{"details", first_error}, {"entity_id", entity_id}}); + HandlerContext::send_error( + res, StatusCode::ServiceUnavailable_503, ERR_X_MEDKIT_ROS2_NODE_UNAVAILABLE, + "Failed to list parameters from any node", + {{"details", first_error}, {"entity_id", entity_id}, {"failed_node", first_error_node}}); return; } @@ -270,22 +379,22 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http } // Parameter ID may be prefixed with app_id: for aggregated configs - if (param_id.empty() || param_id.length() > 512) { + if (param_id.empty() || param_id.length() > MAX_AGGREGATED_PARAM_ID_LENGTH) { HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter ID", {{"details", "Parameter ID is empty or too long"}}); return; } - // Use unified entity lookup - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { + // Verify entity exists + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto entity_ref = cache.find_entity(entity_id); + if (!entity_ref) { HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; } // Get aggregated configurations info - const auto & cache = ctx_.node()->get_thread_safe_cache(); auto agg_configs = cache.get_entity_configurations(entity_id); if (agg_configs.nodes.empty()) { @@ -324,7 +433,7 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http HandlerContext::send_json(res, response); } else { - auto err = classify_parameter_error(result.error_message); + auto err = classify_parameter_error(result); HandlerContext::send_error(res, err.status_code, err.error_code, err.status_code == StatusCode::NotFound_404 ? "Parameter not found" : "Failed to get parameter", @@ -335,8 +444,8 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http // For non-aggregated or no prefix: search all nodes for the parameter // Track errors to provide meaningful response if parameter not found anywhere - std::string last_error; - bool all_not_found = true; // Track if all failures are "not found" vs other errors + ParameterResult last_result; // Track full result for error code info + bool all_not_found = true; // Track if all failures are "not found" vs other errors for (const auto & node_info : agg_configs.nodes) { auto result = config_mgr->get_parameter(node_info.node_fqn, parsed.param_name); @@ -359,8 +468,8 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http } // Track the error for later reporting - last_error = result.error_message; - auto err = classify_parameter_error(result.error_message); + last_result = result; + auto err = classify_parameter_error(result); if (err.status_code != StatusCode::NotFound_404) { all_not_found = false; } @@ -372,9 +481,10 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http json{{"entity_id", entity_id}, {"id", param_id}}); } else { // Some nodes had non-"not found" errors (e.g., unavailable) - report 503 - auto err = classify_parameter_error(last_error); - HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to get parameter from any node", - json{{"details", last_error}, {"entity_id", entity_id}, {"id", param_id}}); + auto err = classify_parameter_error(last_result); + HandlerContext::send_error( + res, err.status_code, err.error_code, "Failed to get parameter from any node", + json{{"details", last_result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); } } catch (const std::exception & e) { @@ -405,7 +515,7 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - if (param_id.empty() || param_id.length() > 512) { + if (param_id.empty() || param_id.length() > MAX_AGGREGATED_PARAM_ID_LENGTH) { HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid parameter ID", {{"details", "Parameter ID is empty or too long"}}); return; @@ -433,16 +543,16 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - // Use unified entity lookup - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { + // Verify entity exists + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto entity_ref = cache.find_entity(entity_id); + if (!entity_ref) { HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; } // Get aggregated configurations info - const auto & cache = ctx_.node()->get_thread_safe_cache(); auto agg_configs = cache.get_entity_configurations(entity_id); if (agg_configs.nodes.empty()) { @@ -475,9 +585,7 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return true; } - auto err = classify_parameter_error(result.error_message); - HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to set parameter", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + send_parameter_error(res, result, "set", entity_id, param_id); return false; }; @@ -540,16 +648,16 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h return; } - // Use unified entity lookup - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { + // Verify entity exists + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto entity_ref = cache.find_entity(entity_id); + if (!entity_ref) { HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; } // Get aggregated configurations info - const auto & cache = ctx_.node()->get_thread_safe_cache(); auto agg_configs = cache.get_entity_configurations(entity_id); if (agg_configs.nodes.empty()) { @@ -569,9 +677,7 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h res.status = StatusCode::NoContent_204; return true; } - auto err = classify_parameter_error(result.error_message); - HandlerContext::send_error(res, err.status_code, err.error_code, "Failed to reset parameter", - json{{"details", result.error_message}, {"entity_id", entity_id}, {"id", param_id}}); + send_parameter_error(res, result, "reset", entity_id, param_id); return false; }; @@ -631,16 +737,16 @@ void ConfigHandlers::handle_delete_all_configurations(const httplib::Request & r return; } - // Use unified entity lookup - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { + // Verify entity exists + const auto & cache = ctx_.node()->get_thread_safe_cache(); + auto entity_ref = cache.find_entity(entity_id); + if (!entity_ref) { HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); return; } // Get aggregated configurations info - const auto & cache = ctx_.node()->get_thread_safe_cache(); auto agg_configs = cache.get_entity_configurations(entity_id); if (agg_configs.nodes.empty()) { diff --git a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp index fc4b477..c544072 100644 --- a/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp +++ b/src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp @@ -460,9 +460,9 @@ AggregatedConfigurations ThreadSafeEntityCache::get_component_configurations(con result.source_ids.push_back(app.id); } } - if (!apps_it->second.empty()) { - result.is_aggregated = true; - } + // Only mark as aggregated if there are multiple nodes (sources) + // A component with a single app should behave like a non-aggregated entity + result.is_aggregated = result.nodes.size() > 1; } return result; @@ -472,7 +472,6 @@ AggregatedConfigurations ThreadSafeEntityCache::get_area_configurations(const st std::shared_lock lock(mutex_); AggregatedConfigurations result; result.aggregation_level = "area"; - result.is_aggregated = true; // Area configurations are always aggregated auto area_it = area_index_.find(area_id); if (area_it == area_index_.end()) { @@ -513,6 +512,9 @@ AggregatedConfigurations ThreadSafeEntityCache::get_area_configurations(const st } } + // Only mark as aggregated if there are multiple nodes + result.is_aggregated = result.nodes.size() > 1; + return result; } @@ -520,7 +522,6 @@ AggregatedConfigurations ThreadSafeEntityCache::get_function_configurations(cons std::shared_lock lock(mutex_); AggregatedConfigurations result; result.aggregation_level = "function"; - result.is_aggregated = true; // Function configurations are always aggregated auto func_it = function_index_.find(function_id); if (func_it == function_index_.end()) { @@ -548,6 +549,9 @@ AggregatedConfigurations ThreadSafeEntityCache::get_function_configurations(cons } } + // Only mark as aggregated if there are multiple nodes + result.is_aggregated = result.nodes.size() > 1; + return result; }