Skip to content

Commit 226332c

Browse files
committed
feat(openapi): derive query parameters from a typed query contract
Query parameters were the one part of the HTTP contract not derived from a descriptor. Handlers read them ad-hoc via req.query_param(), and the OpenAPI generator had no way to see them, so the exported spec - and every client generated from it - carried zero query parameters even though the gateway reads them at runtime. Make query parameters a typed, descriptor-driven contract, like request and response bodies: - dto::QueryParamWriter<T> emits the OpenAPI `parameters` (all in: query) for a query DTO from its dto_fields, mirroring SchemaWriter. - TypedRequest::query<T>() parses the request query string into a typed query DTO via the same descriptor. - RouteEntry::query<T>() declares those parameters on the route. A handler can only read fields that exist on its query DTO, and those fields are exactly what the route advertises, so the parsed object and the spec cannot drift. Adding a parameter means adding a member - which appears in the spec automatically. Applied to the endpoints whose handlers read query parameters: - GET /faults and GET /{entity}/faults: status, include_muted, include_clusters - DELETE /faults: status - GET /{entity}/logs: severity, context - GET /updates: origin, target-version A pre-commit guard (check_handlers_typed_query.sh) bans raw query reads in handlers so the typed path stays the only way in. Adds RouteRegistry and TypedRequest tests covering the writer and reader.
1 parent 575ece8 commit 226332c

17 files changed

Lines changed: 411 additions & 32 deletions

File tree

.pre-commit-config.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,12 @@ repos:
7878
stages: [pre-push]
7979
pass_filenames: false
8080
always_run: true
81+
- id: gateway-handlers-typed-query
82+
name: handlers must read query params via typed query<T>()
83+
entry: ./src/ros2_medkit_gateway/scripts/check_handlers_typed_query.sh
84+
language: script
85+
pass_filenames: false
86+
always_run: true
8187

8288
# ── Incremental clang-tidy (pre-push only) ────────────────────────
8389
# Requires: pre-commit install --hook-type pre-push

src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/http/http_utils.hpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <ctime>
2323
#include <filesystem>
2424
#include <fstream>
25+
#include <optional>
2526
#include <string>
2627
#include <vector>
2728

@@ -99,15 +100,15 @@ struct FaultStatusFilter {
99100
};
100101

