Skip to content

Commit d58db39

Browse files
bburdabburda
committed
fix(gateway): unify entity validation across handlers
- Add validate_entity_for_route() helper in HandlerContext that combines entity_id validation, path-based type extraction, and entity lookup - Refactor data_handlers.cpp (3 handlers) to use unified validation - Refactor config_handlers.cpp (7 handlers) to use unified validation - Refactor fault_handlers.cpp (5 handlers) to use unified validation - Refactor operation_handlers.cpp (3 handlers) to use unified validation - Net reduction of ~270 lines of duplicated validation code Addresses review comments from mfaferek93 on PR #160
1 parent 8dafac9 commit d58db39

File tree

6 files changed

+136
-409
lines changed

6 files changed

+136
-409
lines changed

‎src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handler_context.hpp‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,24 @@ class HandlerContext {
176176
static std::optional<std::string> validate_collection_access(const EntityInfo & entity,
177177
ResourceCollection collection);
178178

179-
179+
/**
180+
* @brief Validate entity exists and matches expected route type
181+
*
182+
* Unified validation helper that:
183+
* 1. Validates entity ID format
184+
* 2. Looks up entity in the expected collection (based on route path)
185+
* 3. Sends appropriate error responses:
186+
* - 400 with "invalid-parameter" if ID format is invalid
187+
* - 400 with "invalid-parameter" if entity exists but wrong type for route
188+
* - 404 with "entity-not-found" if entity doesn't exist
189+
*
190+
* @param req HTTP request (used to extract expected type from path)
191+
* @param res HTTP response (error responses sent here)
192+
* @param entity_id Entity ID to validate
193+
* @return EntityInfo if valid, std::nullopt if error response was sent
194+
*/
195+
std::optional<EntityInfo> validate_entity_for_route(const httplib::Request & req, httplib::Response & res,
196+
const std::string & entity_id) const;
180197

181198
/**
182199
* @brief Set CORS headers on response if origin is allowed

‎src/ros2_medkit_gateway/src/http/handlers/config_handlers.cpp‎

Lines changed: 25 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -221,35 +221,14 @@ void ConfigHandlers::handle_list_configurations(const httplib::Request & req, ht
221221

222222
entity_id = req.matches[1];
223223

224-
auto entity_validation = ctx_.validate_entity_id(entity_id);
225-
if (!entity_validation) {
226-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
227-
{{"details", entity_validation.error()}, {"entity_id", entity_id}});
228-
return;
229-
}
230-
231-
// First, verify that the entity actually exists in the cache
232-
const auto & cache = ctx_.node()->get_thread_safe_cache();
233-
auto entity_ref = cache.find_entity(entity_id);
234-
if (!entity_ref) {
235-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
236-
{{"entity_id", entity_id}});
237-
return;
238-
}
239-
240-
// Validate entity type matches the route path
241-
auto expected_type = extract_entity_type_from_path(req.path);
242-
if (expected_type != SovdEntityType::UNKNOWN && entity_ref->type != expected_type) {
243-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
244-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
245-
to_string(entity_ref->type),
246-
{{"entity_id", entity_id},
247-
{"expected_type", to_string(expected_type)},
248-
{"actual_type", to_string(entity_ref->type)}});
249-
return;
224+
// Validate entity ID and type for this route
225+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
226+
if (!entity_opt) {
227+
return; // Error response already sent
250228
}
251229

252230
// Get aggregated configurations info for this entity
231+
const auto & cache = ctx_.node()->get_thread_safe_cache();
253232
auto agg_configs = cache.get_entity_configurations(entity_id);
254233

255234
// If no nodes to query, return empty result
@@ -384,11 +363,10 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http
384363
entity_id = req.matches[1];
385364
param_id = req.matches[2];
386365

387-
auto entity_validation = ctx_.validate_entity_id(entity_id);
388-
if (!entity_validation) {
389-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
390-
{{"details", entity_validation.error()}, {"entity_id", entity_id}});
391-
return;
366+
// Validate entity ID and type for this route
367+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
368+
if (!entity_opt) {
369+
return; // Error response already sent
392370
}
393371

394372
// Parameter ID may be prefixed with app_id: for aggregated configs
@@ -398,28 +376,8 @@ void ConfigHandlers::handle_get_configuration(const httplib::Request & req, http
398376
return;
399377
}
400378

401-
// Verify entity exists
402-
const auto & cache = ctx_.node()->get_thread_safe_cache();
403-
auto entity_ref = cache.find_entity(entity_id);
404-
if (!entity_ref) {
405-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
406-
{{"entity_id", entity_id}});
407-
return;
408-
}
409-
410-
// Validate entity type matches the route path
411-
auto expected_type = extract_entity_type_from_path(req.path);
412-
if (expected_type != SovdEntityType::UNKNOWN && entity_ref->type != expected_type) {
413-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
414-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
415-
to_string(entity_ref->type),
416-
{{"entity_id", entity_id},
417-
{"expected_type", to_string(expected_type)},
418-
{"actual_type", to_string(entity_ref->type)}});
419-
return;
420-
}
421-
422379
// Get aggregated configurations info
380+
const auto & cache = ctx_.node()->get_thread_safe_cache();
423381
auto agg_configs = cache.get_entity_configurations(entity_id);
424382

425383
if (agg_configs.nodes.empty()) {
@@ -533,11 +491,10 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http
533491
entity_id = req.matches[1];
534492
param_id = req.matches[2];
535493

536-
auto entity_validation = ctx_.validate_entity_id(entity_id);
537-
if (!entity_validation) {
538-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
539-
{{"details", entity_validation.error()}, {"entity_id", entity_id}});
540-
return;
494+
// Validate entity ID and type for this route
495+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
496+
if (!entity_opt) {
497+
return; // Error response already sent
541498
}
542499

543500
if (param_id.empty() || param_id.length() > MAX_AGGREGATED_PARAM_ID_LENGTH) {
@@ -568,28 +525,8 @@ void ConfigHandlers::handle_set_configuration(const httplib::Request & req, http
568525
return;
569526
}
570527

571-
// Verify entity exists
572-
const auto & cache = ctx_.node()->get_thread_safe_cache();
573-
auto entity_ref = cache.find_entity(entity_id);
574-
if (!entity_ref) {
575-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
576-
{{"entity_id", entity_id}});
577-
return;
578-
}
579-
580-
// Validate entity type matches the route path
581-
auto expected_type = extract_entity_type_from_path(req.path);
582-
if (expected_type != SovdEntityType::UNKNOWN && entity_ref->type != expected_type) {
583-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
584-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
585-
to_string(entity_ref->type),
586-
{{"entity_id", entity_id},
587-
{"expected_type", to_string(expected_type)},
588-
{"actual_type", to_string(entity_ref->type)}});
589-
return;
590-
}
591-
592528
// Get aggregated configurations info
529+
const auto & cache = ctx_.node()->get_thread_safe_cache();
593530
auto agg_configs = cache.get_entity_configurations(entity_id);
594531

595532
if (agg_configs.nodes.empty()) {
@@ -678,35 +615,14 @@ void ConfigHandlers::handle_delete_configuration(const httplib::Request & req, h
678615
entity_id = req.matches[1];
679616
param_id = req.matches[2];
680617

681-
auto entity_validation = ctx_.validate_entity_id(entity_id);
682-
if (!entity_validation) {
683-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
684-
{{"details", entity_validation.error()}, {"entity_id", entity_id}});
685-
return;
686-
}
687-
688-
// Verify entity exists
689-
const auto & cache = ctx_.node()->get_thread_safe_cache();
690-
auto entity_ref = cache.find_entity(entity_id);
691-
if (!entity_ref) {
692-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
693-
{{"entity_id", entity_id}});
694-
return;
695-
}
696-
697-
// Validate entity type matches the route path
698-
auto expected_type = extract_entity_type_from_path(req.path);
699-
if (expected_type != SovdEntityType::UNKNOWN && entity_ref->type != expected_type) {
700-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
701-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
702-
to_string(entity_ref->type),
703-
{{"entity_id", entity_id},
704-
{"expected_type", to_string(expected_type)},
705-
{"actual_type", to_string(entity_ref->type)}});
706-
return;
618+
// Validate entity ID and type for this route
619+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
620+
if (!entity_opt) {
621+
return; // Error response already sent
707622
}
708623

709624
// Get aggregated configurations info
625+
const auto & cache = ctx_.node()->get_thread_safe_cache();
710626
auto agg_configs = cache.get_entity_configurations(entity_id);
711627

712628
if (agg_configs.nodes.empty()) {
@@ -779,35 +695,14 @@ void ConfigHandlers::handle_delete_all_configurations(const httplib::Request & r
779695

780696
entity_id = req.matches[1];
781697

782-
auto entity_validation = ctx_.validate_entity_id(entity_id);
783-
if (!entity_validation) {
784-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
785-
{{"details", entity_validation.error()}, {"entity_id", entity_id}});
786-
return;
787-
}
788-
789-
// Verify entity exists
790-
const auto & cache = ctx_.node()->get_thread_safe_cache();
791-
auto entity_ref = cache.find_entity(entity_id);
792-
if (!entity_ref) {
793-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
794-
{{"entity_id", entity_id}});
795-
return;
796-
}
797-
798-
// Validate entity type matches the route path
799-
auto expected_type = extract_entity_type_from_path(req.path);
800-
if (expected_type != SovdEntityType::UNKNOWN && entity_ref->type != expected_type) {
801-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
802-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
803-
to_string(entity_ref->type),
804-
{{"entity_id", entity_id},
805-
{"expected_type", to_string(expected_type)},
806-
{"actual_type", to_string(entity_ref->type)}});
807-
return;
698+
// Validate entity ID and type for this route
699+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
700+
if (!entity_opt) {
701+
return; // Error response already sent
808702
}
809703

810704
// Get aggregated configurations info
705+
const auto & cache = ctx_.node()->get_thread_safe_cache();
811706
auto agg_configs = cache.get_entity_configurations(entity_id);
812707

813708
if (agg_configs.nodes.empty()) {

‎src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp‎

Lines changed: 14 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -38,39 +38,15 @@ void DataHandlers::handle_list_data(const httplib::Request & req, httplib::Respo
3838

3939
entity_id = req.matches[1];
4040

41-
auto validation_result = ctx_.validate_entity_id(entity_id);
42-
if (!validation_result) {
43-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
44-
{{"details", validation_result.error()}, {"entity_id", entity_id}});
45-
return;
46-
}
47-
48-
// First, verify that the entity actually exists in the cache
49-
const auto & cache = ctx_.node()->get_thread_safe_cache();
50-
51-
// Get expected type from route and look up entity in that specific collection
52-
auto expected_type = extract_entity_type_from_path(req.path);
53-
auto entity_info = ctx_.get_entity_info(entity_id, expected_type);
54-
if (entity_info.type == EntityType::UNKNOWN) {
55-
// Check if entity exists in ANY collection (for better error message)
56-
auto any_entity = ctx_.get_entity_info(entity_id);
57-
if (any_entity.type != EntityType::UNKNOWN) {
58-
// Entity exists but wrong type -> 400
59-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
60-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
61-
to_string(any_entity.sovd_type()),
62-
{{"entity_id", entity_id},
63-
{"expected_type", to_string(expected_type)},
64-
{"actual_type", to_string(any_entity.sovd_type())}});
65-
} else {
66-
// Entity doesn't exist at all -> 404
67-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
68-
{{"entity_id", entity_id}});
69-
}
70-
return;
41+
// Validate entity ID and type for this route
42+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
43+
if (!entity_opt) {
44+
return; // Error response already sent
7145
}
46+
auto entity_info = *entity_opt;
7247

7348
// Use unified cache method to get aggregated data
49+
const auto & cache = ctx_.node()->get_thread_safe_cache();
7450
auto aggregated = cache.get_entity_data(entity_id);
7551

7652
// Get data access manager for type introspection
@@ -145,33 +121,10 @@ void DataHandlers::handle_get_data_item(const httplib::Request & req, httplib::R
145121
// cpp-httplib automatically decodes percent-encoded characters in URL path
146122
topic_name = req.matches[2];
147123

148-
auto validation_result = ctx_.validate_entity_id(entity_id);
149-
if (!validation_result) {
150-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
151-
{{"details", validation_result.error()}, {"entity_id", entity_id}});
152-
return;
153-
}
154-
155-
// Verify entity exists in the correct collection for this route
156-
auto expected_type = extract_entity_type_from_path(req.path);
157-
auto entity_info = ctx_.get_entity_info(entity_id, expected_type);
158-
if (entity_info.type == EntityType::UNKNOWN) {
159-
// Check if entity exists in ANY collection (for better error message)
160-
auto any_entity = ctx_.get_entity_info(entity_id);
161-
if (any_entity.type != EntityType::UNKNOWN) {
162-
// Entity exists but wrong type -> 400
163-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
164-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
165-
to_string(any_entity.sovd_type()),
166-
{{"entity_id", entity_id},
167-
{"expected_type", to_string(expected_type)},
168-
{"actual_type", to_string(any_entity.sovd_type())}});
169-
} else {
170-
// Entity doesn't exist at all -> 404
171-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
172-
{{"entity_id", entity_id}});
173-
}
174-
return;
124+
// Validate entity ID and type for this route
125+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
126+
if (!entity_opt) {
127+
return; // Error response already sent
175128
}
176129

177130
// Determine the full ROS topic path
@@ -250,11 +203,10 @@ void DataHandlers::handle_put_data_item(const httplib::Request & req, httplib::R
250203
entity_id = req.matches[1];
251204
topic_name = req.matches[2];
252205

253-
auto validation_result = ctx_.validate_entity_id(entity_id);
254-
if (!validation_result) {
255-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER, "Invalid entity ID",
256-
{{"details", validation_result.error()}, {"entity_id", entity_id}});
257-
return;
206+
// Validate entity ID and type for this route
207+
auto entity_opt = ctx_.validate_entity_for_route(req, res, entity_id);
208+
if (!entity_opt) {
209+
return; // Error response already sent
258210
}
259211

260212
// Parse request body
@@ -297,28 +249,6 @@ void DataHandlers::handle_put_data_item(const httplib::Request & req, httplib::R
297249
return;
298250
}
299251

300-
// Verify entity exists in the correct collection for this route
301-
auto expected_type = extract_entity_type_from_path(req.path);
302-
auto entity_info = ctx_.get_entity_info(entity_id, expected_type);
303-
if (entity_info.type == EntityType::UNKNOWN) {
304-
// Check if entity exists in ANY collection (for better error message)
305-
auto any_entity = ctx_.get_entity_info(entity_id);
306-
if (any_entity.type != EntityType::UNKNOWN) {
307-
// Entity exists but wrong type -> 400
308-
HandlerContext::send_error(res, StatusCode::BadRequest_400, ERR_INVALID_PARAMETER,
309-
"Invalid entity type for route: expected " + to_string(expected_type) + ", got " +
310-
to_string(any_entity.sovd_type()),
311-
{{"entity_id", entity_id},
312-
{"expected_type", to_string(expected_type)},
313-
{"actual_type", to_string(any_entity.sovd_type())}});
314-
} else {
315-
// Entity doesn't exist at all -> 404
316-
HandlerContext::send_error(res, StatusCode::NotFound_404, ERR_ENTITY_NOT_FOUND, "Entity not found",
317-
{{"entity_id", entity_id}});
318-
}
319-
return;
320-
}
321-
322252
// Build full topic path (mirror GET logic: only prefix '/' when needed)
323253
std::string full_topic_path = topic_name;
324254
if (!full_topic_path.empty() && full_topic_path.front() != '/') {

0 commit comments

Comments
 (0)