From 7090fa5e42b9ee557346692b238d8a22cffbd019 Mon Sep 17 00:00:00 2001 From: Rain Date: Mon, 16 Jun 2025 17:55:06 +0000 Subject: [PATCH 1/5] [spr] initial version Created using spr 1.3.6-beta.1 --- dropshot/tests/integration-tests/openapi.rs | 63 +++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/dropshot/tests/integration-tests/openapi.rs b/dropshot/tests/integration-tests/openapi.rs index ae8c2cf94..bd46f2e12 100644 --- a/dropshot/tests/integration-tests/openapi.rs +++ b/dropshot/tests/integration-tests/openapi.rs @@ -548,6 +548,68 @@ async fn handler29( todo!() } +#[derive(Deserialize, JsonSchema)] +struct PathArgs30 { + #[expect(unused)] + thing: 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 XRustTypeParam { + #[expect(unused)] + data: String, +} + +#[endpoint { + method = PUT, + path = "/testing/{thing}" +}] +async fn handler30( + _: RequestContext<()>, + _: Path, +) -> Result, HttpError> { + todo!(); +} + fn make_api( maybe_tag_config: Option, ) -> Result, ApiDescriptionRegisterError> { @@ -586,6 +648,7 @@ fn make_api( api.register(handler27)?; api.register(handler28)?; api.register(handler29)?; + api.register(handler30)?; Ok(api) } From c7531cb6c9a1fe29bf52755538b004afa3884b22 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Fri, 20 Jun 2025 18:12:35 -0700 Subject: [PATCH 2/5] massively simplify argument schema processing --- dropshot/src/extractor/common.rs | 2 +- dropshot/src/extractor/metadata.rs | 22 ++++---- dropshot/src/handler.rs | 21 ++++---- dropshot/src/schema_util.rs | 45 +--------------- dropshot/tests/integration-tests/openapi.rs | 3 +- dropshot/tests/test_openapi.json | 60 +++++++++++++++++++++ dropshot/tests/test_openapi_fuller.json | 60 +++++++++++++++++++++ 7 files changed, 146 insertions(+), 67 deletions(-) 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..7fd9793ee 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -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..0ad3e0e7c 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) { @@ -842,6 +798,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 bd46f2e12..b447fa7cb 100644 --- a/dropshot/tests/integration-tests/openapi.rs +++ b/dropshot/tests/integration-tests/openapi.rs @@ -601,7 +601,8 @@ struct XRustTypeParam { #[endpoint { method = PUT, - path = "/testing/{thing}" + path = "/testing/{thing}", + tags = ["it"] }] async fn handler30( _: RequestContext<()>, diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 1ac4e3a49..3220265c5 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -764,6 +764,42 @@ } } }, + "/testing/{thing}": { + "put": { + "tags": [ + "it" + ], + "operationId": "handler30", + "parameters": [ + { + "in": "path", + "name": "thing", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustTypeParam" + } + } + ], + "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 +1224,30 @@ "items" ] }, + "WithXRustTypeForXRustTypeParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustTypeParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "XRustTypeParam": { + "type": "object", + "properties": { + "data": { + "type": "string" + } + }, + "required": [ + "data" + ] + }, "Foo": { "type": "string" } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 3f63e9b87..6f8cde2d3 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -772,6 +772,42 @@ } } }, + "/testing/{thing}": { + "put": { + "tags": [ + "it" + ], + "operationId": "handler30", + "parameters": [ + { + "in": "path", + "name": "thing", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustTypeParam" + } + } + ], + "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 +1232,30 @@ "items" ] }, + "WithXRustTypeForXRustTypeParam": { + "x-rust-type": { + "crate": "foo", + "parameters": [ + { + "$ref": "#/components/schemas/XRustTypeParam" + } + ], + "path": "foo", + "version": "*" + }, + "type": "string" + }, + "XRustTypeParam": { + "type": "object", + "properties": { + "data": { + "type": "string" + } + }, + "required": [ + "data" + ] + }, "Foo": { "type": "string" } From ec6194a4c7a44fb514d891773c6b6bbdb7e176a8 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Fri, 20 Jun 2025 18:34:58 -0700 Subject: [PATCH 3/5] allow for empty `enum` array --- dropshot/src/schema_util.rs | 107 +++++++++++++------- dropshot/tests/integration-tests/openapi.rs | 11 +- dropshot/tests/test_openapi.json | 36 +++++-- dropshot/tests/test_openapi_fuller.json | 36 +++++-- 4 files changed, 136 insertions(+), 54 deletions(-) diff --git a/dropshot/src/schema_util.rs b/dropshot/src/schema_util.rs index 0ad3e0e7c..2942bdb03 100644 --- a/dropshot/src/schema_util.rs +++ b/dropshot/src/schema_util.rs @@ -367,6 +367,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(_)) => { @@ -379,7 +446,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 { @@ -441,45 +508,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( diff --git a/dropshot/tests/integration-tests/openapi.rs b/dropshot/tests/integration-tests/openapi.rs index b447fa7cb..d7ea6a2d9 100644 --- a/dropshot/tests/integration-tests/openapi.rs +++ b/dropshot/tests/integration-tests/openapi.rs @@ -551,7 +551,9 @@ async fn handler29( #[derive(Deserialize, JsonSchema)] struct PathArgs30 { #[expect(unused)] - thing: WithXRustType, + aa: WithXRustType, + #[expect(unused)] + bb: WithXRustType, } #[derive(Deserialize, Debug)] @@ -594,14 +596,17 @@ impl JsonSchema for WithXRustType { } #[derive(Debug, Deserialize, JsonSchema)] -struct XRustTypeParam { +struct XRustAParam { #[expect(unused)] data: String, } +#[derive(Debug, Deserialize, JsonSchema)] +enum XRustBParam {} + #[endpoint { method = PUT, - path = "/testing/{thing}", + path = "/testing/{aa}/{bb}", tags = ["it"] }] async fn handler30( diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 3220265c5..8f063da22 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -764,7 +764,7 @@ } } }, - "/testing/{thing}": { + "/testing/{aa}/{bb}": { "put": { "tags": [ "it" @@ -773,10 +773,18 @@ "parameters": [ { "in": "path", - "name": "thing", + "name": "aa", "required": true, "schema": { - "$ref": "#/components/schemas/WithXRustTypeForXRustTypeParam" + "$ref": "#/components/schemas/WithXRustTypeForXRustAParam" + } + }, + { + "in": "path", + "name": "bb", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustBParam" } } ], @@ -1224,12 +1232,25 @@ "items" ] }, - "WithXRustTypeForXRustTypeParam": { + "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/XRustTypeParam" + "$ref": "#/components/schemas/XRustBParam" } ], "path": "foo", @@ -1237,7 +1258,7 @@ }, "type": "string" }, - "XRustTypeParam": { + "XRustAParam": { "type": "object", "properties": { "data": { @@ -1248,6 +1269,9 @@ "data" ] }, + "XRustBParam": { + "not": {} + }, "Foo": { "type": "string" } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 6f8cde2d3..caa2d7eb6 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -772,7 +772,7 @@ } } }, - "/testing/{thing}": { + "/testing/{aa}/{bb}": { "put": { "tags": [ "it" @@ -781,10 +781,18 @@ "parameters": [ { "in": "path", - "name": "thing", + "name": "aa", "required": true, "schema": { - "$ref": "#/components/schemas/WithXRustTypeForXRustTypeParam" + "$ref": "#/components/schemas/WithXRustTypeForXRustAParam" + } + }, + { + "in": "path", + "name": "bb", + "required": true, + "schema": { + "$ref": "#/components/schemas/WithXRustTypeForXRustBParam" } } ], @@ -1232,12 +1240,25 @@ "items" ] }, - "WithXRustTypeForXRustTypeParam": { + "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/XRustTypeParam" + "$ref": "#/components/schemas/XRustBParam" } ], "path": "foo", @@ -1245,7 +1266,7 @@ }, "type": "string" }, - "XRustTypeParam": { + "XRustAParam": { "type": "object", "properties": { "data": { @@ -1256,6 +1277,9 @@ "data" ] }, + "XRustBParam": { + "not": {} + }, "Foo": { "type": "string" } From 7ac4abe12af64561d917798630ac11c6e4ef4a23 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Fri, 20 Jun 2025 18:48:16 -0700 Subject: [PATCH 4/5] also handle the Bool(false) schema for funsies --- dropshot/src/schema_util.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/dropshot/src/schema_util.rs b/dropshot/src/schema_util.rs index 2942bdb03..dbae46322 100644 --- a/dropshot/src/schema_util.rs +++ b/dropshot/src/schema_util.rs @@ -335,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), } From b43bd9a6ca737bb92bd16e57cce93ad0e384d895 Mon Sep 17 00:00:00 2001 From: "Adam H. Leventhal" Date: Fri, 20 Jun 2025 20:43:23 -0700 Subject: [PATCH 5/5] update copyrights --- dropshot/src/handler.rs | 2 +- dropshot/tests/integration-tests/openapi.rs | 2 +- dropshot_endpoint/src/metadata.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 7fd9793ee..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, diff --git a/dropshot/tests/integration-tests/openapi.rs b/dropshot/tests/integration-tests/openapi.rs index d7ea6a2d9..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, 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.