101102
/**
102-
* @brief Parse fault status query parameter from request
103-
* @param req HTTP request
104-
* @return Filter flags and validity. If status param is invalid, is_valid=false.
103+
* @brief Map a fault `status` query value to a FaultStatusFilter.
104+
* @param status_opt The `status` query parameter value, or nullopt if absent.
105+
* @return Filter flags and validity. If status value is unknown, is_valid=false.
105106
*/
106-
inline FaultStatusFilter parse_fault_status_param(const httplib::Request & req) {
107+
inline FaultStatusFilter parse_fault_status_param(const std::optional<std::string> & status_opt) {
107108
FaultStatusFilter filter;
108109

109-
if (req.has_param("status")) {
110-
std::string status = req.get_param_value("status");
110+
if (status_opt) {
111+
const std::string & status = *status_opt;
111112
// Reset defaults when explicit status filter is provided
112113
filter.include_pending = false;
113114
filter.include_confirmed = false;

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/enums.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ inline constexpr std::string_view kFaultSeverityLabelValues[] = {"INFO", "WARN",
3030
/// Fault aggregated status (fault_detail_schema - status.aggregatedStatus).
3131
inline constexpr std::string_view kFaultAggregatedStatusValues[] = {"active", "passive", "cleared"};
3232

33+
/// Fault status query filter (FaultListQuery.status / FaultClearQuery.status).
34+
/// Mirrors the values parse_fault_status_param() accepts in http_utils.hpp;
35+
/// any other value yields ERR_INVALID_PARAMETER. The leading entry must be a
36+
/// handler-accepted value (the OpenAPI callability test sends enum[0]).
37+
inline constexpr std::string_view kFaultStatusFilterValues[] = {"pending", "confirmed", "cleared", "healed", "all"};
38+
3339
/// Log aggregation level (log_entry_list_schema - x-medkit.aggregation_level).
3440
inline constexpr std::string_view kLogAggregationLevelValues[] = {"function", "area", "component"};
3541

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/faults.hpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,5 +457,35 @@ struct dto_sample<FaultClearResult> {
457457
}
458458
};
459459

460+
// =============================================================================
461+
// FaultListQuery - query parameters for GET /faults and GET /{entity}/faults.
462+
// Read by the handlers via TypedRequest::query<FaultListQuery>() and declared in
463+
// the OpenAPI spec via the same descriptor, so the two cannot drift.
464+
// =============================================================================
465+
struct FaultListQuery {
466+
std::optional<std::string> status;
467+
bool include_muted = false;
468+
bool include_clusters = false;
469+
};
470+
471+
template <>
472+
inline constexpr auto dto_fields<FaultListQuery> = std::make_tuple(
473+
field_enum("status", &FaultListQuery::status, kFaultStatusFilterValues,
474+
"Filter by fault status: pending, confirmed, cleared, healed, or all"),
475+
field("include_muted", &FaultListQuery::include_muted, Presence::kOptional, "Include muted faults in the response"),
476+
field("include_clusters", &FaultListQuery::include_clusters, Presence::kOptional,
477+
"Include fault clusters in the response"));
478+
479+
// FaultClearQuery - query parameters for DELETE /faults (clear all). Only the
480+
// status filter applies; the correlation flags are list-only.
481+
struct FaultClearQuery {
482+
std::optional<std::string> status;
483+
};
484+
485+
template <>
486+
inline constexpr auto dto_fields<FaultClearQuery> =
487+
std::make_tuple(field_enum("status", &FaultClearQuery::status, kFaultStatusFilterValues,
488+
"Clear only faults in this status: pending, confirmed, cleared, healed, or all"));
489+
460490
} // namespace dto
461491
} // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/logs.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,5 +173,21 @@ inline constexpr auto dto_fields<LogConfiguration> = std::make_tuple(
173173
template <>
174174
inline constexpr std::string_view dto_name<LogConfiguration> = "LogConfiguration";
175175

176+
// =============================================================================
177+
// LogQuery - query parameters for GET /{entity}/logs. Read by the handler via
178+
// TypedRequest::query<LogQuery>() and declared in the OpenAPI spec via the same
179+
// descriptor, so the two cannot drift.
180+
// =============================================================================
181+
struct LogQuery {
182+
std::optional<std::string> severity;
183+
std::optional<std::string> context;
184+
};
185+
186+
template <>
187+
inline constexpr auto dto_fields<LogQuery> =
188+
std::make_tuple(field_enum("severity", &LogQuery::severity, kLogSeverityFilterValues,
189+
"Filter by minimum severity: debug, info, warning, error, or fatal"),
190+
field("context", &LogQuery::context, "Filter by logger context substring (max 256 chars)"));
191+
176192
} // namespace dto
177193
} // namespace ros2_medkit_gateway
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2026 bburda
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#pragma once
16+
17+
// Query-parameter contract: the fourth descriptor-driven visitor.
18+
//
19+
// A "query DTO" is a plain struct with a `dto_fields<T>` specialization whose
20+
// members are query-parameter scalars (string / bool, optionally wrapped in
21+
// std::optional). The same descriptor drives two sides so they cannot drift:
22+
//
23+
// * QueryParamWriter<T> - emits the OpenAPI `parameters` array (all
24+
// `in: query`) for the route, mirroring SchemaWriter.
25+
// * TypedRequest::query<T>() (see http/typed_router.hpp) - parses the request
26+
// query string into a typed T, using `assign_query_field` below.
27+
//
28+
// Because a handler can only read fields that exist on T, and those same fields
29+
// are what the spec advertises, a handler cannot read an undeclared query
30+
// parameter. Adding a parameter means adding a member - which appears in the
31+
// spec automatically.
32+
33+
#include <nlohmann/json.hpp>
34+
#include <optional>
35+
#include <string>
36+
#include <type_traits>
37+
#include <utility>
38+
39+
#include "ros2_medkit_gateway/dto/contract.hpp"
40+
#include "ros2_medkit_gateway/dto/schema_writer.hpp"
41+
42+
namespace ros2_medkit_gateway {
43+
namespace dto {
44+
45+
/// Unwrap std::optional<U> -> U, identity otherwise. Query-parameter optionality
46+
/// is expressed via `required:false`, not a nullable schema, so the emitted
47+
/// `schema` uses the unwrapped scalar type.
48+
template <class M>
49+
struct query_value {
50+
using type = M;
51+
};
52+
template <class U>
53+
struct query_value<std::optional<U>> {
54+
using type = U;
55+
};
56+
template <class M>
57+
using query_value_t = typename query_value<M>::type;
58+
59+
/// Emits the OpenAPI `parameters` array (all `in: query`) for a query DTO `T`,
60+
/// derived from the same `dto_fields<T>` descriptor the typed reader consumes.
61+
template <class T>
62+
struct QueryParamWriter {
63+
static nlohmann::json parameters() {
64+
nlohmann::json params = nlohmann::json::array();
65+
for_each_field<T>([&](const auto & f) {
66+
using FieldT = std::decay_t<decltype(f)>;
67+
static_assert(!is_opaque_object_field_v<FieldT>, "query DTOs must not contain opaque-object fields");
68+
using MemberT = std::decay_t<decltype(std::declval<T>().*(f.ptr))>;
69+
70+
nlohmann::json param;
71+
param["name"] = std::string(f.key);
72+
param["in"] = "query";
73+
param["required"] = (f.presence == Presence::kRequired);
74+
if (!f.description.empty()) {
75+
param["description"] = std::string(f.description);
76+
}
77+
nlohmann::json schema = schema_of<query_value_t<MemberT>>();
78+
if (f.enum_count > 0) {
79+
nlohmann::json values = nlohmann::json::array();
80+
for (std::size_t i = 0; i < f.enum_count; ++i) {
81+
values.push_back(std::string(f.enum_values[i]));
82+
}
83+
schema["enum"] = std::move(values);
84+
}
85+
param["schema"] = std::move(schema);
86+
params.push_back(std::move(param));
87+
});
88+
return params;
89+
}
90+
};
91+
92+
/// Assigns a raw query-string value to a typed query-DTO member. Only the scalar
93+
/// shapes a query parameter can take are supported; any other field type is a
94+
/// compile error so unsupported types fail loudly at the route, not silently at
95+
/// runtime.
96+
template <class M>
97+
void assign_query_field(M & member, const std::string & raw) {
98+
if constexpr (std::is_same_v<M, std::string> || std::is_same_v<M, std::optional<std::string>>) {
99+
member = raw;
100+
} else if constexpr (std::is_same_v<M, bool> || std::is_same_v<M, std::optional<bool>>) {
101+
member = (raw == "true");
102+
} else {
103+
static_assert(sizeof(M) == 0, "assign_query_field: unsupported query field type");
104+
}
105+
}
106+
107+
} // namespace dto
108+
} // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/updates.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,5 +289,20 @@ inline constexpr auto dto_fields<UpdateRegisterResponse> = std::make_tuple(field
289289
template <>
290290
inline constexpr std::string_view dto_name<UpdateRegisterResponse> = "UpdateRegisterResponse";
291291

