mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-12 01:45:14 +02:00
mr-668: OpenAPI multi-mode cluster filter (PR 4b/10)
PR 4b of the MR-668 multi-graph server work. In multi mode, the served
`/openapi.json` reports cluster routes (`/graphs/{graph_id}/...`) instead
of the legacy flat protected paths — matching what `build_app` actually
mounts (PR 4a's `Router::nest`). Single mode is unchanged.
Implementation:
- New `server_openapi` branch: when `state.mode()` is `Multi`, call
`nest_paths_under_cluster_prefix(&mut doc)` after `ApiDoc::openapi()`.
- The rewrite consumes `doc.paths.paths`, then for every path-item:
- If the path is in `ALWAYS_FLAT_PATHS` (`/healthz` for now), keep
it flat.
- Otherwise, prefix every operation_id with `cluster_` and reinsert
the item at `/graphs/{graph_id}<original_path>`.
- Single mode hits no extra work — the path map is untouched.
- The static `ApiDoc::openapi()` still emits the flat surface, so
in-process callers (the existing `openapi_json()` helper in tests)
see the unmodified spec.
Why cluster_ prefix on operation IDs: OpenAPI specs require unique
operation_ids across the document. With both flat (single-mode) and
cluster (multi-mode) surfaces ever co-existing in a generated SDK,
the prefix prevents collision. The current served doc only carries
one surface, so the prefix is forward-compat with potential future
dual-surface generation.
Tests: 6 new in `tests/openapi.rs`, all via the `/openapi.json` route
(not the static `ApiDoc::openapi()` helper):
- `multi_mode_openapi_lists_cluster_paths` — every protected path
appears as a cluster variant.
- `multi_mode_openapi_drops_flat_protected_paths` — flat protected
paths are absent.
- `multi_mode_openapi_keeps_healthz_flat` — `/healthz` survives.
- `multi_mode_openapi_prefixes_operation_ids_with_cluster` — every
cluster operation_id starts with `cluster_`.
- `multi_mode_operation_ids_are_unique` — no operation_id collisions.
- `single_mode_openapi_unchanged_by_cluster_filter` — single mode
still emits the legacy flat surface (regression).
New test helper `app_for_multi_mode(graph_ids)` exercises the new
`AppState::new_multi` constructor from PR 4a — first user of multi-mode
construction outside of unit tests.
Result: 66 openapi tests + 57 server integration tests + 74 lib tests
= 197 green. No regression in the existing OpenAPI drift check
(`openapi_spec_is_up_to_date` still validates the static flat surface
matches the committed openapi.json).
LOC: +67 in lib.rs (rewrite logic), +219 in tests/openapi.rs (test
suite + helper).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ragnor Comerford
parent
57f5f191ab
commit
ecf01ef3fe
2 changed files with 286 additions and 0 deletions
|
|
@ -889,9 +889,76 @@ async fn server_openapi(State(state): State<AppState>) -> Json<utoipa::openapi::
|
|||
if !state.requires_bearer_auth() {
|
||||
strip_security(&mut doc);
|
||||
}
|
||||
// MR-668 PR 4b: in multi mode, the protected routes live under
|
||||
// `/graphs/{graph_id}/...`. Rewrite the doc so the spec matches
|
||||
// the routes the router actually serves. Public paths (`/healthz`)
|
||||
// stay flat in both modes.
|
||||
if matches!(state.mode(), ServerMode::Multi { .. }) {
|
||||
nest_paths_under_cluster_prefix(&mut doc);
|
||||
}
|
||||
Json(doc)
|
||||
}
|
||||
|
||||
/// Path prefix used to namespace per-graph routes in multi mode.
|
||||
/// Kept in sync with the `Router::nest(...)` invocation in `build_app`.
|
||||
const CLUSTER_PATH_PREFIX: &str = "/graphs/{graph_id}";
|
||||
|
||||
/// Operation-id prefix applied to every cloned cluster operation.
|
||||
/// Decision 7 in the implementation plan — keeps operation IDs unique
|
||||
/// across the spec when both flat and nested variants ever appear in
|
||||
/// the same generation pass.
|
||||
const CLUSTER_OPERATION_ID_PREFIX: &str = "cluster_";
|
||||
|
||||
/// Paths that stay flat in every server mode (public, no per-graph
|
||||
/// dependency). Update this list when adding new always-public endpoints.
|
||||
const ALWAYS_FLAT_PATHS: &[&str] = &["/healthz"];
|
||||
|
||||
/// In multi-mode `server_openapi`, every protected path-item is
|
||||
/// reattached under the cluster prefix. Operation IDs gain the
|
||||
/// `cluster_` prefix so SDK generators don't collide if/when both
|
||||
/// surfaces are merged. The `{graph_id}` URL placeholder is left
|
||||
/// implicit in the path; consuming clients see it as a standard
|
||||
/// OpenAPI path parameter.
|
||||
///
|
||||
/// Removing the flat protected paths matches the runtime router —
|
||||
/// in multi mode, requests to `/snapshot` etc. return 404, so the
|
||||
/// spec must agree.
|
||||
fn nest_paths_under_cluster_prefix(doc: &mut utoipa::openapi::OpenApi) {
|
||||
let original = std::mem::take(&mut doc.paths.paths);
|
||||
let mut rewritten = std::collections::BTreeMap::new();
|
||||
for (path, mut item) in original {
|
||||
if ALWAYS_FLAT_PATHS.contains(&path.as_str()) {
|
||||
rewritten.insert(path, item);
|
||||
continue;
|
||||
}
|
||||
rename_operation_ids(&mut item, CLUSTER_OPERATION_ID_PREFIX);
|
||||
let new_path = format!("{CLUSTER_PATH_PREFIX}{path}");
|
||||
rewritten.insert(new_path, item);
|
||||
}
|
||||
doc.paths.paths = rewritten;
|
||||
}
|
||||
|
||||
/// Prefix every operation_id in this PathItem with `prefix`.
|
||||
fn rename_operation_ids(item: &mut utoipa::openapi::PathItem, prefix: &str) {
|
||||
for op in [
|
||||
item.get.as_mut(),
|
||||
item.post.as_mut(),
|
||||
item.put.as_mut(),
|
||||
item.delete.as_mut(),
|
||||
item.options.as_mut(),
|
||||
item.head.as_mut(),
|
||||
item.patch.as_mut(),
|
||||
item.trace.as_mut(),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
{
|
||||
if let Some(id) = op.operation_id.as_deref() {
|
||||
op.operation_id = Some(format!("{prefix}{id}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_security(doc: &mut utoipa::openapi::OpenApi) {
|
||||
if let Some(components) = doc.components.as_mut() {
|
||||
components.security_schemes.clear();
|
||||
|
|
|
|||
|
|
@ -957,3 +957,222 @@ fn openapi_spec_is_up_to_date() {
|
|||
"openapi.json is out of date. Run: OMNIGRAPH_UPDATE_OPENAPI=1 cargo test -p omnigraph-server --test openapi openapi_spec_is_up_to_date"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// MR-668 PR 4b — multi-mode OpenAPI cluster filter
|
||||
// ---------------------------------------------------------------------------
|
||||
//
|
||||
// In multi-graph mode, `/openapi.json` reports cluster routes
|
||||
// (`/graphs/{graph_id}/...`) instead of the legacy flat routes. The
|
||||
// only flat path that survives is `/healthz`. Operation IDs gain a
|
||||
// `cluster_` prefix so SDK generators have stable, unique ids.
|
||||
//
|
||||
// These tests exercise the request-time `server_openapi` handler via
|
||||
// `oneshot`, not the static `ApiDoc::openapi()` — the rewrite happens
|
||||
// only on the served document.
|
||||
|
||||
const EXPECTED_CLUSTER_PATHS: &[&str] = &[
|
||||
"/graphs/{graph_id}/snapshot",
|
||||
"/graphs/{graph_id}/read",
|
||||
"/graphs/{graph_id}/export",
|
||||
"/graphs/{graph_id}/change",
|
||||
"/graphs/{graph_id}/schema",
|
||||
"/graphs/{graph_id}/schema/apply",
|
||||
"/graphs/{graph_id}/ingest",
|
||||
"/graphs/{graph_id}/branches",
|
||||
"/graphs/{graph_id}/branches/{branch}",
|
||||
"/graphs/{graph_id}/branches/merge",
|
||||
"/graphs/{graph_id}/commits",
|
||||
"/graphs/{graph_id}/commits/{commit_id}",
|
||||
];
|
||||
|
||||
async fn app_for_multi_mode(graph_ids: &[&str]) -> (Vec<tempfile::TempDir>, Router) {
|
||||
use std::sync::Arc;
|
||||
|
||||
use omnigraph_server::{GraphHandle, GraphId, GraphKey};
|
||||
|
||||
let mut dirs = Vec::with_capacity(graph_ids.len());
|
||||
let mut handles = Vec::with_capacity(graph_ids.len());
|
||||
for id in graph_ids {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let graph_uri = dir.path().join(id).to_str().unwrap().to_string();
|
||||
let schema = fs::read_to_string(fixture("test.pg")).unwrap();
|
||||
let engine = Omnigraph::init(&graph_uri, &schema).await.unwrap();
|
||||
handles.push(Arc::new(GraphHandle {
|
||||
key: GraphKey::cluster(GraphId::try_from(*id).unwrap()),
|
||||
uri: graph_uri,
|
||||
engine: Arc::new(engine),
|
||||
policy: None,
|
||||
}));
|
||||
dirs.push(dir);
|
||||
}
|
||||
let workload = omnigraph_server::workload::WorkloadController::from_env();
|
||||
let state = AppState::new_multi(handles, Vec::new(), None, workload, None).unwrap();
|
||||
let app = build_app(state);
|
||||
(dirs, app)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_mode_openapi_lists_cluster_paths() {
|
||||
let (_dirs, app) = app_for_multi_mode(&["alpha"]).await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (status, json) = json_response(&app, request).await;
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
let paths = json["paths"].as_object().expect("paths must be an object");
|
||||
let path_keys: HashSet<&str> = paths.keys().map(|k| k.as_str()).collect();
|
||||
for expected in EXPECTED_CLUSTER_PATHS {
|
||||
assert!(
|
||||
path_keys.contains(expected),
|
||||
"missing cluster path in multi-mode spec: {expected}. \
|
||||
Found: {path_keys:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_mode_openapi_drops_flat_protected_paths() {
|
||||
let (_dirs, app) = app_for_multi_mode(&["alpha"]).await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (_, json) = json_response(&app, request).await;
|
||||
let paths = json["paths"].as_object().unwrap();
|
||||
// None of the legacy flat protected paths should appear in multi mode.
|
||||
let flat_protected = [
|
||||
"/snapshot",
|
||||
"/read",
|
||||
"/export",
|
||||
"/change",
|
||||
"/schema",
|
||||
"/schema/apply",
|
||||
"/ingest",
|
||||
"/branches",
|
||||
"/branches/{branch}",
|
||||
"/branches/merge",
|
||||
"/commits",
|
||||
"/commits/{commit_id}",
|
||||
];
|
||||
for flat in flat_protected {
|
||||
assert!(
|
||||
!paths.contains_key(flat),
|
||||
"flat path {flat} must not appear in multi-mode spec; \
|
||||
cluster routes are the only protected surface"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_mode_openapi_keeps_healthz_flat() {
|
||||
let (_dirs, app) = app_for_multi_mode(&["alpha"]).await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (_, json) = json_response(&app, request).await;
|
||||
let paths = json["paths"].as_object().unwrap();
|
||||
assert!(
|
||||
paths.contains_key("/healthz"),
|
||||
"/healthz must remain flat in multi mode"
|
||||
);
|
||||
assert!(
|
||||
!paths.contains_key("/graphs/{graph_id}/healthz"),
|
||||
"/healthz must NOT be cluster-prefixed"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_mode_openapi_prefixes_operation_ids_with_cluster() {
|
||||
let (_dirs, app) = app_for_multi_mode(&["alpha"]).await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (_, json) = json_response(&app, request).await;
|
||||
// Every cluster path operation must have a `cluster_` operation_id.
|
||||
let paths = json["paths"].as_object().unwrap();
|
||||
let mut checked = 0;
|
||||
for (path, item) in paths {
|
||||
if path == "/healthz" {
|
||||
continue;
|
||||
}
|
||||
for method in ["get", "post", "put", "delete", "patch"] {
|
||||
if let Some(op) = item.get(method).filter(|v| v.is_object()) {
|
||||
if let Some(id) = op["operationId"].as_str() {
|
||||
assert!(
|
||||
id.starts_with("cluster_"),
|
||||
"operation_id at {path}.{method} must start with `cluster_`, got `{id}`"
|
||||
);
|
||||
checked += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
checked >= EXPECTED_CLUSTER_PATHS.len(),
|
||||
"expected at least {} cluster operation_ids; checked {checked}",
|
||||
EXPECTED_CLUSTER_PATHS.len()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_mode_operation_ids_are_unique() {
|
||||
// Sanity check: the cluster_ prefix prevents collision with flat ids
|
||||
// (which don't appear in multi mode, but the contract is "unique
|
||||
// across the spec"). Verify every operation_id in the multi-mode
|
||||
// spec is unique.
|
||||
let (_dirs, app) = app_for_multi_mode(&["alpha"]).await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (_, json) = json_response(&app, request).await;
|
||||
let paths = json["paths"].as_object().unwrap();
|
||||
let mut seen_ids: HashSet<String> = HashSet::new();
|
||||
for (_, item) in paths {
|
||||
for method in ["get", "post", "put", "delete", "patch"] {
|
||||
if let Some(op) = item.get(method).filter(|v| v.is_object()) {
|
||||
if let Some(id) = op["operationId"].as_str() {
|
||||
assert!(
|
||||
seen_ids.insert(id.to_string()),
|
||||
"duplicate operation_id `{id}` in multi-mode spec"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn single_mode_openapi_unchanged_by_cluster_filter() {
|
||||
// Regression: single mode still emits the legacy flat surface.
|
||||
let (_temp, app) = app_for_loaded_graph().await;
|
||||
let request = Request::builder()
|
||||
.method(Method::GET)
|
||||
.uri("/openapi.json")
|
||||
.body(Body::empty())
|
||||
.unwrap();
|
||||
let (_, json) = json_response(&app, request).await;
|
||||
let paths = json["paths"].as_object().unwrap();
|
||||
let path_keys: HashSet<&str> = paths.keys().map(|k| k.as_str()).collect();
|
||||
for expected in EXPECTED_PATHS {
|
||||
assert!(
|
||||
path_keys.contains(expected),
|
||||
"single mode must still emit flat path: {expected}"
|
||||
);
|
||||
}
|
||||
for cluster in EXPECTED_CLUSTER_PATHS {
|
||||
assert!(
|
||||
!path_keys.contains(cluster),
|
||||
"single mode must NOT emit cluster path: {cluster}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue