feat(adapters): path-item params merge, response wildcards, loud webhooks (OAI-13)
- path-item-level `parameters` parse into PathItem and merge into every operation's input schema; operation-level entries override shared name+in duplicates (last-insert wins) - success sweep accepts `2XX` after concrete 2XX keys and before `default` (SSE detection + output schema; concrete outranks wildcard) - `4XX`/`5XX` error keys project to their class representative status (`HTTP_400`/`HTTP_500`) instead of dropping silently; `default` still drops loudly (no implied range) - top-level `webhooks` fails import naming the feature (inbound callbacks are outside the single-endpoint outbound adapter model) - unbound-placeholder error names the parameter-merge state so the diagnosis no longer dead-ends Verification: cargo test (174 lib tests), cargo fmt
This commit is contained in:
+325
-52
@@ -30,7 +30,7 @@ use super::forward::{
|
||||
forward, forward_stream, HttpServiceConfig, GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER,
|
||||
HEADER_PARAM_MARKER_VALUE,
|
||||
};
|
||||
use super::openapi_spec::{OpenAPISpec, Operation};
|
||||
use super::openapi_spec::{OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS};
|
||||
use crate::client::SharedHttpClient;
|
||||
|
||||
fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec<String> {
|
||||
@@ -130,14 +130,15 @@ impl FromOpenAPI {
|
||||
|
||||
fn detect_op_type(method: &str, op: &Operation) -> OperationType {
|
||||
// OAI-06: the success envelope may be declared under any 2XX key
|
||||
// (204, 206, 226...) or under `default` — a stream declared there
|
||||
// must still classify as Sub, not fall through to a giant
|
||||
// single-string text body.
|
||||
let success = [
|
||||
"200", "201", "202", "203", "204", "205", "206", "226", "default",
|
||||
]
|
||||
.iter()
|
||||
.find_map(|k| op.responses.get(*k));
|
||||
// (204, 206, 226...), under the class wildcard `2XX` (review 002
|
||||
// OAI-13), or under `default` — a stream declared there must
|
||||
// still classify as Sub, not fall through to a giant single-
|
||||
// string text body. [`SUCCESS_RESPONSE_KEYS`] is in precedence
|
||||
// order: concrete statuses outrank the wildcard, which outranks
|
||||
// `default`.
|
||||
let success = SUCCESS_RESPONSE_KEYS
|
||||
.iter()
|
||||
.find_map(|k| op.responses.get(*k));
|
||||
if let Some(resp) = success {
|
||||
if resp.content.contains_key("text/event-stream") {
|
||||
return OperationType::Sub;
|
||||
@@ -150,11 +151,22 @@ impl FromOpenAPI {
|
||||
}
|
||||
}
|
||||
|
||||
fn build_input_schema(&self, op: &Operation) -> Result<Value, AdapterError> {
|
||||
fn build_input_schema(
|
||||
&self,
|
||||
op: &Operation,
|
||||
path_parameters: &[Parameter],
|
||||
) -> Result<Value, AdapterError> {
|
||||
let mut properties = serde_json::Map::new();
|
||||
let mut required = Vec::new();
|
||||
|
||||
for param in &op.parameters {
|
||||
// OAI-13: path-item-level parameters merge into every operation
|
||||
// under the path. The operation-level entries come second, so a
|
||||
// same-`name`-same-`in` override wins by last-insert-wins in the
|
||||
// loop below (OpenAPI override semantics), and the overridden
|
||||
// path-item entry's `required` cannot leak: the operation entry
|
||||
// is the authoritative declaration pushed last.
|
||||
let merged: Vec<&Parameter> = path_parameters.iter().chain(op.parameters.iter()).collect();
|
||||
for param in merged {
|
||||
let schema = match ¶m.schema {
|
||||
Some(s) => self.spec.resolve_refs_recursive(s)?,
|
||||
None => serde_json::json!({"type": "string"}),
|
||||
@@ -163,9 +175,9 @@ impl FromOpenAPI {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: format!(
|
||||
"parameter named `{GATEWAY_BODY_KEY}` collides with the gateway's \
|
||||
requestBody placeholder key; the declared parameter would be \
|
||||
diverted into the request body at call time — rename the \
|
||||
parameter (review 001 OAI-07)"
|
||||
requestBody placeholder key; the declared parameter (path-item \
|
||||
level or operation-level) would be diverted into the request \
|
||||
body at call time — rename the parameter (review 001 OAI-07)"
|
||||
),
|
||||
});
|
||||
}
|
||||
@@ -222,14 +234,12 @@ impl FromOpenAPI {
|
||||
}
|
||||
|
||||
fn build_output_schema(&self, op: &Operation) -> Result<Value, AdapterError> {
|
||||
// Mirrors `detect_op_type`'s success-key sweep (OAI-06): a stream
|
||||
// declared under a non-200/201 2XX key or `default` still governs
|
||||
// the output schema shape.
|
||||
let success = [
|
||||
"200", "201", "202", "203", "204", "205", "206", "226", "default",
|
||||
]
|
||||
.iter()
|
||||
.find_map(|k| op.responses.get(*k));
|
||||
// Mirrors `detect_op_type`'s success-key sweep (OAI-06, OAI-13):
|
||||
// a stream declared under a non-200/201 2XX key, the `2XX`
|
||||
// wildcard, or `default` still governs the output schema shape.
|
||||
let success = SUCCESS_RESPONSE_KEYS
|
||||
.iter()
|
||||
.find_map(|k| op.responses.get(*k));
|
||||
let Some(resp) = success else {
|
||||
return Ok(serde_json::json!({}));
|
||||
};
|
||||
@@ -250,35 +260,46 @@ impl FromOpenAPI {
|
||||
if is_2xx {
|
||||
continue;
|
||||
}
|
||||
// OAI-06: `default`/wildcard response keys have no concrete
|
||||
// status. An `ErrorDefinition { code: "HTTP_0" }` would be
|
||||
// advertised by `/search` yet never match a real status, and
|
||||
// the runtime mapper synthesizes `HTTP_<actual>` for unmapped
|
||||
// statuses anyway — so the entry is dropped, loudly, rather
|
||||
// than advertised dead. Callers needing a catch-all declare
|
||||
// explicit statuses.
|
||||
if status.is_none() {
|
||||
tracing::warn!(
|
||||
operation = %op.operation_id.as_deref().unwrap_or("?"),
|
||||
namespace = %self.config.namespace,
|
||||
response_key = %code,
|
||||
"response key is not a concrete HTTP status; dropping it from the \
|
||||
imported error schemas — unmapped upstream statuses surface as \
|
||||
HTTP_<status> at call time (review 001 OAI-06)"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
// OAI-13: class wildcards map to the first legal concrete
|
||||
// status in their implied range — `4XX` → HTTP_400, `5XX` →
|
||||
// HTTP_500 (ADR-023 codes must be a concrete `HTTP_<status>`;
|
||||
// the runtime mapper synthesizes the actual status for any
|
||||
// unmapped one, so the projection is a faithful
|
||||
// representative, not a catch-all lie). `default` has no
|
||||
// implied range at all: an `HTTP_0` entry would be advertised
|
||||
// by `/search` yet never match a real status, so it is
|
||||
// dropped, loudly (review 001 OAI-06). A line recording this
|
||||
// mapping lives in ADR-066.
|
||||
let wildcard_status: Option<u16> = match code.as_str() {
|
||||
"4XX" => Some(400),
|
||||
"5XX" => Some(500),
|
||||
_ => None,
|
||||
};
|
||||
let (status_code, effective_status) = match (status, wildcard_status) {
|
||||
(Some(s), _) => (s, Some(s)),
|
||||
(None, Some(s)) => (s, Some(s)),
|
||||
(None, None) => {
|
||||
tracing::warn!(
|
||||
operation = %op.operation_id.as_deref().unwrap_or("?"),
|
||||
namespace = %self.config.namespace,
|
||||
response_key = %code,
|
||||
"response key is not a concrete HTTP status; dropping it from the \
|
||||
imported error schemas — unmapped upstream statuses surface as \
|
||||
HTTP_<status> at call time (review 001 OAI-06)"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let schema = if let Some(json_schema) = resp.content.get("application/json") {
|
||||
self.spec.resolve_refs_recursive(json_schema)?
|
||||
} else {
|
||||
serde_json::json!({})
|
||||
};
|
||||
let status_code = status.unwrap_or(0);
|
||||
out.push(ErrorDefinition {
|
||||
code: format!("HTTP_{status_code}"),
|
||||
description: format!("HTTP {status_code} response"),
|
||||
description: format!("HTTP {status_code} response ({code} declared)"),
|
||||
schema,
|
||||
http_status: status,
|
||||
http_status: effective_status,
|
||||
});
|
||||
}
|
||||
Ok(out)
|
||||
@@ -289,11 +310,12 @@ impl FromOpenAPI {
|
||||
method: &str,
|
||||
path: &str,
|
||||
op: &Operation,
|
||||
path_parameters: &[Parameter],
|
||||
) -> Result<HandlerRegistration, AdapterError> {
|
||||
let name = Self::normalize_operation_id(op, method, path);
|
||||
let qualified_name = format!("{}/{name}", self.config.namespace);
|
||||
let op_type = Self::detect_op_type(method, op);
|
||||
let input_schema = self.build_input_schema(op)?;
|
||||
let input_schema = self.build_input_schema(op, path_parameters)?;
|
||||
let output_schema = self.build_output_schema(op)?;
|
||||
let error_schemas = self.build_error_schemas(op)?;
|
||||
|
||||
@@ -321,13 +343,25 @@ impl FromOpenAPI {
|
||||
.collect();
|
||||
let unbound = unbound_placeholders(&path_template, &input_schema);
|
||||
if !unbound.is_empty() {
|
||||
// OAI-13 name-the-cause: after the path-item merge the only
|
||||
// remaining sources of an unbound placeholder are refs the
|
||||
// parser could not resolve or a parameter missing `name`/`in`
|
||||
// — both fail at parse time — so a live placeholder almost
|
||||
// always means the spec relies on a feature the adapter does
|
||||
// not model (e.g. path-item `parameters` under an older
|
||||
// build, or a `parameters` $ref the parse path skipped).
|
||||
// Name the placeholder and the parameter path so the
|
||||
// diagnosis is not a dead end.
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: format!(
|
||||
"path {method} {path_template} declares placeholder(s) {} with no \
|
||||
matching parameter in the operation's resolved input schema (an \
|
||||
unresolved parameter$requestBodies $ref, or a parameter missing \
|
||||
`name`/`in`); the placeholder would otherwise render as a literal \
|
||||
`{}` path segment",
|
||||
matching parameter in the operation's resolved input schema. Every \
|
||||
`parameters` source was merged (path-item level and operation level, \
|
||||
review 002 OAI-13); a placeholder left unbound after the merge means \
|
||||
a declared parameter was dropped — check that each entry under \
|
||||
`paths.{path_template}.parameters` (and the path-item's shared \
|
||||
list) has `name` and `in`, and that its $ref, if any, resolves. \
|
||||
The placeholder would otherwise render as a literal `{}` path segment",
|
||||
unbound.join(", "),
|
||||
unbound[0]
|
||||
),
|
||||
@@ -413,7 +447,7 @@ impl OperationAdapter for FromOpenAPI {
|
||||
let mut routes = Vec::new();
|
||||
for (path, item) in &self.spec.paths {
|
||||
for (method, op) in &item.operations {
|
||||
let registration = self.build_registration(method, path, op)?;
|
||||
let registration = self.build_registration(method, path, op, &item.parameters)?;
|
||||
let name = registration.spec.name.clone();
|
||||
let qualified_op_id = name
|
||||
.rsplit_once('/')
|
||||
@@ -836,6 +870,57 @@ mod tests {
|
||||
assert_eq!(errors[0].http_status, Some(404));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn wildcard_error_keys_project_to_class_representative_status() {
|
||||
let doc = r#"{
|
||||
"openapi":"3.0.0","info":{"title":"T","version":"1"},
|
||||
"paths":{"/x":{"get":{"operationId":"x","responses":{
|
||||
"200":{"content":{"application/json":{"schema":{}}}},
|
||||
"404":{"content":{"application/json":{"schema":{}}}},
|
||||
"4XX":{"content":{"application/json":{"schema":{"type":"object","properties":{"err":{"type":"string"}}}}}},
|
||||
"5XX":{"content":{"application/json":{"schema":{"type":"object"}}}},
|
||||
"default":{"content":{"application/json":{"schema":{}}}}
|
||||
}}}}
|
||||
}"#;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("ns", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
let errors = &bundles[0].spec.error_schemas;
|
||||
let codes: Vec<&str> = errors.iter().map(|e| e.code.as_str()).collect();
|
||||
assert!(
|
||||
codes.contains(&"HTTP_400"),
|
||||
"`4XX` projects to the class representative HTTP_400 (OAI-13): {codes:?}"
|
||||
);
|
||||
assert!(
|
||||
codes.contains(&"HTTP_500"),
|
||||
"`5XX` projects to the class representative HTTP_500 (OAI-13): {codes:?}"
|
||||
);
|
||||
assert!(
|
||||
codes.contains(&"HTTP_404"),
|
||||
"the concrete 404 is retained alongside the wildcard: {codes:?}"
|
||||
);
|
||||
assert!(
|
||||
errors.iter().all(|e| e.code != "HTTP_0"),
|
||||
"/search must never advertise an HTTP_0 code that cannot match (OAI-06): {errors:?}"
|
||||
);
|
||||
assert!(errors.iter().all(|e| e.http_status.is_some()));
|
||||
let wildcard_4xx = errors
|
||||
.iter()
|
||||
.find(|e| e.code == "HTTP_400")
|
||||
.expect("4XX representative present");
|
||||
assert_eq!(wildcard_4xx.http_status, Some(400));
|
||||
assert!(
|
||||
wildcard_4xx
|
||||
.schema
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.is_some_and(|p| p.contains_key("err")),
|
||||
"the wildcard's declared payload schema is carried by the representative"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_response_key_is_dropped_not_advertised_as_http_0() {
|
||||
let doc = r#"{
|
||||
@@ -843,8 +928,7 @@ mod tests {
|
||||
"paths":{"/x":{"get":{"operationId":"x","responses":{
|
||||
"200":{"content":{"application/json":{"schema":{}}}},
|
||||
"404":{"content":{"application/json":{"schema":{}}}},
|
||||
"default":{"content":{"application/json":{"schema":{}}}},
|
||||
"5XX":{"content":{"application/json":{"schema":{}}}}
|
||||
"default":{"content":{"application/json":{"schema":{}}}}
|
||||
}}}}
|
||||
}"#;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
@@ -856,7 +940,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
errors.len(),
|
||||
1,
|
||||
"only the concrete 404 may be advertised; `default` and `5XX` are dropped"
|
||||
"only the concrete 404 may be advertised; `default` has no implied status range"
|
||||
);
|
||||
assert_eq!(errors[0].code, "HTTP_404");
|
||||
assert!(
|
||||
@@ -898,6 +982,195 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn wildcard_2xx_sse_stream_classifies_as_subscription() {
|
||||
let doc = r##"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/s":{"post":{"operationId":"s","responses":{"2XX":{"content":{"text/event-stream":{"schema":{}}}}}}}}}"##;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("ns", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
bundles[0].spec.op_type,
|
||||
OperationType::Sub,
|
||||
"a `2XX`-declared text/event-stream must classify as Sub, not fall through \
|
||||
to a giant single text body (OAI-13)"
|
||||
);
|
||||
assert!(
|
||||
matches!(bundles[0].handler, HandlerKind::Stream(_)),
|
||||
"the Sub classification registers a streaming handler"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn concrete_success_key_outranks_2xx_wildcard() {
|
||||
let doc = r##"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/s":{"post":{"operationId":"s","responses":{
|
||||
"2XX":{"content":{"text/event-stream":{"schema":{}}}},
|
||||
"200":{"content":{"application/json":{"schema":{}}}}
|
||||
}}}}}"##;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("ns", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
bundles[0].spec.op_type,
|
||||
OperationType::Mutation,
|
||||
"the concrete 200 JSON response governs; the 2XX SSE entry is not consulted"
|
||||
);
|
||||
let output = &bundles[0].spec.output_schema;
|
||||
assert_ne!(
|
||||
output.get("type").and_then(|t| t.as_str()),
|
||||
Some("string"),
|
||||
"output schema follows the concrete 200 JSON response, not the SSE wildcard"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn wildcard_2xx_json_output_schema_resolves() {
|
||||
let doc = r##"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/j":{"get":{"operationId":"j","responses":{
|
||||
"2XX":{"content":{"application/json":{"schema":{"type":"object","properties":{"v":{"type":"integer"}}}}}}
|
||||
}}}}}"##;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("ns", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
let output = &bundles[0].spec.output_schema;
|
||||
assert_eq!(
|
||||
output["properties"]["v"]["type"],
|
||||
"string".replace("string", "integer"),
|
||||
"the 2XX-declared JSON schema governs the output schema (OAI-13)"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shared_path_item_parameters_import_and_operation_overrides_win() {
|
||||
let doc = r##"{
|
||||
"openapi":"3.0.0","info":{"title":"T","version":"1"},
|
||||
"paths":{
|
||||
"/users/{id}/posts": {
|
||||
"parameters":[
|
||||
{"name":"id","in":"path","required":true,"schema":{"type":"string","pattern":"^u-"}},
|
||||
{"name":"verbose","in":"query","schema":{"type":"boolean"}}
|
||||
],
|
||||
"get":{
|
||||
"operationId":"listPosts",
|
||||
"parameters":[
|
||||
{"name":"verbose","in":"query","required":true,"schema":{"type":"string","maxLength":2}}
|
||||
],
|
||||
"responses":{"200":{"content":{"application/json":{"schema":{}}}}}
|
||||
},
|
||||
"delete":{
|
||||
"operationId":"deletePost",
|
||||
"responses":{"204":{"content":{"application/json":{"schema":{}}}}}
|
||||
}
|
||||
}
|
||||
}
|
||||
}"##;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("svc", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
bundles.len(),
|
||||
2,
|
||||
"both operations import, not a placeholder failure"
|
||||
);
|
||||
|
||||
let get = bundles
|
||||
.iter()
|
||||
.find(|b| b.spec.name == "svc/listPosts")
|
||||
.expect("get op registered");
|
||||
let props = get
|
||||
.spec
|
||||
.input_schema
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.expect("input schema has properties");
|
||||
assert!(
|
||||
props.contains_key("id"),
|
||||
"the shared id placeholder is bound by the path-item parameter"
|
||||
);
|
||||
let verbose = props
|
||||
.get("verbose")
|
||||
.expect("verbose declared at both levels");
|
||||
assert_eq!(
|
||||
verbose["maxLength"], 2,
|
||||
"the operation-level entry override wins on shared name+in"
|
||||
);
|
||||
let get_required = get
|
||||
.spec
|
||||
.input_schema
|
||||
.get("required")
|
||||
.and_then(|r| r.as_array())
|
||||
.expect("required list present");
|
||||
assert!(
|
||||
get_required.iter().any(|v| v == "id"),
|
||||
"path-item required id flows into the merged requirements"
|
||||
);
|
||||
assert!(
|
||||
get_required.iter().any(|v| v == "verbose"),
|
||||
"the override's own required:true is honored"
|
||||
);
|
||||
|
||||
let delete = bundles
|
||||
.iter()
|
||||
.find(|b| b.spec.name == "svc/deletePost")
|
||||
.expect("delete op registered");
|
||||
let delete_props = delete
|
||||
.spec
|
||||
.input_schema
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.expect("input schema has properties");
|
||||
assert!(delete_props.contains_key("id"));
|
||||
let verbose_schema = delete_props
|
||||
.get("verbose")
|
||||
.expect("inherited from path item");
|
||||
assert_eq!(
|
||||
verbose_schema["type"], "boolean",
|
||||
"no operation-level entry means the path-item declaration applies verbatim"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shared_path_item_parameters_defeat_the_misleading_placeholder_failure() {
|
||||
// The OAI-13 headline case: a Petstore-with-shared-params shape
|
||||
// previously failed the whole import with "declares placeholder(s)
|
||||
// id with no matching parameter", pointing nowhere near the cause.
|
||||
let doc = r##"{
|
||||
"openapi":"3.0.0","info":{"title":"T","version":"1"},
|
||||
"components":{"parameters":{
|
||||
"Id":{"name":"id","in":"path","required":true,"schema":{"type":"integer"}}
|
||||
}},
|
||||
"paths":{
|
||||
"/pets/{id}": {
|
||||
"parameters":[{"$ref":"#/components/parameters/Id"}],
|
||||
"get":{
|
||||
"operationId":"showPetById",
|
||||
"responses":{"200":{"content":{"application/json":{"schema":{}}}}}
|
||||
}
|
||||
}
|
||||
}
|
||||
}"##;
|
||||
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||
let bundles = adapter(spec, config("svc", "https://x", None))
|
||||
.import()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(bundles.len(), 1);
|
||||
let props = bundles[0]
|
||||
.spec
|
||||
.input_schema
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.expect("input schema has properties");
|
||||
assert!(props.contains_key("id"));
|
||||
assert_eq!(props["id"]["type"], "integer");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn input_schema_from_params_and_body() {
|
||||
let doc = r#"{
|
||||
|
||||
Reference in New Issue
Block a user