fix(api): close OpenAPI schema fidelity gaps
This commit is contained in:
@@ -26,6 +26,12 @@ const MAX_SAFE_INTEGER: i64 = 9_007_199_254_740_991;
|
||||
/// The default component name delegates to [`JsonSchema::schema_name`]; types
|
||||
/// may override it to provide a stable public wire name.
|
||||
pub trait OpenApiSchema: JsonSchema {
|
||||
/// Whether a parameter of this type must be present on the wire.
|
||||
///
|
||||
/// This semantic hook intentionally lives on the resolved type, so aliases
|
||||
/// such as `type Header = Option<String>` retain optionality.
|
||||
const PARAMETER_REQUIRED: bool = true;
|
||||
|
||||
fn openapi_schema_name() -> Cow<'static, str> {
|
||||
Self::schema_name()
|
||||
}
|
||||
@@ -39,7 +45,9 @@ macro_rules! impl_openapi_schema {
|
||||
|
||||
impl_openapi_schema!((), bool, String, char, i8, i16, i32, u8, u16, u32, f32, f64);
|
||||
|
||||
impl<T: OpenApiSchema> OpenApiSchema for Option<T> {}
|
||||
impl<T: OpenApiSchema> OpenApiSchema for Option<T> {
|
||||
const PARAMETER_REQUIRED: bool = false;
|
||||
}
|
||||
impl<T: OpenApiSchema> OpenApiSchema for Vec<T> {}
|
||||
impl<T: OpenApiSchema> OpenApiSchema for Box<T> {}
|
||||
impl<T: OpenApiSchema> OpenApiSchema for std::sync::Arc<T> {}
|
||||
@@ -184,7 +192,7 @@ impl OpenApiBuilder {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn finish(self) -> OpenApiDocument {
|
||||
pub fn finish(self) -> Result<OpenApiDocument, OpenApiError> {
|
||||
let paths = self
|
||||
.paths
|
||||
.into_iter()
|
||||
@@ -195,12 +203,14 @@ impl OpenApiBuilder {
|
||||
.collect::<Map<_, _>>();
|
||||
let schemas = self.schemas.into_iter().collect::<Map<_, _>>();
|
||||
|
||||
OpenApiDocument(canonicalize(json!({
|
||||
let document = canonicalize(json!({
|
||||
"openapi": "3.1.0",
|
||||
"info": self.info,
|
||||
"paths": paths,
|
||||
"components": { "schemas": schemas },
|
||||
})))
|
||||
}));
|
||||
validate_component_references(&document)?;
|
||||
Ok(OpenApiDocument(document))
|
||||
}
|
||||
|
||||
fn schema_ref<T: OpenApiSchema>(&mut self) -> Result<Value, OpenApiError> {
|
||||
@@ -268,7 +278,6 @@ impl OpenApiOperation<'_> {
|
||||
&mut self,
|
||||
name: &'static str,
|
||||
location: &'static str,
|
||||
required: bool,
|
||||
) -> Result<(), OpenApiError> {
|
||||
if !matches!(location, "path" | "query" | "header") {
|
||||
return Err(OpenApiError::InvalidContract(format!(
|
||||
@@ -276,12 +285,7 @@ impl OpenApiOperation<'_> {
|
||||
self.operation_id
|
||||
)));
|
||||
}
|
||||
if location == "path" && !required {
|
||||
return Err(OpenApiError::InvalidContract(format!(
|
||||
"path parameter `{name}` for `{}` must be required",
|
||||
self.operation_id
|
||||
)));
|
||||
}
|
||||
let required = location == "path" || T::PARAMETER_REQUIRED;
|
||||
let schema = self.parent.schema_ref::<T>()?;
|
||||
self.parameters.push(json!({
|
||||
"name": name,
|
||||
@@ -396,6 +400,46 @@ fn validate_component_name(name: &str) -> Result<(), OpenApiError> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_component_references(document: &Value) -> Result<(), OpenApiError> {
|
||||
let schemas = document["components"]["schemas"]
|
||||
.as_object()
|
||||
.ok_or_else(|| OpenApiError::InvalidContract("missing component schema map".to_owned()))?;
|
||||
let mut references = Vec::new();
|
||||
collect_references(document, &mut references);
|
||||
for reference in references {
|
||||
let Some(name) = reference.strip_prefix("#/components/schemas/") else {
|
||||
return Err(OpenApiError::InvalidContract(format!(
|
||||
"unsupported non-component schema reference `{reference}`"
|
||||
)));
|
||||
};
|
||||
if !schemas.contains_key(name) {
|
||||
return Err(OpenApiError::InvalidContract(format!(
|
||||
"unresolved OpenAPI component reference `{reference}`"
|
||||
)));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn collect_references<'a>(value: &'a Value, references: &mut Vec<&'a str>) {
|
||||
match value {
|
||||
Value::Object(object) => {
|
||||
if let Some(reference) = object.get("$ref").and_then(Value::as_str) {
|
||||
references.push(reference);
|
||||
}
|
||||
for value in object.values() {
|
||||
collect_references(value, references);
|
||||
}
|
||||
}
|
||||
Value::Array(values) => {
|
||||
for value in values {
|
||||
collect_references(value, references);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn rewrite_definition_refs(value: &mut Value) {
|
||||
match value {
|
||||
Value::Object(object) => {
|
||||
@@ -421,11 +465,11 @@ fn validate_schema(value: &Value, component: &str) -> Result<(), OpenApiError> {
|
||||
object.get("format").and_then(Value::as_str),
|
||||
Some("int8" | "uint8" | "int16" | "uint16" | "int32" | "uint32")
|
||||
);
|
||||
let minimum = object.get("minimum").and_then(Value::as_i64);
|
||||
let maximum = object.get("maximum").and_then(Value::as_u64);
|
||||
let minimum = object.get("minimum").and_then(Value::as_f64);
|
||||
let maximum = object.get("maximum").and_then(Value::as_f64);
|
||||
if !safe_format
|
||||
&& (minimum.is_none_or(|value| value < -MAX_SAFE_INTEGER)
|
||||
|| maximum.is_none_or(|value| value > MAX_SAFE_INTEGER as u64))
|
||||
&& (minimum.is_none_or(|value| value < -(MAX_SAFE_INTEGER as f64))
|
||||
|| maximum.is_none_or(|value| value > MAX_SAFE_INTEGER as f64))
|
||||
{
|
||||
return Err(OpenApiError::InvalidContract(format!(
|
||||
"integer schema in `{component}` exceeds the JSON safe-integer range; expose a bounded wire integer"
|
||||
|
||||
@@ -6,7 +6,7 @@ use api_macros::{
|
||||
openapi::{OpenApiInfo, OpenApiSchema},
|
||||
};
|
||||
use schemars::JsonSchema;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde::{Deserialize, Deserializer, Serialize, Serializer};
|
||||
use serde_json::{Value, json};
|
||||
|
||||
fn nullable_string_schema(generator: &mut schemars::SchemaGenerator) -> schemars::Schema {
|
||||
@@ -20,10 +20,36 @@ pub struct GeoPoint {
|
||||
}
|
||||
impl OpenApiSchema for GeoPoint {}
|
||||
|
||||
/// A custom Serde wire representation admitted only after an explicit,
|
||||
/// matching schema hook and `OpenApiSchema` attestation.
|
||||
#[derive(Debug, JsonSchema)]
|
||||
#[schemars(transparent)]
|
||||
pub struct WireSlug(String);
|
||||
|
||||
impl Serialize for WireSlug {
|
||||
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
|
||||
serializer.serialize_str(&self.0)
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for WireSlug {
|
||||
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
|
||||
String::deserialize(deserializer).map(Self)
|
||||
}
|
||||
}
|
||||
impl OpenApiSchema for WireSlug {}
|
||||
|
||||
#[derive(Debug, Deserialize, JsonSchema, Serialize)]
|
||||
pub struct CreateWidget {
|
||||
#[schemars(length(min = 1, max = 64))]
|
||||
name: String,
|
||||
#[schemars(range(min = 1, max = 1_000))]
|
||||
count: u32,
|
||||
#[schemars(range(min = -100, max = 100))]
|
||||
bounded_balance: i64,
|
||||
#[schemars(length(min = 1, max = 8))]
|
||||
tags: Vec<String>,
|
||||
slug: WireSlug,
|
||||
/// Missing and explicit null are separate wire states.
|
||||
#[schemars(default, schema_with = "nullable_string_schema")]
|
||||
optional_label: Option<String>,
|
||||
@@ -47,11 +73,14 @@ pub struct Widget {
|
||||
impl OpenApiSchema for Widget {}
|
||||
|
||||
#[derive(Debug, Deserialize, JsonSchema, Serialize)]
|
||||
#[serde(tag = "code", content = "details")]
|
||||
pub enum PublicError {
|
||||
Invalid { message: String },
|
||||
}
|
||||
impl OpenApiSchema for PublicError {}
|
||||
|
||||
type OptionalRequestId = Option<String>;
|
||||
|
||||
#[api(openapi)]
|
||||
pub trait FixtureApi {
|
||||
#[post(
|
||||
@@ -64,7 +93,7 @@ pub trait FixtureApi {
|
||||
&self,
|
||||
#[path] widget_id: u32,
|
||||
#[query] lookup: Lookup,
|
||||
#[header("x-request-id")] request_id: Option<String>,
|
||||
#[header("x-request-id")] request_id: OptionalRequestId,
|
||||
#[body] request: CreateWidget,
|
||||
) -> Result<Widget, PublicError>;
|
||||
|
||||
@@ -105,6 +134,41 @@ fn output_is_deterministic_and_accepted_by_an_independent_parser() {
|
||||
);
|
||||
assert!(!first.contains("generatedAt"));
|
||||
assert!(!first.contains("#/$defs/"));
|
||||
|
||||
let schemas = value["components"]["schemas"]
|
||||
.as_object()
|
||||
.expect("component map");
|
||||
let mut references = Vec::new();
|
||||
collect_schema_references(&value, &mut references);
|
||||
assert!(!references.is_empty());
|
||||
for reference in references {
|
||||
let name = reference
|
||||
.strip_prefix("#/components/schemas/")
|
||||
.expect("only local component references are emitted");
|
||||
assert!(
|
||||
schemas.contains_key(name),
|
||||
"unresolved reference {reference}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn collect_schema_references<'a>(value: &'a Value, references: &mut Vec<&'a str>) {
|
||||
match value {
|
||||
Value::Object(object) => {
|
||||
if let Some(reference) = object.get("$ref").and_then(Value::as_str) {
|
||||
references.push(reference);
|
||||
}
|
||||
for value in object.values() {
|
||||
collect_schema_references(value, references);
|
||||
}
|
||||
}
|
||||
Value::Array(values) => {
|
||||
for value in values {
|
||||
collect_schema_references(value, references);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -156,6 +220,7 @@ fn operations_and_components_preserve_the_wire_contract() {
|
||||
"Lookup",
|
||||
"PublicError",
|
||||
"Widget",
|
||||
"WireSlug",
|
||||
"uint32",
|
||||
"Nullable_string",
|
||||
] {
|
||||
@@ -169,6 +234,33 @@ fn operations_and_components_preserve_the_wire_contract() {
|
||||
schemas["CreateWidget"]["properties"]["location"]["$ref"],
|
||||
"#/components/schemas/GeoPoint"
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["slug"]["$ref"],
|
||||
"#/components/schemas/WireSlug"
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["name"]["minLength"],
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["name"]["maxLength"],
|
||||
64
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["count"]["maximum"],
|
||||
1_000
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["bounded_balance"]["minimum"],
|
||||
-100
|
||||
);
|
||||
assert_eq!(
|
||||
schemas["CreateWidget"]["properties"]["bounded_balance"]["maximum"],
|
||||
100
|
||||
);
|
||||
assert_eq!(schemas["CreateWidget"]["properties"]["tags"]["minItems"], 1);
|
||||
assert_eq!(schemas["CreateWidget"]["properties"]["tags"]["maxItems"], 8);
|
||||
assert!(schemas["PublicError"]["oneOf"].is_array());
|
||||
|
||||
let required = schemas["CreateWidget"]["required"]
|
||||
.as_array()
|
||||
@@ -220,6 +312,20 @@ pub enum AmbiguousWireEnum {
|
||||
}
|
||||
impl OpenApiSchema for AmbiguousWireEnum {}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct DanglingReference;
|
||||
|
||||
impl JsonSchema for DanglingReference {
|
||||
fn schema_name() -> std::borrow::Cow<'static, str> {
|
||||
"DanglingReference".into()
|
||||
}
|
||||
|
||||
fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema {
|
||||
schemars::json_schema!({ "$ref": "#/$defs/Missing" })
|
||||
}
|
||||
}
|
||||
impl OpenApiSchema for DanglingReference {}
|
||||
|
||||
#[test]
|
||||
fn schema_name_collisions_and_ambiguous_wire_shapes_fail_closed() {
|
||||
use api_macros::openapi::{OpenApiBuilder, OpenApiError};
|
||||
@@ -254,6 +360,21 @@ fn schema_name_collisions_and_ambiguous_wire_shapes_fail_closed() {
|
||||
.response::<AmbiguousWireEnum>(200, "application/json", "ok")
|
||||
.expect_err("untagged/ambiguous unions must fail closed");
|
||||
assert!(error.to_string().contains("untagged"));
|
||||
|
||||
let mut dangling = OpenApiBuilder::new(info).expect("builder");
|
||||
{
|
||||
let mut operation = dangling
|
||||
.operation("GET", "/dangling", "dangling")
|
||||
.expect("operation");
|
||||
operation
|
||||
.response::<DanglingReference>(200, "application/json", "ok")
|
||||
.expect("schema registration succeeds before graph validation");
|
||||
operation.finish().expect("operation");
|
||||
}
|
||||
let error = dangling
|
||||
.finish()
|
||||
.expect_err("dangling component references must fail closed");
|
||||
assert!(error.to_string().contains("unresolved"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user