[#142] Consolidate discovery handlers into unified class #143

bburda · 2026-01-25T21:38:43Z

Data endpoints unified across all entity types via DataHandlers.

Pull Request

Summary

Create unified DiscoveryHandlers class with entity-agnostic methods
Create unified DataHandlers class for data operations across entities
Add AggregatedData and TopicData structs to ThreadSafeEntityCache
Remove handlers/discovery/ folder (8 files, ~1600 lines)
Update rest_server to use discovery_handlers_ and data_handlers_
~55% code reduction through handler consolidation

Discovery endpoints now handled by single class:

Areas: list, get, components, subareas, contains
Components: list, get, subcomponents, hosts, depends-on
Apps: list, get, depends-on
Functions: list, get, hosts

Issue

Link the related issue (required):

closes Consolidate discovery handlers into a single unified DiscoveryHandlers #142

Type

Bug fix
New feature or tests
Breaking change
Documentation only

Testing

How was this tested / how should reviewers verify it?

Checklist

Breaking changes are clearly described (and announced in docs / changelog if needed)
Tests were added or updated if needed
Docs were updated if behavior or public API changed

- Create unified DiscoveryHandlers class with entity-agnostic methods - Create unified DataHandlers class for data operations across entities - Add AggregatedData and TopicData structs to ThreadSafeEntityCache - Remove handlers/discovery/ folder (8 files, ~1600 lines) - Update rest_server to use discovery_handlers_ and data_handlers_ - ~55% code reduction through handler consolidation Discovery endpoints now handled by single class: - Areas: list, get, components, subareas, contains - Components: list, get, subcomponents, hosts, depends-on - Apps: list, get, depends-on - Functions: list, get, hosts Data endpoints unified across all entity types via DataHandlers.

Copilot

Pull request overview

Consolidates entity discovery and data endpoints by replacing per-entity handler classes with unified DiscoveryHandlers and DataHandlers, and adds cache-level data aggregation to support entity-agnostic /data operations across areas/components/apps/functions.

Changes:

Replaced per-entity discovery handlers with a unified DiscoveryHandlers.
Introduced unified DataHandlers plus new ThreadSafeEntityCache data aggregation APIs/structs to support /data across entity types.
Updated REST routing and build configuration; removed legacy handlers/discovery/* handler files.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp	Adds data aggregation implementation used by unified data endpoints.
src/ros2_medkit_gateway/src/http/rest_server.cpp	Rewires routes to unified handlers; adds new area/function data routes.
src/ros2_medkit_gateway/src/http/handlers/discovery_handlers.cpp	New consolidated discovery endpoint implementations.
src/ros2_medkit_gateway/src/http/handlers/discovery/function_handlers.cpp	Removed legacy function discovery handler implementation.
src/ros2_medkit_gateway/src/http/handlers/discovery/component_handlers.cpp	Removed legacy component discovery/data handler implementation.
src/ros2_medkit_gateway/src/http/handlers/discovery/area_handlers.cpp	Removed legacy area discovery handler implementation.
src/ros2_medkit_gateway/src/http/handlers/discovery/app_handlers.cpp	Removed legacy app discovery/data handler implementation.
src/ros2_medkit_gateway/src/http/handlers/data_handlers.cpp	New consolidated `/data` endpoint implementations across entity types.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/models/thread_safe_entity_cache.hpp	Adds `AggregatedData`/`TopicData` and new data aggregation APIs.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/rest_server.hpp	Updates handler member fields to unified handler classes.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/handlers.hpp	Updates umbrella handler includes to unified discovery/data handlers.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery_handlers.hpp	New public header for consolidated discovery handlers.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery/function_handlers.hpp	Removed legacy function handler header.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery/component_handlers.hpp	Removed legacy component handler header.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery/area_handlers.hpp	Removed legacy area handler header.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery/app_handlers.hpp	Removed legacy app handler header.
src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/data_handlers.hpp	New public header for consolidated data handlers.
src/ros2_medkit_gateway/CMakeLists.txt	Removes legacy discovery handler sources; adds new unified handler sources.

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

src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp

src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/handlers/discovery_handlers.hpp

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

src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp

src/ros2_medkit_gateway/src/http/rest_server.cpp

src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp

- Add missing #include <algorithm> for std::count in data_handlers.cpp - Fix handle_list_data() to check entity existence before source_ids - Fix handle_put_data_item() to mirror GET logic for full_topic_path - Change discovery_handlers.hpp to use #pragma once - Add Cap::FAULTS to handle_get_app() capabilities list - Add entity_id to source_ids in get_area_data/get_function_data - Fix direction='both' detection in collect_topics_from_app/component These changes ensure: - Consistent include guard style across codebase - Correct 200/404 for empty vs missing entities - No double slash in topic paths for PUT - Accurate pub/sub direction metadata in aggregated responses

…andlers Add integration tests for aggregated data endpoints: - test_109: GET /areas/{area_id}/data for valid area - test_110: GET /areas/{area_id}/data for nonexistent area - test_111: GET /areas/root/data for system-wide aggregation - test_112: GET /areas/{area_id}/data returns 200 for empty areas - test_113: GET /functions/{function_id}/data for valid function - test_114: GET /functions/{function_id}/data for nonexistent function - test_115: GET /functions/{function_id}/data rejects invalid IDs Fix existing tests for unified handler error messages: - Update tests to expect 'Entity not found' / 'Invalid entity ID' - Update tests to use 'entity_id' parameter in error responses - Add readiness helpers to tests 07-10 for discovery race condition - Allow direction='both' in topic data assertions - Use 'aggregated_from' field name in new tests Apply clang-format to handler files.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

src/ros2_medkit_gateway/src/models/thread_safe_entity_cache.cpp

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

src/ros2_medkit_gateway/test/test_integration.test.py

Changes: - Fix get_area_data() to recursively traverse subareas using area_to_subareas_ for proper area hierarchy aggregation behavior - Fix handle_list_data() to use topic.name as ID instead of normalized form, making IDs directly usable for GET/PUT calls (clients URL-encode as needed) - Fix _ensure_app_data_ready() helper to poll /apps/{id}/data endpoint instead of /apps/{id} to match what tests actually depend on - Fix unused topic_names variable in test_111 by adding it to print output - Fix function discovery: call discover_functions() in refresh_cache() and pass results to thread_safe_cache_.update_all() (was passing empty {}) - Apply clang-format to modified files This fixes issues with: - Area data not aggregating topics from subareas - Data list IDs not being usable for subsequent GET/PUT item requests - Test flakiness due to polling wrong endpoint - Function data/operations endpoints returning 404 (functions not in cache)

bburda requested a review from mfaferek93 January 25, 2026 21:38

bburda self-assigned this Jan 25, 2026

Copilot AI review requested due to automatic review settings January 25, 2026 21:38

Copilot started reviewing on behalf of bburda January 25, 2026 21:38 View session

Copilot AI reviewed Jan 25, 2026

View reviewed changes

bburda added 2 commits January 26, 2026 07:14

bburda requested a review from Copilot January 26, 2026 07:29

Copilot started reviewing on behalf of bburda January 26, 2026 07:29 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#142] Consolidate discovery handlers into unified class #143

[#142] Consolidate discovery handlers into unified class #143

Uh oh!

bburda commented Jan 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[#142] Consolidate discovery handlers into unified class #143

Are you sure you want to change the base?

[#142] Consolidate discovery handlers into unified class #143

Uh oh!

Conversation

bburda commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request

Summary

Issue

Type

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bburda commented Jan 25, 2026 •

edited

Loading