fix(adapters): hoist path-template validation into shared forward core, run at from_openapi import (JS-02)
Import-time path-template checks now run on both adapters:
- from_jsonschema: validate_path_template moves to forward.rs (same
checks, same messages, shared implementation)
- from_openapi: build_registration calls it before building each op, so
a template like /x{open fails import with 'unterminated placeholder'
instead of a per-call INTERNAL on first invoke
- new test: unterminated_path_template_fails_import_not_first_call
This commit is contained in:
@@ -122,6 +122,7 @@
|
|||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
use alkcall::client::AdapterError;
|
||||||
use alkcall::protocol::wire::{CallError, ResponseEnvelope};
|
use alkcall::protocol::wire::{CallError, ResponseEnvelope};
|
||||||
use alkcall::registry::context::OperationContext;
|
use alkcall::registry::context::OperationContext;
|
||||||
use alkcall::registry::registration::ResponseStream;
|
use alkcall::registry::registration::ResponseStream;
|
||||||
@@ -176,6 +177,35 @@ pub(crate) const GATEWAY_BODY_KEY: &str = "body";
|
|||||||
pub(crate) const HEADER_PARAM_IN_MARKER: &str = "wire";
|
pub(crate) const HEADER_PARAM_IN_MARKER: &str = "wire";
|
||||||
pub(crate) const HEADER_PARAM_MARKER_VALUE: &str = "header";
|
pub(crate) const HEADER_PARAM_MARKER_VALUE: &str = "header";
|
||||||
|
|
||||||
|
/// Import-time path-template validation shared by `from_jsonschema`
|
||||||
|
/// (review 001 OAI-09) and `from_openapi` (review 002 JS-02): every
|
||||||
|
/// `{placeholder}` must terminate with a `}` and carry a name. A
|
||||||
|
/// template that slips through (e.g. `/x{open`) otherwise surfaces as a
|
||||||
|
/// per-call `INTERNAL` error on first invoke — the eager-validation
|
||||||
|
/// promise both adapters make.
|
||||||
|
pub(crate) fn validate_path_template(path_template: &str) -> Result<(), AdapterError> {
|
||||||
|
let mut rest = path_template;
|
||||||
|
while let Some(start) = rest.find('{') {
|
||||||
|
let Some(end_rel) = rest[start..].find('}') else {
|
||||||
|
return Err(AdapterError::SchemaParse {
|
||||||
|
message: format!("path template `{path_template}` has an unterminated placeholder"),
|
||||||
|
});
|
||||||
|
};
|
||||||
|
if rest[start + 1..start + end_rel].is_empty() {
|
||||||
|
return Err(AdapterError::SchemaParse {
|
||||||
|
message: format!("path template `{path_template}` has an empty placeholder name"),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
rest = &rest[start + end_rel + 1..];
|
||||||
|
}
|
||||||
|
if path_template.contains('}') && !path_template.contains('{') {
|
||||||
|
return Err(AdapterError::SchemaParse {
|
||||||
|
message: format!("path template `{path_template}` has `}}` without a matching `{{`"),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// The credential scheme forwarded handlers apply to outbound requests
|
/// The credential scheme forwarded handlers apply to outbound requests
|
||||||
/// (ADR-014). The credential value itself flows through
|
/// (ADR-014). The credential value itself flows through
|
||||||
/// `OperationContext.capabilities` at call time — never through this
|
/// `OperationContext.capabilities` at call time — never through this
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ use async_trait::async_trait;
|
|||||||
use reqwest::Method;
|
use reqwest::Method;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use super::forward::{forward, forward_stream, HttpServiceConfig};
|
use super::forward::{forward, forward_stream, validate_path_template, HttpServiceConfig};
|
||||||
use crate::client::SharedHttpClient;
|
use crate::client::SharedHttpClient;
|
||||||
|
|
||||||
/// The HTTP-backed single-endpoint adapter (ADR-066): one caller-built
|
/// The HTTP-backed single-endpoint adapter (ADR-066): one caller-built
|
||||||
@@ -139,30 +139,6 @@ fn validate_method(method: &str) -> Result<(), AdapterError> {
|
|||||||
})?;
|
})?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
fn validate_path_template(path_template: &str) -> Result<(), AdapterError> {
|
|
||||||
let mut rest = path_template;
|
|
||||||
while let Some(start) = rest.find('{') {
|
|
||||||
let Some(end_rel) = rest[start..].find('}') else {
|
|
||||||
return Err(AdapterError::SchemaParse {
|
|
||||||
message: format!("path template `{path_template}` has an unterminated placeholder"),
|
|
||||||
});
|
|
||||||
};
|
|
||||||
if rest[start + 1..start + end_rel].is_empty() {
|
|
||||||
return Err(AdapterError::SchemaParse {
|
|
||||||
message: format!("path template `{path_template}` has an empty placeholder name"),
|
|
||||||
});
|
|
||||||
}
|
|
||||||
rest = &rest[start + end_rel + 1..];
|
|
||||||
}
|
|
||||||
if path_template.contains('}') && !path_template.contains('{') {
|
|
||||||
return Err(AdapterError::SchemaParse {
|
|
||||||
message: format!("path template `{path_template}` has `}}` without a matching `{{`"),
|
|
||||||
});
|
|
||||||
}
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
|
|
||||||
fn spec_name_references_undeclared(spec: &OperationSpec, path_template: &str) -> bool {
|
fn spec_name_references_undeclared(spec: &OperationSpec, path_template: &str) -> bool {
|
||||||
let properties = spec
|
let properties = spec
|
||||||
.input_schema
|
.input_schema
|
||||||
|
|||||||
@@ -27,8 +27,8 @@ use async_trait::async_trait;
|
|||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use super::forward::{
|
use super::forward::{
|
||||||
forward, forward_stream, HttpServiceConfig, GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER,
|
forward, forward_stream, validate_path_template, HttpServiceConfig, GATEWAY_BODY_KEY,
|
||||||
HEADER_PARAM_MARKER_VALUE,
|
HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
||||||
};
|
};
|
||||||
use super::openapi_spec::{OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS};
|
use super::openapi_spec::{OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS};
|
||||||
use crate::client::SharedHttpClient;
|
use crate::client::SharedHttpClient;
|
||||||
@@ -310,6 +310,7 @@ impl FromOpenAPI {
|
|||||||
op: &Operation,
|
op: &Operation,
|
||||||
path_parameters: &[Parameter],
|
path_parameters: &[Parameter],
|
||||||
) -> Result<HandlerRegistration, AdapterError> {
|
) -> Result<HandlerRegistration, AdapterError> {
|
||||||
|
validate_path_template(path)?;
|
||||||
let name = Self::normalize_operation_id(op, method, path);
|
let name = Self::normalize_operation_id(op, method, path);
|
||||||
let qualified_name = format!("{}/{name}", self.config.namespace);
|
let qualified_name = format!("{}/{name}", self.config.namespace);
|
||||||
let op_type = Self::detect_op_type(method, op);
|
let op_type = Self::detect_op_type(method, op);
|
||||||
@@ -810,6 +811,32 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn unterminated_path_template_fails_import_not_first_call() {
|
||||||
|
let doc = r#"{
|
||||||
|
"openapi":"3.0.0","info":{"title":"T","version":"1"},
|
||||||
|
"paths":{"/x{open":{"get":{"operationId":"x","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}}
|
||||||
|
}"#;
|
||||||
|
let spec = OpenAPISpec::from_json(doc).unwrap();
|
||||||
|
let result = adapter(spec, config("ns", "https://x", None))
|
||||||
|
.import()
|
||||||
|
.await;
|
||||||
|
match result {
|
||||||
|
Err(AdapterError::SchemaParse { message }) => {
|
||||||
|
assert!(
|
||||||
|
message.contains("unterminated placeholder"),
|
||||||
|
"message was: {message}"
|
||||||
|
);
|
||||||
|
assert!(message.contains("/x{open"), "message was: {message}");
|
||||||
|
}
|
||||||
|
Ok(bundles) => panic!(
|
||||||
|
"unterminated path template must fail at import (JS-02), got {} bundles",
|
||||||
|
bundles.len()
|
||||||
|
),
|
||||||
|
Err(e) => panic!("expected SchemaParse, got {e}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn op_type_detection() {
|
async fn op_type_detection() {
|
||||||
let get_doc = r#"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/g":{"get":{"operationId":"g","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}}}"#;
|
let get_doc = r#"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/g":{"get":{"operationId":"g","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}}}"#;
|
||||||
|
|||||||
Reference in New Issue
Block a user