292+
// =============================================================================
293+
// UpdateListQuery - query parameters for GET /updates. Read by the handler via
294+
// TypedRequest::query<UpdateListQuery>() and declared in the OpenAPI spec via
295+
// the same descriptor, so the two cannot drift.
296+
// =============================================================================
297+
struct UpdateListQuery {
298+
std::optional<std::string> origin;
299+
std::optional<std::string> target_version;
300+
};
301+
302+
template <>
303+
inline constexpr auto dto_fields<UpdateListQuery> =
304+
std::make_tuple(field("origin", &UpdateListQuery::origin, "Filter by update origin identifier"),
305+
field("target-version", &UpdateListQuery::target_version, "Filter by target version"));
306+
292307
} // namespace dto
293308
} // namespace ros2_medkit_gateway

src/ros2_medkit_gateway/include/ros2_medkit_gateway/http/typed_router.hpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <string_view>
2222

2323
#include "ros2_medkit_gateway/core/http/error_codes.hpp"
24+
#include "ros2_medkit_gateway/dto/query.hpp"
2425
// Re-export the httplib-free handler-result vocabulary so existing includers of
2526
// typed_router.hpp keep seeing Result/NoContent/Forwarded/ValidatorResult/
2627
// ResponseAttachments. The markers themselves live in this leaf header, which
@@ -94,6 +95,23 @@ class TypedRequest {
9495
return req_.get_param_value(name_str.c_str());
9596
}
9697

