refactor(gateway): introduce core/ layer scaffolding for provider injection#392
Draft
refactor(gateway): introduce core/ layer scaffolding for provider injection#392
Conversation
Move provider abstract interfaces (DataProvider, OperationProvider, FaultProvider, LogProvider, ScriptProvider, UpdateProvider, IntrospectionProvider, HostInfoProvider) to the middleware-neutral core layer. No logic, no signature changes. Provider concrete implementations will be relocated alongside their respective manager refactors.
REST server router, rate limiter, SSE client tracker, HTTP server, HTTP utilities, x-medkit vendor extensions, error codes, warning codes, entity path utilities, and fan-out helpers carry no transport-layer ROS dependencies and belong to the middleware-neutral core.
…andlers/ Handlers verified middleware-neutral by includes audit. Handlers with transitive logger or subscription dependencies (handler_context, fault_handlers, sse_fault_handler, cyclic_subscription_handlers) remain at their current paths and will relocate alongside the corresponding manager refactors.
JWT authentication, role-based authorization, middleware policy, and auth model types form a self-contained, middleware-neutral subsystem.
…aggregation/ Peer client, entity merger, mDNS discovery, network utilities, SSE stream proxy, and classification utilities are HTTP-only and middleware-neutral. The aggregation_manager orchestrator currently couples to the gateway node via rclcpp::Logger and remains in place for a follow-up refactor.
…/models/ Area, Component, App, and Function entity types - the SOVD entity model shared across all discovery layers - are pure data structures with no middleware dependencies.
Lock manager, bulk data store, subscription manager, script manager, update manager, trigger manager, entity cache, and related transport and type headers are middleware-neutral and form the core business-logic layer. Plugin framework, OpenAPI, data types, discovery enums, strategy interfaces, manifest parser, and manifest/layer headers are also neutral and are co-located under their respective core/ subdirectories. Managers that today take rclcpp::Node* (data access, operation, configuration, fault facade, log, trigger subscriber) remain in place pending their provider injection refactors.
Every source file whose header was relocated under core/ in earlier commits and which itself carries no ROS includes joins its header under the matching src/core/ path. A handful of source files remain in place because they contain ROS-specific code that will move only once the corresponding manager refactor lands (rest_server, http_server, sse_transport_provider, trigger_handlers, default_script_provider, script_manager, plugin_loader, plugin_manager, plugin_context).
Introduces the medkit:: neutral namespace and an alias mapping the legacy ros2_medkit_gateway:: namespace onto it. Allows call sites that still reference the legacy namespace to compile while individual files migrate to the new namespace one subsystem at a time.
Updates every #include directive in the package to reflect the relocated header paths under ros2_medkit_gateway/core/. Pure mechanical rewrite — no source-content edits beyond include statements. Generated by sed substitution against the rename history of the preceding eight commits.
gateway_core is a position-independent static library compiled from sources under src/core/ via GLOB_RECURSE; it links only header-only and C-level externals (cpp-httplib, nlohmann/json, yaml-cpp, tl::expected, jwt-cpp, OpenSSL, SQLite, dl) and carries no rclcpp dependency. gateway_ros2 is the ROS adapter static library compiled from the remaining sources under src/. It depends on rclcpp / rcl_interfaces / action_msgs / ros2_medkit_msgs / ros2_medkit_serialization / rosidl_typesupport via medkit_target_dependencies and links gateway_core so the executable and tests get both layers from a single dependency. Source audit during build iteration moved 22 files from src/core/ back to src/ because their include chains transitively pull in rclcpp or ros2_medkit_serialization: all HTTP handler implementations (via handler_context.hpp -> rclcpp), openapi spec/schema builders (via schema_builder.hpp -> ros2_medkit_serialization), type_introspection, manifest_parser, manifest_layer, runtime_layer, trigger_manager, and plugin_http_types. path_resolver.cpp and route_registry.cpp remain in gateway_core as they have no ROS dependencies. The gateway_node executable now links gateway_ros2; all test targets follow. PCH split between layers: gateway_core's PCH contains std + nlohmann/json + httplib + tl::expected; gateway_ros2 extends with rclcpp/rclcpp.hpp. Private openapi headers in src/openapi/ are exposed to both targets via PRIVATE target_include_directories. All 81 unit test targets pass (2098 assertions).
Greps include/ros2_medkit_gateway/core/ and src/core/ for any direct include of ROS-specific headers (rclcpp, rcl_interfaces, rosidl, action_msgs, message-package families, ros2_medkit_msgs, rcutils, rmw). Exits non-zero on any match. Wired into CTest as a linter target by the next commit so the build fails when the neutral layer contract is violated.
Adds gateway_core_purity to the linter-labeled CTest suite so it runs in the linter CI job alongside ament_lint_cmake, clang-format, and the other static checks. Build fails on any direct ROS include leaked into the neutral core layer.
Compiles a translation unit including a sampling of core/ public headers (entity model types, provider interface contracts) and links exclusively against gateway_core + GTest. No ament_target_dependencies, no rclcpp on the link line. A ROS transitive include silently reaching the core layer surfaces here as a missing-symbol link error, catching leaks that the grep-based purity script can miss when the include is reached through a third-party header.
Adds static_asserts and namespace using-declarations for each included entity model (Area, Component, App, Function) and provider interface (DataProvider, OperationProvider, FaultProvider, LogProvider, IntrospectionProvider) so the includes are recognized as used by include-cleaner tooling. The link-time invariant still rests on linking gateway_core alone with no ament_target_dependencies.
Plugins consuming the gateway's public API still reference the pre-scaffold include paths (plugins/, providers/, discovery/models/, http/error_codes.hpp, discovery/introspection_provider.hpp). Add forwarding shims at the old paths so downstream packages (graph_provider, opcua, sovd_service_interface, linux introspection, param_beacon, topic_beacon) compile without change.
Add missing Rule of Five = delete declarations to classes with user-defined destructors: SSEStreamProxy, ConfigurationManager, DefaultScriptProvider, Ros2TopicDataProvider, RESTServer, LogManager, OperationManager. Add move = delete to SqliteStatement (already had copy = delete). Fix NotifyGuard anonymous struct to satisfy the rule. Fix performance-unnecessary-value-param in Ros2TopicDataProvider subscription callback. Wrap main() body in outer try/catch to prevent exception escape (bugprone-exception-escape).
Removes a timing race in TestCombinedIntrospection.test_01_procfs_returns_200_on_host where a fresh process can momentarily report RssAnon=0 in /proc/<pid>/status before any memory has been touched. The poll helper now waits for both a 200 response AND rss_bytes > 0 before returning, eliminating intermittent '0 not greater than 0' failures observed in local runs.
readability-named-parameter: name the unused override parameters in the mock plugin classes via comment placeholders so the compiler-visible parameter list satisfies the rule without changing semantics. performance-inefficient-vector-operation: reserve the std::thread vector capacity in the three concurrency stress tests before the emplace_back loops so no incremental reallocation happens during the hot path. performance-for-range-copy: take the test parameter pair by const reference instead of by value in CyclicSubscriptionJsonTest.
The neutral entity model imports nlohmann/json under the alias ros2_medkit_gateway::json (defined in core/auth/auth_models.hpp). Two test files re-declared a local using-alias of the same name, which clang-diagnostic-shadow flags as an error once the test pulls in any core/ header that exposes the namespace alias. Drop the local alias and rely on the namespace import.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Splits
src/ros2_medkit_gateway/into a middleware-neutralcore/layer and the existing top-level layout (ROS adapters), introducing two named build targets and two structural CI guards as outlined in the linked issue. Pure structural change — no business logic edits, no signature changes, no public API changes for consumers. Subsequent issues will cover the per-manager refactors (data access, operation, configuration, fault facade, log, trigger) that introduce provider injection so the managers themselves move intocore/.What changed:
src/core/(include/ros2_medkit_gateway/core/for headers). Newgateway_coreSTATIC library compiles them with header-only and C-level externals (cpp-httplib, nlohmann/json, yaml-cpp, tl::expected, jwt-cpp, OpenSSL, SQLite, dl) and zero ROS dependencies.gateway_ros2STATIC library that publicly linksgateway_core.gateway_nodeand the existing test targets linkgateway_ros2, so they transitively get both layers.#includepath inside the package rewritten via sed against the rename history of the relocation commits — pure mechanical edit, no other source changes.compat.hppintroducesnamespace ros2_medkit_gateway = medkit;so call sites that still reference the legacy namespace continue to compile during the gradual migration.providers/,plugins/,discovery/models/,http/error_codes.hpp,discovery/introspection_provider.hpp) so out-of-tree consumers (graph_provider, opcua, sovd_service_interface, linux_introspection, param_beacon, topic_beacon) compile without modification.scripts/check_core_purity.shgrepscore/for any direct ROS include and is wired as thegateway_core_purityCTest under thelinterlabel. Smoke-tested by planting a#include <rclcpp/rclcpp.hpp>— correctly rejected.test_gateway_core_smokecompiles a translation unit that includes a sampling ofcore/headers and links exclusively againstgateway_core+ GTest, with noament_target_dependencies. Catches transitive ROS coupling that the grep guard might miss.gateway_corePCH covers std +nlohmann/json.hpp+httplib.h+tl/expected.hpp;gateway_ros2PCH addsrclcpp/rclcpp.hppon top.cppcoreguidelines-special-member-functionsRule of Five,bugprone-exception-escapeonmain,performance-unnecessary-value-param) surfaced by the new build setup.test_combined_introspection.test_01_procfs_returns_200_on_hostwhere a freshly forked process could momentarily reportRssAnon=0before any memory was touched. The poll helper now waits forrss_bytes > 0before asserting.Issue
Type
Testing
Local validation on Jazzy:
colcon build --symlink-install(15 packages)./scripts/test.sh unit./scripts/test.sh lint --packages-select ros2_medkit_gatewaygateway_core_puritylinter)./scripts/test.sh integ --packages-select ros2_medkit_integration_tests./scripts/test.sh tidy --packages-select ros2_medkit_gatewaycolcon test --ctest-args -R gateway_core_smokegateway_corealone with noament_target_dependenciesgateway_core_purityplanted-violation smoke test#include <rclcpp/rclcpp.hpp>incore/was correctly rejectedReviewers should verify: (1)
gateway_core_purityruns as part of the linter job, (2)test_gateway_core_smokebuilds and runs without ament dependencies, (3) downstream packages that consume the gateway's headers still compile via the shim layer.Checklist
compat.hppalias and shim headersgateway_core_puritylinter,test_gateway_core_smokelink test, and a polling fix intest_combined_introspection