diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handler_context.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handler_context.hpp index f6eee12..25674c6 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handler_context.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handler_context.hpp @@ -29,6 +29,7 @@ #include "ros2_medkit_gateway/auth/auth_manager.hpp" #include "ros2_medkit_gateway/config.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" #include "ros2_medkit_gateway/models/entity_capabilities.hpp" #include "ros2_medkit_gateway/models/entity_types.hpp" @@ -151,13 +152,16 @@ class HandlerContext { /** * @brief Get information about any entity (Component, App, Area, Function) * - * Searches through all entity types in order: Component, App, Area, Function. - * Returns the first match found. + * If expected_type is specified, searches ONLY in that collection. + * If expected_type is UNKNOWN (default), searches all types in order: + * Component, App, Area, Function - returns the first match found. * * @param entity_id Entity ID to look up + * @param expected_type Optional: restrict search to specific entity type * @return EntityInfo with resolved details, or EntityInfo with UNKNOWN type if not found */ - EntityInfo get_entity_info(const std::string & entity_id) const; + EntityInfo get_entity_info(const std::string & entity_id, + SovdEntityType expected_type = SovdEntityType::UNKNOWN) const; /** * @brief Validate that entity supports a resource collection @@ -172,6 +176,25 @@ class HandlerContext { static std::optional validate_collection_access(const EntityInfo & entity, ResourceCollection collection); + /** + * @brief Validate entity exists and matches expected route type + * + * Unified validation helper that: + * 1. Validates entity ID format + * 2. Looks up entity in the expected collection (based on route path) + * 3. Sends appropriate error responses: + * - 400 with "invalid-parameter" if ID format is invalid + * - 400 with "invalid-parameter" if entity exists but wrong type for route + * - 404 with "entity-not-found" if entity doesn't exist + * + * @param req HTTP request (used to extract expected type from path) + * @param res HTTP response (error responses sent here) + * @param entity_id Entity ID to validate + * @return EntityInfo if valid, std::nullopt if error response was sent + */ + std::optional validate_entity_for_route(const httplib::Request & req, httplib::Response & res, + const std::string & entity_id) const; + /** * @brief Set CORS headers on response if origin is allowed * @param res HTTP response diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/http_utils.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/http_utils.hpp index c17c379..a556e93 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/http_utils.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/http_utils.hpp @@ -18,6 +18,8 @@ #include +#include "ros2_medkit_gateway/models/entity_types.hpp" + namespace ros2_medkit_gateway { /// API version prefix for all endpoints @@ -32,6 +34,52 @@ inline std::string api_path(const std::string & endpoint) { return std::string(API_BASE_PATH) + endpoint; } +/** + * @brief Extract expected entity type from request path + * + * Parses the URL path to determine which entity type the route expects. + * Used for semantic validation - ensuring /components/{id} only accepts + * component IDs, not app IDs. + * + * @param path Request path (e.g., "/api/v1/components/my_component/data") + * @return Expected entity type, or UNKNOWN if path doesn't match entity routes + */ +inline SovdEntityType extract_entity_type_from_path(const std::string & path) { + // Path format: /api/v1/{entity_type}/{id}/... + // Check for each entity type prefix after API base path + const std::string base = std::string(API_BASE_PATH) + "/"; + + // Require a segment boundary after the collection name: + // matches "/api/v1/components" or "/api/v1/components/...", but not "/api/v1/componentship". + auto matches_collection = [&path, &base](const std::string & collection) -> bool { + const std::string prefix = base + collection; + if (path.compare(0, prefix.size(), prefix) != 0) { + return false; + } + if (path.size() == prefix.size()) { + // Exact match: "/api/v1/{collection}" + return true; + } + // Require a '/' segment separator after the collection name + return path[prefix.size()] == '/'; + }; + + if (matches_collection("components")) { + return SovdEntityType::COMPONENT; + } + if (matches_collection("apps")) { + return SovdEntityType::APP; + } + if (matches_collection("areas")) { + return SovdEntityType::AREA; + } + if (matches_collection("functions")) { + return SovdEntityType::FUNCTION; + } + + return SovdEntityType::UNKNOWN; +} + /** * @brief Fault status filter flags for fault listing endpoints */ 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 08c71e3..5c9bbc5 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp @@ -19,6 +19,7 @@ #include "ros2_medkit_gateway/gateway_node.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" #include "ros2_medkit_gateway/http/x_medkit.hpp" using json = nlohmann::json; @@ -220,23 +221,14 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht entity_id = req.matches[1]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; - } - - // 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; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // 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 @@ -371,11 +363,10 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http entity_id = req.matches[1]; param_id = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // Parameter ID may be prefixed with app_id: for aggregated configs @@ -385,16 +376,8 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http return; } - // 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()) { @@ -508,6 +491,7 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http entity_id = req.matches[1]; param_id = req.matches[2]; + // Validate entity_id format first auto entity_validation = ctx_.validate_entity_id(entity_id); if (!entity_validation) { HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", @@ -521,7 +505,7 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - // Parse request body + // Parse request body before checking entity existence json body; try { body = json::parse(req.body); @@ -543,16 +527,14 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http return; } - // 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; + // Now validate entity exists and matches route type + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // 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()) { @@ -641,23 +623,14 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h entity_id = req.matches[1]; param_id = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; - } - - // 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; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // 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()) { @@ -730,23 +703,14 @@ void ConfigHandlers::handle_delete_all_configurations(const httplib::Request & r entity_id = req.matches[1]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; - } - - // 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; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // 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/http/handlers/data_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp index 9f960f1..9c96d34 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp @@ -38,23 +38,15 @@ void DataHandlers::handle_list_data(const httplib::Request & req, httplib::Respo entity_id = req.matches[1]; - auto validation_result = ctx_.validate_entity_id(entity_id); - if (!validation_result) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", validation_result.error()}, {"entity_id", entity_id}}); - return; - } - - // 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; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Use unified cache method to get aggregated data + const auto & cache = ctx_.node()->get_thread_safe_cache(); auto aggregated = cache.get_entity_data(entity_id); // Get data access manager for type introspection @@ -129,20 +121,10 @@ void DataHandlers::handle_get_data_item(const httplib::Request & req, httplib::R // cpp-httplib automatically decodes percent-encoded characters in URL path topic_name = req.matches[2]; - auto validation_result = ctx_.validate_entity_id(entity_id); - if (!validation_result) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", validation_result.error()}, {"entity_id", entity_id}}); - return; - } - - // 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; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // Determine the full ROS topic path @@ -221,11 +203,10 @@ void DataHandlers::handle_put_data_item(const httplib::Request & req, httplib::R entity_id = req.matches[1]; topic_name = req.matches[2]; - auto validation_result = ctx_.validate_entity_id(entity_id); - if (!validation_result) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", validation_result.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } // Parse request body @@ -268,15 +249,6 @@ void DataHandlers::handle_put_data_item(const httplib::Request & req, httplib::R return; } - // 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; - } - // Build full topic path (mirror GET logic: only prefix '/' when needed) std::string full_topic_path = topic_name; if (!full_topic_path.empty() && full_topic_path.front() != '/') { diff --git a/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp index b1f92fd..6884fde 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp @@ -187,19 +187,12 @@ void FaultHandlers::handle_list_faults(const httplib::Request & req, httplib::Re entity_id = req.matches[1]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; - } - - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", - {{"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; auto filter = parse_fault_status_param(req); if (!filter.is_valid) { @@ -353,12 +346,12 @@ void FaultHandlers::handle_get_fault(const httplib::Request & req, httplib::Resp entity_id = req.matches[1]; fault_code = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Fault codes may contain dots and underscores, validate basic constraints if (fault_code.empty() || fault_code.length() > 256) { @@ -367,12 +360,6 @@ void FaultHandlers::handle_get_fault(const httplib::Request & req, httplib::Resp return; } - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", - {{"entity_id", entity_id}}); - return; - } std::string namespace_path = entity_info.namespace_path; auto fault_mgr = ctx_.node()->get_fault_manager(); @@ -421,12 +408,12 @@ void FaultHandlers::handle_clear_fault(const httplib::Request & req, httplib::Re entity_id = req.matches[1]; fault_code = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Validate fault code if (fault_code.empty() || fault_code.length() > 256) { @@ -435,14 +422,6 @@ void FaultHandlers::handle_clear_fault(const httplib::Request & req, httplib::Re return; } - // Verify entity exists - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", - {{"entity_id", entity_id}}); - return; - } - auto fault_mgr = ctx_.node()->get_fault_manager(); auto result = fault_mgr->clear_fault(fault_code); @@ -480,20 +459,12 @@ void FaultHandlers::handle_clear_all_faults(const httplib::Request & req, httpli entity_id = req.matches[1]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; - } - - // Verify entity exists - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", - {{"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Get all faults for this entity auto fault_mgr = ctx_.node()->get_fault_manager(); @@ -588,12 +559,12 @@ void FaultHandlers::handle_get_component_snapshots(const httplib::Request & req, entity_id = req.matches[1]; fault_code = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Validate fault code if (fault_code.empty() || fault_code.length() > 256) { @@ -602,13 +573,6 @@ void FaultHandlers::handle_get_component_snapshots(const httplib::Request & req, return; } - auto entity_info = ctx_.get_entity_info(entity_id); - if (entity_info.type == EntityType::UNKNOWN) { - HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", - {{"entity_id", entity_id}}); - return; - } - // Optional topic filter from query parameter std::string topic_filter = req.get_param_value("topic"); diff --git a/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp b/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp index afe882c..3267a81 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/handler_context.cpp @@ -72,11 +72,70 @@ HandlerContext::get_component_namespace_path(const std::string & component_id) c return tl::unexpected("Component not found"); } -EntityInfo HandlerContext::get_entity_info(const std::string & entity_id) const { +EntityInfo HandlerContext::get_entity_info(const std::string & entity_id, SovdEntityType expected_type) const { const auto & cache = node_->get_thread_safe_cache(); EntityInfo info; info.id = entity_id; + // If expected_type is specified, search ONLY in that collection + // This prevents ID collisions (e.g., Area "powertrain" vs Component "powertrain") + if (expected_type != SovdEntityType::UNKNOWN) { + switch (expected_type) { + case SovdEntityType::COMPONENT: + if (auto component = cache.get_component(entity_id)) { + info.type = EntityType::COMPONENT; + info.namespace_path = component->namespace_path; + info.fqn = component->fqn; + info.id_field = "component_id"; + info.error_name = "Component"; + return info; + } + break; + + case SovdEntityType::APP: + if (auto app = cache.get_app(entity_id)) { + info.type = EntityType::APP; + info.namespace_path = app->bound_fqn.value_or(""); + info.fqn = app->bound_fqn.value_or(""); + info.id_field = "app_id"; + info.error_name = "App"; + return info; + } + break; + + case SovdEntityType::AREA: + if (auto area = cache.get_area(entity_id)) { + info.type = EntityType::AREA; + info.namespace_path = area->namespace_path; + info.fqn = area->namespace_path; + info.id_field = "area_id"; + info.error_name = "Area"; + return info; + } + break; + + case SovdEntityType::FUNCTION: + if (auto func = cache.get_function(entity_id)) { + info.type = EntityType::FUNCTION; + info.namespace_path = ""; + info.fqn = ""; + info.id_field = "function_id"; + info.error_name = "Function"; + return info; + } + break; + + case SovdEntityType::SERVER: + default: + break; + } + // Not found in expected collection + info.type = EntityType::UNKNOWN; + info.error_name = "Entity"; + return info; + } + + // No expected_type specified - search all collections in order (legacy behavior) // Search components first (O(1) lookup) if (auto component = cache.get_component(entity_id)) { info.type = EntityType::COMPONENT; @@ -135,6 +194,42 @@ std::optional HandlerContext::validate_collection_access(const Enti return std::nullopt; } +std::optional HandlerContext::validate_entity_for_route(const httplib::Request & req, + httplib::Response & res, + const std::string & entity_id) const { + // Step 1: Validate entity ID format + auto validation_result = validate_entity_id(entity_id); + if (!validation_result) { + send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", + {{"details", validation_result.error()}, {"entity_id", entity_id}}); + return std::nullopt; + } + + // Step 2: Get expected type from route path and look up entity + auto expected_type = extract_entity_type_from_path(req.path); + auto entity_info = get_entity_info(entity_id, expected_type); + + if (entity_info.type == EntityType::UNKNOWN) { + // Step 3: Check if entity exists in ANY collection (for better error message) + auto any_entity = get_entity_info(entity_id); + if (any_entity.type != EntityType::UNKNOWN) { + // Entity exists but wrong type for this route -> 400 + send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, + "Invalid entity type for route: expected " + to_string(expected_type) + ", got " + + to_string(any_entity.sovd_type()), + {{"entity_id", entity_id}, + {"expected_type", to_string(expected_type)}, + {"actual_type", to_string(any_entity.sovd_type())}}); + } else { + // Entity doesn't exist at all -> 404 + send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found", {{"entity_id", entity_id}}); + } + return std::nullopt; + } + + return entity_info; +} + void HandlerContext::set_cors_headers(httplib::Response & res, const std::string & origin) const { res.set_header("Access-Control-Allow-Origin", origin); diff --git a/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp index c43e9d3..02952c2 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/operation_handlers.cpp @@ -18,6 +18,7 @@ #include "ros2_medkit_gateway/gateway_node.hpp" #include "ros2_medkit_gateway/http/error_codes.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" #include "ros2_medkit_gateway/http/x_medkit.hpp" #include "ros2_medkit_gateway/operation_manager.hpp" @@ -37,12 +38,12 @@ void OperationHandlers::handle_list_operations(const httplib::Request & req, htt entity_id = req.matches[1]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Use ThreadSafeEntityCache for O(1) entity lookup and aggregated operations const auto & cache = ctx_.node()->get_thread_safe_cache(); @@ -51,14 +52,7 @@ void OperationHandlers::handle_list_operations(const httplib::Request & req, htt AggregatedOperations ops; std::string entity_type; - 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; - } - - switch (entity_ref->type) { + switch (entity_info.sovd_type()) { case SovdEntityType::COMPONENT: ops = cache.get_component_operations(entity_id); entity_type = "component"; @@ -179,28 +173,20 @@ void OperationHandlers::handle_get_operation(const httplib::Request & req, httpl entity_id = req.matches[1]; operation_id = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Use ThreadSafeEntityCache for O(1) entity lookup const auto & cache = ctx_.node()->get_thread_safe_cache(); - // Find entity - 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 operations based on entity type AggregatedOperations ops; std::string entity_type; - switch (entity_ref->type) { + switch (entity_info.sovd_type()) { case SovdEntityType::COMPONENT: ops = cache.get_component_operations(entity_id); entity_type = "component"; @@ -354,12 +340,12 @@ void OperationHandlers::handle_create_execution(const httplib::Request & req, ht entity_id = req.matches[1]; operation_id = req.matches[2]; - auto entity_validation = ctx_.validate_entity_id(entity_id); - if (!entity_validation) { - HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID", - {{"details", entity_validation.error()}, {"entity_id", entity_id}}); - return; + // Validate entity ID and type for this route + auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id); + if (!entity_opt) { + return; // Error response already sent } + auto entity_info = *entity_opt; // Parse request body json body = json::object(); @@ -376,18 +362,10 @@ void OperationHandlers::handle_create_execution(const httplib::Request & req, ht // Use ThreadSafeEntityCache for O(1) entity lookup const auto & cache = ctx_.node()->get_thread_safe_cache(); - // Find entity and get its operations - 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 operations based on entity type AggregatedOperations ops; std::string entity_type; - switch (entity_ref->type) { + switch (entity_info.sovd_type()) { case SovdEntityType::COMPONENT: ops = cache.get_component_operations(entity_id); entity_type = "component"; diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index 3460d35..5e9c522 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -578,11 +578,6 @@ void RESTServer::setup_routes() { discovery_handlers_->handle_get_area(req, res); }); - // TODO(#158): Dual-path design issue - in runtime-only mode, these routes accept both - // component IDs and app IDs. Semantically, /components should only accept - // synthetic component IDs, not individual ROS 2 node IDs. Consider enforcing - // proper entity type validation in a future refactor. - // // Component topic data (specific topic) - register before general route // Use (.+) for topic_name to accept slashes from percent-encoded URLs (%2F -> /) srv->Get((api_path("/components") + R"(/([^/]+)/data/(.+)$)"), diff --git a/src/ros2_medkit_gateway/test/test_gateway_node.cpp b/src/ros2_medkit_gateway/test/test_gateway_node.cpp index 27ec6ca..cbd2809 100644 --- a/src/ros2_medkit_gateway/test/test_gateway_node.cpp +++ b/src/ros2_medkit_gateway/test/test_gateway_node.cpp @@ -22,6 +22,7 @@ #include #include "ros2_medkit_gateway/gateway_node.hpp" +#include "ros2_medkit_gateway/http/http_utils.hpp" using namespace std::chrono_literals; using httplib::StatusCode; @@ -697,6 +698,62 @@ TEST_F(TestGatewayNode, test_invalid_function_id_bad_request) { EXPECT_EQ(res->status, StatusCode::BadRequest_400); } +// ============================================================================= +// Entity type path extraction tests +// ============================================================================= + +TEST(ExtractEntityTypePath, components_route) { + using ros2_medkit_gateway::extract_entity_type_from_path; + using ros2_medkit_gateway::SovdEntityType; + + EXPECT_EQ(extract_entity_type_from_path("/api/v1/components"), SovdEntityType::COMPONENT); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/components/my_comp"), SovdEntityType::COMPONENT); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/components/my_comp/data"), SovdEntityType::COMPONENT); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/components/test/operations"), SovdEntityType::COMPONENT); +} + +TEST(ExtractEntityTypePath, apps_route) { + using ros2_medkit_gateway::extract_entity_type_from_path; + using ros2_medkit_gateway::SovdEntityType; + + EXPECT_EQ(extract_entity_type_from_path("/api/v1/apps"), SovdEntityType::APP); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/apps/my_app"), SovdEntityType::APP); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/apps/my_app/data"), SovdEntityType::APP); +} + +TEST(ExtractEntityTypePath, areas_route) { + using ros2_medkit_gateway::extract_entity_type_from_path; + using ros2_medkit_gateway::SovdEntityType; + + EXPECT_EQ(extract_entity_type_from_path("/api/v1/areas"), SovdEntityType::AREA); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/areas/root"), SovdEntityType::AREA); +} + +TEST(ExtractEntityTypePath, functions_route) { + using ros2_medkit_gateway::extract_entity_type_from_path; + using ros2_medkit_gateway::SovdEntityType; + + EXPECT_EQ(extract_entity_type_from_path("/api/v1/functions"), SovdEntityType::FUNCTION); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/functions/my_func"), SovdEntityType::FUNCTION); +} + +TEST(ExtractEntityTypePath, unknown_routes) { + using ros2_medkit_gateway::extract_entity_type_from_path; + using ros2_medkit_gateway::SovdEntityType; + + EXPECT_EQ(extract_entity_type_from_path("/api/v1/health"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/faults"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/version-info"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/other/path"), SovdEntityType::UNKNOWN); + + // Verify paths that look similar to entity collections but aren't + // (segment boundary validation) + EXPECT_EQ(extract_entity_type_from_path("/api/v1/componentship"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/applications"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/areascan"), SovdEntityType::UNKNOWN); + EXPECT_EQ(extract_entity_type_from_path("/api/v1/functional"), SovdEntityType::UNKNOWN); +} + int main(int argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/src/ros2_medkit_gateway/test/test_integration.test.py b/src/ros2_medkit_gateway/test/test_integration.test.py index ef19dca..239594d 100644 --- a/src/ros2_medkit_gateway/test/test_integration.test.py +++ b/src/ros2_medkit_gateway/test/test_integration.test.py @@ -1698,10 +1698,15 @@ def test_40_action_status_endpoint(self): self.assertEqual(response.status_code, 202) execution_id = response.json()['id'] - # Check status immediately (allow extra time for action server response) + # Poll for execution to be registered (may take a moment) exec_url = (f'{self.BASE_URL}/apps/long_calibration/operations/' f'long_calibration/executions/{execution_id}') - status_response = requests.get(exec_url, timeout=10) + status_response = None + for _ in range(10): + status_response = requests.get(exec_url, timeout=10) + if status_response.status_code == 200: + break + time.sleep(0.2) self.assertEqual(status_response.status_code, 200) data = status_response.json() @@ -3651,12 +3656,17 @@ def test_100_get_execution_status(self): execution_id = create_response.json()['id'] - # Now get the execution status - response = requests.get( - f'{self.BASE_URL}/apps/{app_id}/operations/{operation_id}' - f'/executions/{execution_id}', - timeout=10 - ) + # Poll for execution to be registered (may take a moment) + response = None + for _ in range(10): + response = requests.get( + f'{self.BASE_URL}/apps/{app_id}/operations/{operation_id}' + f'/executions/{execution_id}', + timeout=10 + ) + if response.status_code == 200: + break + time.sleep(0.2) self.assertEqual( response.status_code, 200, @@ -4248,3 +4258,230 @@ def test_115_list_function_data_invalid_id(self): self.assertIn('invalid', data['message'].lower()) print('✓ Function data invalid ID test passed') + + # ========== Entity Type Validation Tests (test_116-120) ========== + + def test_116_component_route_rejects_app_id(self): + """ + Test that /components/{id}/data rejects app IDs. + + In runtime-only discovery mode, /components/{id} should only accept + synthetic component IDs, not individual ROS 2 node (app) IDs. + + @verifies REQ_INTEROP_003 + """ + # First, get list of apps to find a valid app ID + apps_response = requests.get(f'{self.BASE_URL}/apps', timeout=10) + self.assertEqual(apps_response.status_code, 200) + + apps = apps_response.json().get('items', []) + if len(apps) == 0: + self.skipTest('No apps available for testing') + + # Get list of components to avoid selecting an app ID that is also a component ID + components_response = requests.get(f'{self.BASE_URL}/components', timeout=10) + self.assertEqual(components_response.status_code, 200) + + components = components_response.json().get('items', []) + component_ids = {c.get('id') for c in components if isinstance(c, dict) and 'id' in c} + + # Find an app ID that does not collide with any component ID + app_id = None + for app in apps: + if not isinstance(app, dict) or 'id' not in app: + continue + if app['id'] not in component_ids: + app_id = app['id'] + break + + if app_id is None: + self.skipTest('No app ID available that is distinct from component IDs') + + # Verify this ID is recognized as an app via /apps/{id} + app_response = requests.get(f'{self.BASE_URL}/apps/{app_id}', timeout=10) + self.assertEqual(app_response.status_code, 200) + + # Now try to use this app ID with /components/{id}/data - should fail + response = requests.get( + f'{self.BASE_URL}/components/{app_id}/data', + timeout=10 + ) + self.assertEqual( + response.status_code, + 400, + f'Expected 400 when using app ID "{app_id}" with /components route' + ) + + data = response.json() + self.assertIn('error_code', data) + self.assertEqual(data['error_code'], 'invalid-parameter') + self.assertIn('Invalid entity type for route', data['message']) + self.assertIn('expected_type', data.get('parameters', {})) + self.assertIn('actual_type', data.get('parameters', {})) + self.assertEqual(data['parameters']['expected_type'], 'Component') + self.assertEqual(data['parameters']['actual_type'], 'App') + + print(f'✓ Component route correctly rejects app ID: {app_id}') + + def test_117_component_route_rejects_app_id_operations(self): + """ + Test that /components/{id}/operations rejects app IDs. + + @verifies REQ_INTEROP_003 + """ + # First, get list of apps to find a valid app ID + apps_response = requests.get(f'{self.BASE_URL}/apps', timeout=10) + self.assertEqual(apps_response.status_code, 200) + + apps = apps_response.json().get('items', []) + if len(apps) == 0: + self.skipTest('No apps available for testing') + + # Get list of components to avoid selecting an app ID that is also a component ID + components_response = requests.get(f'{self.BASE_URL}/components', timeout=10) + self.assertEqual(components_response.status_code, 200) + + components = components_response.json().get('items', []) + component_ids = {c.get('id') for c in components if isinstance(c, dict) and 'id' in c} + + # Find an app ID that does not collide with any component ID + app_id = None + for app in apps: + if not isinstance(app, dict) or 'id' not in app: + continue + if app['id'] not in component_ids: + app_id = app['id'] + break + + if app_id is None: + self.skipTest('No app ID available that is distinct from component IDs') + + # Try to use app ID with /components/{id}/operations - should fail + response = requests.get( + f'{self.BASE_URL}/components/{app_id}/operations', + timeout=10 + ) + self.assertEqual(response.status_code, 400) + + data = response.json() + self.assertEqual(data['error_code'], 'invalid-parameter') + self.assertIn('Invalid entity type for route', data['message']) + + print(f'✓ Component operations route correctly rejects app ID: {app_id}') + + def test_118_component_route_rejects_app_id_configurations(self): + """ + Test that /components/{id}/configurations rejects app IDs. + + @verifies REQ_INTEROP_003 + """ + apps_response = requests.get(f'{self.BASE_URL}/apps', timeout=10) + self.assertEqual(apps_response.status_code, 200) + + apps = apps_response.json().get('items', []) + if len(apps) == 0: + self.skipTest('No apps available for testing') + + # Get list of components to avoid selecting an app ID that is also a component ID + components_response = requests.get(f'{self.BASE_URL}/components', timeout=10) + self.assertEqual(components_response.status_code, 200) + + components = components_response.json().get('items', []) + component_ids = {c.get('id') for c in components if isinstance(c, dict) and 'id' in c} + + # Find an app ID that does not collide with any component ID + app_id = None + for app in apps: + if not isinstance(app, dict) or 'id' not in app: + continue + if app['id'] not in component_ids: + app_id = app['id'] + break + + if app_id is None: + self.skipTest('No app ID available that is distinct from component IDs') + + response = requests.get( + f'{self.BASE_URL}/components/{app_id}/configurations', + timeout=10 + ) + self.assertEqual(response.status_code, 400) + + data = response.json() + self.assertEqual(data['error_code'], 'invalid-parameter') + self.assertIn('Invalid entity type for route', data['message']) + + print(f'✓ Component configurations route correctly rejects app ID: {app_id}') + + def test_119_component_route_rejects_app_id_faults(self): + """ + Test that /components/{id}/faults rejects app IDs. + + @verifies REQ_INTEROP_003 + """ + apps_response = requests.get(f'{self.BASE_URL}/apps', timeout=10) + self.assertEqual(apps_response.status_code, 200) + + apps = apps_response.json().get('items', []) + if len(apps) == 0: + self.skipTest('No apps available for testing') + + # Get list of components to avoid selecting an app ID that is also a component ID + components_response = requests.get(f'{self.BASE_URL}/components', timeout=10) + self.assertEqual(components_response.status_code, 200) + + components = components_response.json().get('items', []) + component_ids = {c.get('id') for c in components if isinstance(c, dict) and 'id' in c} + + # Find an app ID that does not collide with any component ID + app_id = None + for app in apps: + if not isinstance(app, dict) or 'id' not in app: + continue + if app['id'] not in component_ids: + app_id = app['id'] + break + + if app_id is None: + self.skipTest('No app ID available that is distinct from component IDs') + + response = requests.get( + f'{self.BASE_URL}/components/{app_id}/faults', + timeout=10 + ) + self.assertEqual(response.status_code, 400) + + data = response.json() + self.assertEqual(data['error_code'], 'invalid-parameter') + self.assertIn('Invalid entity type for route', data['message']) + + print(f'✓ Component faults route correctly rejects app ID: {app_id}') + + def test_120_app_routes_work_with_app_id(self): + """ + Test that /apps/{id}/data works correctly with app IDs. + + Verify that while /components rejects app IDs, /apps accepts them. + + @verifies REQ_INTEROP_003 + """ + apps_response = requests.get(f'{self.BASE_URL}/apps', timeout=10) + self.assertEqual(apps_response.status_code, 200) + + apps = apps_response.json().get('items', []) + if len(apps) == 0: + self.skipTest('No apps available for testing') + + app_id = apps[0]['id'] + + # /apps/{id}/data should work + response = requests.get( + f'{self.BASE_URL}/apps/{app_id}/data', + timeout=10 + ) + self.assertEqual(response.status_code, 200) + + data = response.json() + self.assertIn('items', data) + + print(f'✓ App routes correctly accept app ID: {app_id}')