diff --git a/dropshot/src/extractor/common.rs b/dropshot/src/extractor/common.rs index 31f964506..f6635fe3d 100644 --- a/dropshot/src/extractor/common.rs +++ b/dropshot/src/extractor/common.rs @@ -15,7 +15,7 @@ pub struct ExtractorMetadata { pub parameters: Vec, } -/// Extractors that require exclusive access to the underyling `hyper::Request` +/// Extractors that require exclusive access to the underlying `hyper::Request` /// /// These extractors usually need to read the body of the request or else modify /// how the server treats the rest of it (e.g., websocket upgrade). There may diff --git a/dropshot/src/extractor/metadata.rs b/dropshot/src/extractor/metadata.rs index eae67c5c9..0e2ec857c 100644 --- a/dropshot/src/extractor/metadata.rs +++ b/dropshot/src/extractor/metadata.rs @@ -4,7 +4,7 @@ use crate::api_description::ApiSchemaGenerator; use crate::pagination::PAGINATION_PARAM_SENTINEL; use crate::schema_util::schema2struct; use crate::schema_util::schema_extensions; -use crate::schema_util::ReferenceVisitor; +use crate::schema_util::StructMember; use crate::websocket::WEBSOCKET_PARAM_SENTINEL; use crate::ApiEndpointParameter; use crate::ApiEndpointParameterLocation; @@ -61,19 +61,19 @@ where true, ) .into_iter() - .map(|struct_member| { - let mut s = struct_member.schema; - let mut visitor = ReferenceVisitor::new(&generator); - schemars::visit::visit_schema(&mut visitor, &mut s); - + .map(|StructMember { name, description, schema, required }| { ApiEndpointParameter::new_named( loc, - struct_member.name, - struct_member.description, - struct_member.required, + name, + description, + required, ApiSchemaGenerator::Static { - schema: Box::new(s), - dependencies: visitor.dependencies(), + schema: Box::new(schema), + dependencies: generator + .definitions() + .clone() + .into_iter() + .collect(), }, Vec::new(), ) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 82b88536e..de589a4ee 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -1,4 +1,4 @@ -// Copyright 2024 Oxide Computer Company +// Copyright 2025 Oxide Computer Company //! Interface for implementing HTTP endpoint handler functions. //! //! For information about supported endpoint function signatures, argument types, @@ -48,7 +48,7 @@ use crate::pagination::PaginationParams; use crate::router::VariableSet; use crate::schema_util::make_subschema_for; use crate::schema_util::schema2struct; -use crate::schema_util::ReferenceVisitor; +use crate::schema_util::StructMember; use crate::to_map::to_map; use async_trait::async_trait; @@ -1403,18 +1403,19 @@ impl< true, ) .into_iter() - .map(|struct_member| { - let mut s = struct_member.schema; - let mut visitor = ReferenceVisitor::new(&generator); - schemars::visit::visit_schema(&mut visitor, &mut s); + .map(|StructMember { name, description, schema, required }| { ApiEndpointHeader { - name: struct_member.name, - description: struct_member.description, + name, + description, schema: ApiSchemaGenerator::Static { - schema: Box::new(s), - dependencies: visitor.dependencies(), + schema: Box::new(schema), + dependencies: generator + .definitions() + .clone() + .into_iter() + .collect(), }, - required: struct_member.required, + required, } }) .collect::>(); diff --git a/dropshot/src/schema_util.rs b/dropshot/src/schema_util.rs index 71f1b4cbe..dbae46322 100644 --- a/dropshot/src/schema_util.rs +++ b/dropshot/src/schema_util.rs @@ -255,50 +255,6 @@ pub(crate) fn schema_extensions( } } -/// Used to visit all schemas and collect all dependencies. -pub(crate) struct ReferenceVisitor<'a> { - generator: &'a schemars::gen::SchemaGenerator, - dependencies: indexmap::IndexMap, -} - -impl<'a> ReferenceVisitor<'a> { - pub fn new(generator: &'a schemars::gen::SchemaGenerator) -> Self { - Self { generator, dependencies: indexmap::IndexMap::new() } - } - - pub fn dependencies( - self, - ) -> indexmap::IndexMap { - self.dependencies - } -} - -impl schemars::visit::Visitor for ReferenceVisitor<'_> { - fn visit_schema_object(&mut self, schema: &mut SchemaObject) { - if let Some(refstr) = &schema.reference { - let definitions_path = &self.generator.settings().definitions_path; - let name = &refstr[definitions_path.len()..]; - - if !self.dependencies.contains_key(name) { - let mut refschema = self - .generator - .definitions() - .get(name) - .expect("invalid reference") - .clone(); - self.dependencies.insert( - name.to_string(), - schemars::schema::Schema::Bool(false), - ); - schemars::visit::visit_schema(self, &mut refschema); - self.dependencies.insert(name.to_string(), refschema); - } - } - - schemars::visit::visit_schema_object(self, schema); - } -} - pub(crate) fn schema_extract_description( schema: &schemars::schema::Schema, ) -> (Option, schemars::schema::Schema) { @@ -379,14 +335,26 @@ pub(crate) fn j2oas_schema( // when consumers use a type such as serde_json::Value. schemars::schema::Schema::Bool(true) => { openapiv3::ReferenceOr::Item(openapiv3::Schema { - schema_data: openapiv3::SchemaData::default(), - schema_kind: openapiv3::SchemaKind::Any( - openapiv3::AnySchema::default(), - ), + schema_data: Default::default(), + schema_kind: openapiv3::SchemaKind::Any(Default::default()), }) } + // The unsatisfiable, "match nothing" schema. We represent this as + // the `not` of the permissive schema. schemars::schema::Schema::Bool(false) => { - panic!("We don't expect to see a schema that matches the null set") + openapiv3::ReferenceOr::Item(openapiv3::Schema { + schema_data: Default::default(), + schema_kind: openapiv3::SchemaKind::Not { + not: Box::new(openapiv3::ReferenceOr::Item( + openapiv3::Schema { + schema_data: Default::default(), + schema_kind: openapiv3::SchemaKind::Any( + Default::default(), + ), + }, + )), + }, + }) } schemars::schema::Schema::Object(obj) => j2oas_schema_object(name, obj), } @@ -411,6 +379,73 @@ fn j2oas_schema_object( }; } + let kind = j2oas_schema_object_kind(obj); + + let mut data = openapiv3::SchemaData::default(); + + if matches!( + &obj.extensions.get("nullable"), + Some(serde_json::Value::Bool(true)) + ) { + data.nullable = true; + } + + if let Some(metadata) = &obj.metadata { + data.title.clone_from(&metadata.title); + data.description.clone_from(&metadata.description); + data.default.clone_from(&metadata.default); + data.deprecated = metadata.deprecated; + data.read_only = metadata.read_only; + data.write_only = metadata.write_only; + } + + // Preserve extensions + data.extensions = obj + .extensions + .iter() + .filter(|(key, _)| key.starts_with("x-")) + .map(|(key, value)| (key.clone(), value.clone())) + .collect(); + + if let Some(name) = name { + data.title = Some(name.clone()); + } + if let Some(example) = obj.extensions.get("example") { + data.example = Some(example.clone()); + } + + openapiv3::ReferenceOr::Item(openapiv3::Schema { + schema_data: data, + schema_kind: kind, + }) +} + +fn j2oas_schema_object_kind( + obj: &schemars::schema::SchemaObject, +) -> openapiv3::SchemaKind { + // If the JSON schema is attempting to express an unsatisfiable schema + // using an empty enumerated values array, that presents a problem + // translating to the openapiv3 crate's representation. While we might + // like to simply use the schema `false` that is *also* challenging to + // represent via the openapiv3 crate *and* it precludes us from preserving + // extensions, if they happen to be present. Instead we'll represent this + // construction using `{ not: {} }` i.e. the opposite of the permissive + // schema. + if let Some(enum_values) = &obj.enum_values { + if enum_values.is_empty() { + return openapiv3::SchemaKind::Not { + not: Box::new(openapiv3::ReferenceOr::Item( + openapiv3::Schema { + schema_data: Default::default(), + schema_kind: openapiv3::SchemaKind::Any( + Default::default(), + ), + }, + )), + }; + } + } + let ty = match &obj.instance_type { Some(schemars::schema::SingleOrVec::Single(ty)) => Some(ty.as_ref()), Some(schemars::schema::SingleOrVec::Vec(_)) => { @@ -423,7 +458,7 @@ fn j2oas_schema_object( None => None, }; - let kind = match (ty, &obj.subschemas) { + match (ty, &obj.subschemas) { (Some(schemars::schema::InstanceType::Null), None) => { openapiv3::SchemaKind::Type(openapiv3::Type::String( openapiv3::StringType { @@ -485,45 +520,7 @@ fn j2oas_schema_object( openapiv3::SchemaKind::Any(openapiv3::AnySchema::default()) } (Some(_), Some(_)) => j2oas_any(ty, obj), - }; - - let mut data = openapiv3::SchemaData::default(); - - if matches!( - &obj.extensions.get("nullable"), - Some(serde_json::Value::Bool(true)) - ) { - data.nullable = true; - } - - if let Some(metadata) = &obj.metadata { - data.title.clone_from(&metadata.title); - data.description.clone_from(&metadata.description); - data.default.clone_from(&metadata.default); - data.deprecated = metadata.deprecated; - data.read_only = metadata.read_only; - data.write_only = metadata.write_only; } - - // Preserve extensions - data.extensions = obj - .extensions - .iter() - .filter(|(key, _)| key.starts_with("x-")) - .map(|(key, value)| (key.clone(), value.clone())) - .collect(); - - if let Some(name) = name { - data.title = Some(name.clone()); - } - if let Some(example) = obj.extensions.get("example") { - data.example = Some(example.clone()); - } - - openapiv3::ReferenceOr::Item(openapiv3::Schema { - schema_data: data, - schema_kind: kind, - }) } fn j2oas_any( @@ -842,6 +839,7 @@ fn j2oas_string( let enumeration = enum_values .iter() .flat_map(|v| { + assert!(!v.is_empty()); v.iter().map(|vv| match vv { serde_json::Value::Null => None, serde_json::Value::String(s) => Some(s.clone()), diff --git a/dropshot/tests/integration-tests/openapi.rs b/dropshot/tests/integration-tests/openapi.rs index ae8c2cf94..401b019c6 100644 --- a/dropshot/tests/integration-tests/openapi.rs +++ b/dropshot/tests/integration-tests/openapi.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Oxide Computer Company +// Copyright 2025 Oxide Computer Company use dropshot::{ channel, endpoint, http_response_found, http_response_see_other, @@ -548,6 +548,74 @@ async fn handler29( todo!() } +#[derive(Deserialize, JsonSchema)] +struct PathArgs30 { + #[expect(unused)] + aa: WithXRustType, + #[expect(unused)] + bb: WithXRustType, +} + +#[derive(Deserialize, Debug)] +struct WithXRustType { + _data: T, +} + +impl JsonSchema for WithXRustType { + fn schema_name() -> String { + format!("WithXRustTypeFor{}", T::schema_name()) + } + + fn json_schema( + gen: &mut schemars::gen::SchemaGenerator, + ) -> schemars::schema::Schema { + use schemars::schema::*; + + let mut schema = SchemaObject { + instance_type: Some(SingleOrVec::Single(Box::new( + InstanceType::String, + ))), + ..Default::default() + }; + + // Add the x-rust-type extension. + let mut extensions = schemars::Map::new(); + let rust_type = serde_json::json!({ + "crate": "foo", + "version": "*", + "path": "foo", + "parameters": [ + gen.subschema_for::(), + ], + }); + extensions.insert("x-rust-type".to_string(), rust_type); + schema.extensions = extensions; + + Schema::Object(schema) + } +} + +#[derive(Debug, Deserialize, JsonSchema)] +struct XRustAParam { + #[expect(unused)] + data: String, +} + +#[derive(Debug, Deserialize, JsonSchema)] +enum XRustBParam {} + +#[endpoint { + method = PUT, + path = "/testing/{aa}/{bb}", + tags = ["it"] +}] +async fn handler30( + _: RequestContext<()>, + _: Path, +) -> Result, HttpError> { + todo!(); +} + fn make_api( maybe_tag_config: Option, ) -> Result, ApiDescriptionRegisterError> { @@ -586,6 +654,7 @@ fn make_api( api.register(handler27)?; api.register(handler28)?; api.register(handler29)?; + api.register(handler30)?; Ok(api) } diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 1ac4e3a49..8f063da22 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -764,6 +764,50 @@ } } }, + "/testing/{aa}/{bb}": { + "put": { + "tags": [ + "it" + ], + "operationId": "handler30", + "parameters": [ + { + "in": "path", + "name": "aa", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustAParam" + } + }, + { + "in": "path", + "name": "bb", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustBParam" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CoolStruct" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/thing_with_headers": { "get": { "tags": [ @@ -1188,6 +1232,46 @@ "items" ] }, + "WithXRustTypeForXRustAParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustAParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "WithXRustTypeForXRustBParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustBParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "XRustAParam": { + "type": "object", + "properties": { + "data": { + "type": "string" + } + }, + "required": [ + "data" + ] + }, + "XRustBParam": { + "not": {} + }, "Foo": { "type": "string" } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 3f63e9b87..caa2d7eb6 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -772,6 +772,50 @@ } } }, + "/testing/{aa}/{bb}": { + "put": { + "tags": [ + "it" + ], + "operationId": "handler30", + "parameters": [ + { + "in": "path", + "name": "aa", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustAParam" + } + }, + { + "in": "path", + "name": "bb", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustBParam" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CoolStruct" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/thing_with_headers": { "get": { "tags": [ @@ -1196,6 +1240,46 @@ "items" ] }, + "WithXRustTypeForXRustAParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustAParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "WithXRustTypeForXRustBParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustBParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "XRustAParam": { + "type": "object", + "properties": { + "data": { + "type": "string" + } + }, + "required": [ + "data" + ] + }, + "XRustBParam": { + "not": {} + }, "Foo": { "type": "string" } diff --git a/dropshot_endpoint/src/metadata.rs b/dropshot_endpoint/src/metadata.rs index 2851a4c13..90168638b 100644 --- a/dropshot_endpoint/src/metadata.rs +++ b/dropshot_endpoint/src/metadata.rs @@ -1,4 +1,4 @@ -// Copyright 2024 Oxide Computer Company +// Copyright 2025 Oxide Computer Company //! Code to handle metadata associated with an endpoint.