From ecf01ef3fea3d2474621ec8236b40ffb00bcd5c2 Mon Sep 17 00:00:00 2001 From: Ragnor Comerford Date: Mon, 25 May 2026 19:59:02 +0200 Subject: [PATCH] mr-668: OpenAPI multi-mode cluster filter (PR 4b/10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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}`. - 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) --- crates/omnigraph-server/src/lib.rs | 67 +++++++ crates/omnigraph-server/tests/openapi.rs | 219 +++++++++++++++++++++++ 2 files changed, 286 insertions(+) diff --git a/crates/omnigraph-server/src/lib.rs b/crates/omnigraph-server/src/lib.rs index 8ee9f6d..9737da9 100644 --- a/crates/omnigraph-server/src/lib.rs +++ b/crates/omnigraph-server/src/lib.rs @@ -889,9 +889,76 @@ async fn server_openapi(State(state): State) -> Json (Vec, 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 = 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}" + ); + } +}