98+
/// Parses the request's query string into a typed query DTO `T`, using the
99+
/// same `dto_fields<T>` descriptor that declares the parameters in the OpenAPI
100+
/// spec (via `dto::QueryParamWriter<T>`). Absent parameters leave their member
101+
/// at its default. This is the sanctioned way for handlers to read query
102+
/// parameters: a handler cannot read a parameter the route did not declare,
103+
/// because it can only access members of `T`.
104+
template <class T>
105+
T query() const {
106+
T out{};
107+
dto::for_each_field<T>([&](const auto & f) {
108+
if (auto raw = query_param(f.key)) {
109+
dto::assign_query_field(out.*(f.ptr), *raw);
110+
}
111+
});
112+
return out;
113+
}
114+
97115
/// Returns the value of the named header, or std::nullopt if absent.
98116
std::optional<std::string> header(std::string_view name) const {
99117
// Note: see query_param() comment - Humble compatibility requires c_str().
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
#!/usr/bin/env bash
2+
# Copyright 2026 bburda
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
# Asserts that HTTP handlers read query parameters ONLY through the typed
17+
# TypedRequest::query<T>() contract, never via raw req.query_param() or
18+
# get_param_value().
19+
#
20+
# Why: the typed path derives the OpenAPI `parameters` from the same query-DTO
21+
# descriptor (dto::QueryParamWriter<T>) that the handler parses. A raw read is
22+
# invisible to the spec, so the parameter never reaches the generated clients -
23+
# exactly the regression that left every query parameter out of the published
24+
# 0.5.0 clients. Forcing the typed path makes that drift impossible: a handler
25+
# can only read fields that exist on its query DTO, and those fields are what the
26+
# route declares.
27+
#
28+
# To add a query parameter: define a query DTO (struct + dto_fields), declare it
29+
# on the route with .query<T>(), and read it with req.query<T>(). See
30+
# include/ros2_medkit_gateway/dto/query.hpp.
31+
set -euo pipefail
32+
33+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
34+
HANDLERS_DIR="$(cd "${SCRIPT_DIR}/../src/http/handlers" && pwd)"
35+
36+
# Path reads (req.path), header reads (req.header), and fan-out (raw_for_framework
37+
# for path/Authorization) are legitimate; only query-string reads are banned.
38+
pattern='\.query_param\(|->query_param\(|get_param_value\('
39+
40+
hits="$(grep -rnE "${pattern}" "${HANDLERS_DIR}" --include='*.cpp' || true)"
41+
if [[ -n "${hits}" ]]; then
42+
echo "ERROR: handlers must read query parameters via TypedRequest::query<T>(), not raw query reads."
43+
echo "Offending lines:"
44+
echo "${hits}"
45+
echo
46+
echo "Fix: define a query DTO (struct + dto_fields), declare it on the route with"
47+
echo ".query<T>(), and read it via req.query<T>(). See dto/query.hpp."
48+
exit 1
49+
fi
50+
51+
echo "OK: handlers read query parameters only via the typed query<T>() contract."

src/ros2_medkit_gateway/src/core/openapi/route_registry.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ RouteEntry & RouteEntry::query_param(const std::string & name, const std::string
101101
return *this;
102102
}
103103

104+
RouteEntry & RouteEntry::add_query_parameters(const nlohmann::json & params) {
105+
for (const auto & param : params) {
106+
parameters_.push_back(param);
107+
}
108+
return *this;
109+
}
110+
104111
RouteEntry & RouteEntry::header_param(const std::string & name, const std::string & desc, bool required,
105112
const nlohmann::json & schema) {
106113
nlohmann::json param;

0 commit comments

Comments
 (0)