From 41060ad2dab5d1f9deee920a97b79c6c159b2a54 Mon Sep 17 00:00:00 2001 From: Adam Leventhal Date: Thu, 1 Jul 2021 07:36:58 -0700 Subject: [PATCH] Add support for wildcard routes: `/foo/bar/{rest:.*}` (#110) * Add support for wildcard routes: `/foo/bar/{rest:.*}` This will match paths such as `/foo/bar` and `/foo/bar/baz.css` We can use this to serve static assets such as css, html, and image files. Express uses this syntax: `/foo/bar/:rest(.*)` Dropwizard / Jersey / JAX-RS does: `/foo/bar/{rest:.*}` Actix does: `/foo/bar/{rest:.*}` This also adds support for a tag (unpublished = true) in the #[endpoint] macro to omit certain endpoints from the OpenAPI output. This is both useful for non-API endpoints and necessary in that OpenAPI doesn't support any type of multi-segment matching of routes. --- CHANGELOG.adoc | 1 + dropshot/Cargo.toml | 1 + dropshot/examples/index.rs | 97 +++++ dropshot/src/api_description.rs | 17 +- dropshot/src/from_map.rs | 170 ++++++-- dropshot/src/handler.rs | 148 ++++++- dropshot/src/http_util.rs | 4 +- dropshot/src/router.rs | 534 ++++++++++++++++++++++-- dropshot/tests/test_openapi.json | 117 +----- dropshot/tests/test_openapi.rs | 86 ++-- dropshot/tests/test_openapi_fuller.json | 117 +----- dropshot/tests/test_openapi_old.json | 117 +----- dropshot_endpoint/src/lib.rs | 88 ++-- 13 files changed, 1006 insertions(+), 491 deletions(-) create mode 100644 dropshot/examples/index.rs diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 34219e88..77212aa2 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -17,6 +17,7 @@ https://github.com/oxidecomputer/dropshot/compare/v0.5.1\...HEAD[Full list of co * https://github.com/oxidecomputer/dropshot/pull/105[#105] When generating an OpenAPI spec, Dropshot now uses references rather than inline schemas to represent request and response bodies. * https://github.com/oxidecomputer/dropshot/pull/103[#103] When the Dropshot server is dropped before having been shut down, Dropshot now attempts to gracefully shut down rather than panic. +* https://github.com/oxidecomputer/dropshot/pull/110[#110] Wildcard paths are now supported. Consumers may take over routing (e.g. for file serving) by annotating a path component: `/static/{path:.*}`. The `path` member should then be of type `Vec` and it will be filled in with all path components following `/static/`. === Breaking Changes diff --git a/dropshot/Cargo.toml b/dropshot/Cargo.toml index e4520063..aaa5e248 100644 --- a/dropshot/Cargo.toml +++ b/dropshot/Cargo.toml @@ -27,6 +27,7 @@ slog-async = "2.4.0" slog-bunyan = "2.2.0" slog-json = "2.3.0" slog-term = "2.5.0" +syn = "1.0.73" toml = "0.5.6" [dependencies.chrono] diff --git a/dropshot/examples/index.rs b/dropshot/examples/index.rs new file mode 100644 index 00000000..a29a2ff0 --- /dev/null +++ b/dropshot/examples/index.rs @@ -0,0 +1,97 @@ +// Copyright 2021 Oxide Computer Company +/*! + * Example use of Dropshot for matching wildcard paths to serve static content. + */ + +use dropshot::ApiDescription; +use dropshot::ConfigDropshot; +use dropshot::ConfigLogging; +use dropshot::ConfigLoggingLevel; +use dropshot::HttpError; +use dropshot::HttpServerStarter; +use dropshot::RequestContext; +use dropshot::{endpoint, Path}; +use http::{Response, StatusCode}; +use hyper::Body; +use schemars::JsonSchema; +use serde::Deserialize; +use std::sync::Arc; + +#[tokio::main] +async fn main() -> Result<(), String> { + /* + * We must specify a configuration with a bind address. We'll use 127.0.0.1 + * since it's available and won't expose this server outside the host. We + * request port 0, which allows the operating system to pick any available + * port. + */ + let config_dropshot: ConfigDropshot = Default::default(); + + /* + * For simplicity, we'll configure an "info"-level logger that writes to + * stderr assuming that it's a terminal. + */ + let config_logging = ConfigLogging::StderrTerminal { + level: ConfigLoggingLevel::Info, + }; + let log = config_logging + .to_logger("example-basic") + .map_err(|error| format!("failed to create logger: {}", error))?; + + /* + * Build a description of the API. + */ + let mut api = ApiDescription::new(); + api.register(index).unwrap(); + + /* + * Set up the server. + */ + let server = HttpServerStarter::new(&config_dropshot, api, (), &log) + .map_err(|error| format!("failed to create server: {}", error))? + .start(); + + /* + * Wait for the server to stop. Note that there's not any code to shut down + * this server, so we should never get past this point. + */ + server.await +} + +#[derive(Deserialize, JsonSchema)] +struct AllPath { + path: Vec, +} + +/** + * Return static content.for all paths. + */ +#[endpoint { + method = GET, + + /* + * Match literally every path including the empty path. + */ + path = "/{path:.*}", + + /* + * This isn't an API so we don't want this to appear in the OpenAPI + * description if we were to generate it. + */ + unpublished = true, +}] +async fn index( + _rqctx: Arc>, + path: Path, +) -> Result, HttpError> { + Ok(Response::builder() + .status(StatusCode::OK) + .header(http::header::CONTENT_TYPE, "text/html") + .body( + format!( + "nothing at {:?}", + path.into_inner().path + ) + .into(), + )?) +} diff --git a/dropshot/src/api_description.rs b/dropshot/src/api_description.rs index 4d8c990b..855c222d 100644 --- a/dropshot/src/api_description.rs +++ b/dropshot/src/api_description.rs @@ -1,4 +1,4 @@ -// Copyright 2020 Oxide Computer Company +// Copyright 2021 Oxide Computer Company /*! * Describes the endpoints and handler functions in your API */ @@ -36,6 +36,7 @@ pub struct ApiEndpoint { pub description: Option, pub tags: Vec, pub paginated: bool, + pub visible: bool, } impl<'a, Context: ServerContext> ApiEndpoint { @@ -61,6 +62,7 @@ impl<'a, Context: ServerContext> ApiEndpoint { description: None, tags: vec![], paginated: func_parameters.paginated, + visible: true, } } @@ -73,6 +75,11 @@ impl<'a, Context: ServerContext> ApiEndpoint { self.tags.push(tag.to_string()); self } + + pub fn visible(mut self, visible: bool) -> Self { + self.visible = visible; + self + } } /** @@ -230,8 +237,9 @@ impl ApiDescription { let path = route_path_to_segments(&e.path) .iter() .filter_map(|segment| match PathSegment::from(segment) { - PathSegment::Varname(v) => Some(v), - _ => None, + PathSegment::VarnameSegment(v) => Some(v), + PathSegment::VarnameWildcard(v) => Some(v), + PathSegment::Literal(_) => None, }) .collect::>(); let vars = e @@ -380,6 +388,9 @@ impl ApiDescription { indexmap::IndexMap::::new(); for (path, method, endpoint) in &self.router { + if !endpoint.visible { + continue; + } let path = openapi.paths.entry(path).or_insert( openapiv3::ReferenceOr::Item(openapiv3::PathItem::default()), ); diff --git a/dropshot/src/from_map.rs b/dropshot/src/from_map.rs index 5ba1f830..369cab52 100644 --- a/dropshot/src/from_map.rs +++ b/dropshot/src/from_map.rs @@ -4,41 +4,65 @@ use paste::paste; use serde::de::DeserializeSeed; use serde::de::EnumAccess; use serde::de::MapAccess; +use serde::de::SeqAccess; use serde::de::VariantAccess; use serde::de::Visitor; use serde::Deserialize; use serde::Deserializer; use std::any::type_name; use std::collections::BTreeMap; +use std::fmt::Debug; use std::fmt::Display; /** - * Deserialize a BTreeMap into a type, invoking String::parse() - * for all values according to the required type. + * Deserialize a BTreeMap into a type, invoking + * String::parse() for all values according to the required type. MapValue may + * be either a single String or a sequence of Strings. */ -pub(crate) fn from_map<'a, T>( - map: &'a BTreeMap, +pub(crate) fn from_map<'a, T, Z>( + map: &'a BTreeMap, ) -> Result where T: Deserialize<'a>, + Z: MapValue + Debug + Clone + 'static, { let mut deserializer = MapDeserializer::from_map(map); - let x = T::deserialize(&mut deserializer); - x.map_err(|e| e.0) + T::deserialize(&mut deserializer).map_err(|e| e.0) +} + +pub(crate) trait MapValue { + fn as_value(&self) -> Result<&str, MapError>; + fn as_seq(&self) -> Result>, MapError>; +} + +impl MapValue for String { + fn as_value(&self) -> Result<&str, MapError> { + Ok(self.as_str()) + } + + fn as_seq(&self) -> Result>, MapError> { + Err(MapError( + "a string may not be used in place of a sequence of values" + .to_string(), + )) + } } /** - * Deserializer for BTreeMap that interprets the values. It has + * Deserializer for BTreeMap that interprets the values. It has * two modes: about to iterate over the map or about to process a single value. */ #[derive(Debug)] -enum MapDeserializer<'de> { - Map(&'de BTreeMap), - Value(String), +enum MapDeserializer<'de, Z: MapValue + Debug + Clone + 'static> { + Map(&'de BTreeMap), + Value(Z), } -impl<'de> MapDeserializer<'de> { - fn from_map(input: &'de BTreeMap) -> Self { +impl<'de, Z> MapDeserializer<'de, Z> +where + Z: MapValue + Debug + Clone + 'static, +{ + fn from_map(input: &'de BTreeMap) -> Self { MapDeserializer::Map(input) } @@ -48,7 +72,7 @@ impl<'de> MapDeserializer<'de> { */ fn value(&self, deserialize: F) -> Result where - F: FnOnce(&String) -> Result, + F: FnOnce(&Z) -> Result, { match self { MapDeserializer::Value(ref raw_value) => deserialize(raw_value), @@ -61,7 +85,7 @@ impl<'de> MapDeserializer<'de> { } #[derive(Clone, Debug)] -struct MapError(String); +pub(crate) struct MapError(pub String); impl Display for MapError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -89,7 +113,6 @@ macro_rules! de_unimp { where V: Visitor<'de>, { - println!("{:?}", self); unimplemented!(stringify!($i)); } }; @@ -111,11 +134,11 @@ macro_rules! de_value { where V: Visitor<'de>, { - self.value(|raw_value| match raw_value.parse::<$i>() { + self.value(|raw_value| match raw_value.as_value()?.parse::<$i>() { Ok(value) => visitor.[](value), Err(_) => Err(MapError(format!( "unable to parse '{}' as {}", - raw_value, + raw_value.as_value()?, type_name::<$i>() ))), }) @@ -124,7 +147,10 @@ macro_rules! de_value { }; } -impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { +impl<'de, 'a, Z> Deserializer<'de> for &'a mut MapDeserializer<'de, Z> +where + Z: MapValue + Debug + Clone + 'static, +{ type Error = MapError; // Simple values @@ -145,13 +171,13 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { where V: Visitor<'de>, { - self.value(|raw_value| visitor.visit_str(raw_value.as_str())) + self.value(|raw_value| visitor.visit_str(raw_value.as_value()?)) } fn deserialize_string(self, visitor: V) -> Result where V: Visitor<'de>, { - self.value(|raw_value| visitor.visit_str(raw_value.as_str())) + self.value(|raw_value| visitor.visit_str(raw_value.as_value()?)) } fn deserialize_option(self, visitor: V) -> Result @@ -183,11 +209,13 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { { match self { MapDeserializer::Map(map) => { - let x = Box::new(map.clone().into_iter()); - visitor.visit_map(MapMapAccess { + let xx = map.clone(); + let x = Box::new(xx.into_iter()); + let m = MapMapAccess:: { iter: x, value: None, - }) + }; + visitor.visit_map(m) } MapDeserializer::Value(_) => Err(MapError( "destination struct must be fully flattened".to_string(), @@ -201,7 +229,7 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { where V: Visitor<'de>, { - self.value(|raw_value| visitor.visit_str(raw_value.as_str())) + self.value(|raw_value| visitor.visit_str(raw_value.as_value()?)) } fn deserialize_enum( @@ -223,7 +251,7 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { where V: Visitor<'de>, { - self.value(|raw_value| visitor.visit_str(raw_value.as_str())) + self.value(|raw_value| visitor.visit_str(raw_value.as_value()?)) } /* @@ -251,7 +279,7 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { where V: Visitor<'de>, { - self.value(|raw_value| visitor.visit_str(raw_value.as_str())) + self.value(|raw_value| visitor.visit_str(raw_value.as_value()?)) } de_unimp!(deserialize_bytes); @@ -259,15 +287,30 @@ impl<'de, 'a> Deserializer<'de> for &'a mut MapDeserializer<'de> { de_unimp!(deserialize_unit); de_unimp!(deserialize_unit_struct, _name: &'static str); de_unimp!(deserialize_newtype_struct, _name: &'static str); - de_unimp!(deserialize_seq); de_unimp!(deserialize_tuple, _len: usize); de_unimp!(deserialize_tuple_struct, _name: &'static str, _len: usize); + + fn deserialize_seq(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + self.value(|raw_value| { + let x = raw_value.as_seq()?; + let xx = MapSeqAccess { + iter: x, + }; + visitor.visit_seq(xx) + }) + } } /* * Deserializer component for processing enums. */ -impl<'de, 'a> EnumAccess<'de> for &mut MapDeserializer<'de> { +impl<'de, 'a, Z> EnumAccess<'de> for &mut MapDeserializer<'de, Z> +where + Z: MapValue + Debug + Clone + 'static, +{ type Error = MapError; type Variant = Self; @@ -285,7 +328,10 @@ impl<'de, 'a> EnumAccess<'de> for &mut MapDeserializer<'de> { /* * Deserializer component for processing enum variants. */ -impl<'de, 'a> VariantAccess<'de> for &mut MapDeserializer<'de> { +impl<'de, 'a, Z> VariantAccess<'de> for &mut MapDeserializer<'de, Z> +where + Z: MapValue + Clone + Debug + 'static, +{ type Error = MapError; fn unit_variant(self) -> Result<(), MapError> { @@ -325,14 +371,17 @@ impl<'de, 'a> VariantAccess<'de> for &mut MapDeserializer<'de> { /* * Deserializer component for iterating over the Map. */ -struct MapMapAccess { +struct MapMapAccess { /** Iterator through the Map */ - iter: Box>, + iter: Box>, /** Pending value in a key-value pair */ - value: Option, + value: Option, } -impl<'de, 'a> MapAccess<'de> for MapMapAccess { +impl<'de, 'a, Z> MapAccess<'de> for MapMapAccess +where + Z: MapValue + Debug + Clone + 'static, +{ type Error = MapError; fn next_key_seed( @@ -366,7 +415,34 @@ impl<'de, 'a> MapAccess<'de> for MapMapAccess { * This means we were called without a corresponding call to * next_key_seed() which should not be possible. */ - None => panic!("unreachable"), + None => unreachable!(), + } + } +} + +struct MapSeqAccess { + iter: Box>, +} + +impl<'de, 'a, Z> SeqAccess<'de> for MapSeqAccess +where + Z: MapValue + Debug + Clone + 'static, +{ + type Error = MapError; + + fn next_element_seed( + &mut self, + seed: T, + ) -> Result, Self::Error> + where + T: DeserializeSeed<'de>, + { + match self.iter.next() { + Some(value) => { + let mut deserializer = MapDeserializer::Value(value); + seed.deserialize(&mut deserializer).map(Some) + } + None => Ok(None), } } } @@ -380,7 +456,7 @@ mod test { #[test] fn test_lone_literal() { let map = BTreeMap::new(); - match from_map::(&map) { + match from_map::(&map) { Err(s) => assert_eq!( s, "must be applied to a flattened struct rather than a raw type" @@ -401,7 +477,7 @@ mod test { } let mut map = BTreeMap::new(); map.insert("b".to_string(), "B".to_string()); - match from_map::(&map) { + match from_map::(&map) { Err(s) => { assert_eq!(s, "destination struct must be fully flattened") } @@ -422,7 +498,7 @@ mod test { } let mut map = BTreeMap::new(); map.insert("aaa".to_string(), "A".to_string()); - match from_map::(&map) { + match from_map::(&map) { Err(s) => assert_eq!(s, "missing field `sss`"), Ok(_) => panic!("unexpected success"), } @@ -441,7 +517,7 @@ mod test { } let mut map = BTreeMap::new(); map.insert("sss".to_string(), "stringy".to_string()); - match from_map::(&map) { + match from_map::(&map) { Err(s) => assert_eq!(s, "missing field `aaa`"), Ok(_) => panic!("unexpected success"), } @@ -473,7 +549,7 @@ mod test { map.insert("aoption".to_string(), "8".to_string()); map.insert("bstring".to_string(), "B string".to_string()); //map.insert("bbool".to_string(), "true".to_string()); - match from_map::(&map) { + match from_map::(&map) { Ok(a) => { assert_eq!(a.astring, "A string"); assert_eq!(a.au32, 1000); @@ -486,4 +562,20 @@ mod test { Err(s) => panic!("error: {}", s), } } + #[test] + fn wherefore_art_thou_a_valid_sequence_when_in_fact_you_are_a_lone_value() { + #[derive(Deserialize, Debug)] + struct A { + b: Vec, + } + let mut map = BTreeMap::new(); + map.insert("b".to_string(), "stringy".to_string()); + match from_map::(&map) { + Err(s) => assert_eq!( + s, + "a string may not be used in place of a sequence of values" + ), + Ok(_) => panic!("unexpected success"), + } + } } diff --git a/dropshot/src/handler.rs b/dropshot/src/handler.rs index 3d01e382..1e36c786 100644 --- a/dropshot/src/handler.rs +++ b/dropshot/src/handler.rs @@ -46,6 +46,7 @@ use crate::api_description::ApiEndpointResponse; use crate::api_description::ApiSchemaGenerator; use crate::pagination::PaginationParams; use crate::pagination::PAGINATION_PARAM_SENTINEL; +use crate::router::VariableSet; use async_trait::async_trait; use bytes::Bytes; @@ -61,7 +62,6 @@ use serde::de::DeserializeOwned; use serde::Serialize; use slog::Logger; use std::cmp::min; -use std::collections::BTreeMap; use std::fmt::Debug; use std::fmt::Formatter; use std::fmt::Result as FmtResult; @@ -93,7 +93,7 @@ pub struct RequestContext { /** HTTP request details */ pub request: Arc>>, /** HTTP request routing variables */ - pub path_variables: BTreeMap, + pub path_variables: VariableSet, /** unique id assigned to this request */ pub request_id: String, /** logger for this specific request */ @@ -801,6 +801,8 @@ fn schema2parameters( if let Some(object) = object { parameters.extend(object.properties.iter().map( |(name, schema)| { + validate_parameter_schema(loc, name, schema, generator); + // We won't often see referenced schemas here, but we may // in the case of enumerated strings. To handle this, we // package up the dependencies to include in the top- @@ -878,6 +880,148 @@ fn schema2parameters( } } +/** + * Confirm that the parameter schema type is either an atomic value (i.e. + * string, number, boolean) or an array of strings (to accommodate wild card + * paths). + */ +fn validate_parameter_schema( + loc: &ApiEndpointParameterLocation, + name: &String, + schema: &schemars::schema::Schema, + generator: &schemars::gen::SchemaGenerator, +) { + match schema { + /* + * We expect references to be on their own. + */ + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + metadata: _, + instance_type: None, + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: None, + array: None, + object: None, + reference: Some(_), + extensions: _, + }) => validate_parameter_schema( + loc, + name, + generator.dereference(schema).expect("invalid reference"), + generator, + ), + + /* + * Match atomic value types. + */ + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + instance_type: + Some(schemars::schema::SingleOrVec::Single(instance_type)), + subschemas: None, + array: None, + object: None, + reference: None, + .. + }) if match instance_type.as_ref() { + InstanceType::Boolean + | InstanceType::Number + | InstanceType::String + | InstanceType::Integer => true, + _ => false, + } => + { /* All good. */ } + + /* + * For path parameters only, match array types and validate that their + * items are strings. + */ + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + metadata: _, + instance_type: + Some(schemars::schema::SingleOrVec::Single(instance_type)), + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: None, + array: Some(array_validation), + object: None, + reference: None, + extensions: _, + }) if matches!(loc, ApiEndpointParameterLocation::Path) + && matches!(instance_type.as_ref(), InstanceType::Array) => + { + match array_validation.as_ref() { + schemars::schema::ArrayValidation { + items: + Some(schemars::schema::SingleOrVec::Single(item_schema)), + additional_items: None, + .. + } => validate_string_schema(name, item_schema, generator), + _ => panic!("parameter {} has an invalid array type", name), + } + } + + _ => panic!("parameter {} has an invalid type", name), + } +} + +/** + * Validate that the given schema is for a simple string type. + */ +fn validate_string_schema( + name: &String, + schema: &schemars::schema::Schema, + generator: &schemars::gen::SchemaGenerator, +) { + match schema { + /* + * We expect references to be on their own. + */ + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + metadata: _, + instance_type: None, + format: None, + enum_values: None, + const_value: None, + subschemas: None, + number: None, + string: None, + array: None, + object: None, + reference: Some(_), + extensions: _, + }) => validate_string_schema( + name, + generator.dereference(schema).expect("invalid reference"), + generator, + ), + + /* + * Match string value types. + */ + schemars::schema::Schema::Object(schemars::schema::SchemaObject { + instance_type: + Some(schemars::schema::SingleOrVec::Single(instance_type)), + subschemas: None, + array: None, + object: None, + reference: None, + .. + }) if matches!(instance_type.as_ref(), InstanceType::String) => { /* All good. */ + } + + _ => { + panic!("parameter {} is an array of a type other than string", name) + } + } +} + /* * TypedBody: body extractor for formats that can be deserialized to a specific * type. Only JSON is currently supported. diff --git a/dropshot/src/http_util.rs b/dropshot/src/http_util.rs index ddf1c9d3..68170d99 100644 --- a/dropshot/src/http_util.rs +++ b/dropshot/src/http_util.rs @@ -7,10 +7,10 @@ use bytes::BufMut; use bytes::Bytes; use hyper::body::HttpBody; use serde::de::DeserializeOwned; -use std::collections::BTreeMap; use super::error::HttpError; use crate::from_map::from_map; +use crate::router::VariableSet; /** header name for conveying request ids ("x-request-id") */ pub const HEADER_REQUEST_ID: &str = "x-request-id"; @@ -136,7 +136,7 @@ where * TODO-testing: Add automated tests. */ pub fn http_extract_path_params( - path_params: &BTreeMap, + path_params: &VariableSet, ) -> Result { from_map(path_params).map_err(|message| { /* diff --git a/dropshot/src/router.rs b/dropshot/src/router.rs index 146d4e7d..87195d5e 100644 --- a/dropshot/src/router.rs +++ b/dropshot/src/router.rs @@ -1,4 +1,4 @@ -// Copyright 2020 Oxide Computer Company +// Copyright 2021 Oxide Computer Company /*! * Routes incoming HTTP requests to handler functions */ @@ -6,6 +6,8 @@ use super::error::HttpError; use super::handler::RouteHandler; +use crate::from_map::MapError; +use crate::from_map::MapValue; use crate::server::ServerContext; use crate::ApiEndpoint; use http::Method; @@ -92,21 +94,27 @@ struct HttpRouterNode { enum HttpRouterEdges { /** Outgoing edges for literal paths. */ Literals(BTreeMap>>), - /** Outgoing edges for variable-named paths. */ - Variable(String, Box>), + /** Outgoing edge for variable-named paths. */ + VariableSingle(String, Box>), + /** Outgoing edge that consumes all remaining components. */ + VariableRest(String, Box>), } /** * `PathSegment` represents a segment in a URI path when the router is being * configured. Each segment may be either a literal string or a variable (the - * latter indicated by being wrapped in braces. + * latter indicated by being wrapped in braces). Variables may consume a single + * /-delimited segment or several as defined by a regex (currently only `.*` is + * supported). */ -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum PathSegment { /** a path segment for a literal string */ Literal(String), /** a path segment for a variable */ - Varname(String), + VarnameSegment(String), + /** a path segment that matches all remaining components for a variable */ + VarnameWildcard(String), } impl PathSegment { @@ -128,12 +136,31 @@ impl PathSegment { "{}", "HTTP URI path segment variable missing trailing \"}\"" ); + + let var = &segment[1..segment.len() - 1]; + + let (var, pat) = if let Some(index) = var.find(':') { + (&var[..index], Some(&var[index + 1..])) + } else { + (var, None) + }; + assert!( - segment.len() > 2, - "HTTP URI path segment variable name cannot be empty" + valid_identifier(var), + "HTTP URI path segment variable name must be a valid \ + identifier: '{}'", + var ); - PathSegment::Varname((&segment[1..segment.len() - 1]).to_string()) + if let Some(pat) = pat { + assert!( + pat == ".*", + "Only the pattern '.*' is currently supported" + ); + PathSegment::VarnameWildcard(var.to_string()) + } else { + PathSegment::VarnameSegment(var.to_string()) + } } else { PathSegment::Literal(segment.to_string()) } @@ -153,6 +180,45 @@ impl<'a> From<&'a str> for InputPath<'a> { } } +/* + * Validate that the string is a valid Rust identifier. + */ +fn valid_identifier(var: &str) -> bool { + syn::parse_str::(var).is_ok() +} + +/** + * A value for a variable which may either be a single value or a list of + * values in the case of wildcard path matching. + */ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum VariableValue { + String(String), + Components(Vec), +} + +pub type VariableSet = BTreeMap; + +impl MapValue for VariableValue { + fn as_value(&self) -> Result<&str, MapError> { + match self { + VariableValue::String(s) => Ok(s.as_str()), + VariableValue::Components(_) => Err(MapError( + "cannot deserialize sequence as a single value".to_string(), + )), + } + } + + fn as_seq(&self) -> Result>, MapError> { + match self { + VariableValue::String(_) => Err(MapError( + "cannot deserialize a single value as a sequence".to_string(), + )), + VariableValue::Components(v) => Ok(Box::new(v.clone().into_iter())), + } + } +} + /** * `RouterLookupResult` represents the result of invoking * `HttpRouter::lookup_route()`. A successful route lookup includes both the @@ -162,7 +228,7 @@ impl<'a> From<&'a str> for InputPath<'a> { #[derive(Debug)] pub struct RouterLookupResult<'a, Context: ServerContext> { pub handler: &'a dyn RouteHandler, - pub variables: BTreeMap, + pub variables: VariableSet, } impl HttpRouterNode { @@ -194,10 +260,12 @@ impl HttpRouter { let path = endpoint.path.clone(); let all_segments = route_path_to_segments(path.as_str()); + + let mut all_segments = all_segments.into_iter(); let mut varnames: BTreeSet = BTreeSet::new(); let mut node: &mut Box> = &mut self.root; - for raw_segment in all_segments { + while let Some(raw_segment) = all_segments.next() { let segment = PathSegment::from(raw_segment); node = match segment { @@ -212,7 +280,8 @@ impl HttpRouter { * caveats about how matching would work), but it seems * more likely to be a mistake. */ - HttpRouterEdges::Variable(varname, _) => { + HttpRouterEdges::VariableSingle(varname, _) + | HttpRouterEdges::VariableRest(varname, _) => { panic!( "URI path \"{}\": attempted to register route \ for literal path segment \"{}\" when a route \ @@ -227,26 +296,80 @@ impl HttpRouter { } } - PathSegment::Varname(new_varname) => { + PathSegment::VarnameSegment(new_varname) => { + insert_var(&path, &mut varnames, &new_varname); + + let edges = node.edges.get_or_insert( + HttpRouterEdges::VariableSingle( + new_varname.clone(), + Box::new(HttpRouterNode::new()), + ), + ); + match edges { + /* + * See the analogous check above about combining literal + * and variable path segments from the same resource. + */ + HttpRouterEdges::Literals(_) => panic!( + "URI path \"{}\": attempted to register route for \ + variable path segment (variable name: \"{}\") \ + when a route already exists for a literal path \ + segment", + path, new_varname + ), + + HttpRouterEdges::VariableRest(varname, _) => panic!( + "URI path \"{}\": attempted to register route for \ + variable path segment (variable name: \"{}\") \ + when a route already exists for the remainder of \ + the path as {}", + path, new_varname, varname, + ), + + HttpRouterEdges::VariableSingle( + varname, + ref mut node, + ) => { + if *new_varname != *varname { + /* + * Don't allow people to use different names for + * the same part of the path. Again, this could + * be supported, but it seems likely to be + * confusing and probably a mistake. + */ + panic!( + "URI path \"{}\": attempted to use \ + variable name \"{}\", but a different \ + name (\"{}\") has already been used for \ + this", + path, new_varname, varname + ); + } + + node + } + } + } + PathSegment::VarnameWildcard(new_varname) => { /* - * Do not allow the same variable name to be used more than - * once in the path. Again, this could be supported (with - * some caveats), but it seems more likely to be a mistake. + * We don't accept further path segments after the .*. */ - if varnames.contains(&new_varname) { + if all_segments.next().is_some() { panic!( - "URI path \"{}\": variable name \"{}\" is used \ - more than once", - path, new_varname + "URI path \"{}\": attempted to match segments \ + after the wildcard variable \"{}\"", + path, new_varname, ); } - varnames.insert(new_varname.clone()); - let edges = - node.edges.get_or_insert(HttpRouterEdges::Variable( + insert_var(&path, &mut varnames, &new_varname); + + let edges = node.edges.get_or_insert( + HttpRouterEdges::VariableRest( new_varname.clone(), Box::new(HttpRouterNode::new()), - )); + ), + ); match edges { /* * See the analogous check above about combining literal @@ -254,13 +377,22 @@ impl HttpRouter { */ HttpRouterEdges::Literals(_) => panic!( "URI path \"{}\": attempted to register route for \ - variable path segment (variable name: \"{}\") \ - when a route already exists for a literal path \ - segment", + variable path regex (variable name: \"{}\") when \ + a route already exists for a literal path segment", path, new_varname ), - HttpRouterEdges::Variable(varname, ref mut node) => { + HttpRouterEdges::VariableSingle(varname, _) => panic!( + "URI path \"{}\": attempted to register route for \ + variable path regex (variable name: \"{}\") when \ + a route already exists for a segment {}", + path, new_varname, varname, + ), + + HttpRouterEdges::VariableRest( + varname, + ref mut node, + ) => { if *new_varname != *varname { /* * Don't allow people to use different names for @@ -315,19 +447,40 @@ impl HttpRouter { String::from("invalid path encoding"), ) })?; + let mut all_segments = all_segments.into_iter(); let mut node = &self.root; - let mut variables: BTreeMap = BTreeMap::new(); + let mut variables = VariableSet::new(); - for segment in all_segments { + while let Some(segment) = all_segments.next() { let segment_string = segment.to_string(); node = match &node.edges { None => None, + Some(HttpRouterEdges::Literals(edges)) => { edges.get(&segment_string) } - Some(HttpRouterEdges::Variable(varname, ref node)) => { - variables.insert(varname.clone(), segment_string); + Some(HttpRouterEdges::VariableSingle(varname, ref node)) => { + variables.insert( + varname.clone(), + VariableValue::String(segment_string), + ); + Some(node) + } + Some(HttpRouterEdges::VariableRest(varname, node)) => { + let mut rest = vec![segment]; + while let Some(segment) = all_segments.next() { + rest.push(segment); + } + variables.insert( + varname.clone(), + VariableValue::Components(rest), + ); + /* + * There should be no outgoing edges since this is by + * definition a terminal node + */ + assert!(node.edges.is_none()); Some(node) } } @@ -339,6 +492,20 @@ impl HttpRouter { })? } + /* + * The wildcard match consumes the implicit, empty path segment + */ + match &node.edges { + Some(HttpRouterEdges::VariableRest(varname, new_node)) => { + variables + .insert(varname.clone(), VariableValue::Components(vec![])); + /* There should be no outgoing edges */ + assert!(new_node.edges.is_none()); + node = new_node; + } + _ => {} + } + /* * As a somewhat special case, if one requests a node with no handlers * at all, report a 404. We could probably treat this as a 405 as well. @@ -363,6 +530,28 @@ impl HttpRouter { } } +/** + * Insert a variable into the set after checking for duplicates. + */ +fn insert_var( + path: &str, + varnames: &mut BTreeSet, + new_varname: &String, +) -> () { + /* + * Do not allow the same variable name to be used more than + * once in the path. Again, this could be supported (with + * some caveats), but it seems more likely to be a mistake. + */ + if varnames.contains(new_varname) { + panic!( + "URI path \"{}\": variable name \"{}\" is used more than once", + path, new_varname + ); + } + varnames.insert(new_varname.clone()); +} + impl<'a, Context: ServerContext> IntoIterator for &'a HttpRouter { type Item = (String, String, &'a ApiEndpoint); type IntoIter = HttpRouterIter<'a, Context>; @@ -414,9 +603,18 @@ impl<'a, Context: ServerContext> HttpRouterIter<'a, Context> { map.iter() .map(|(s, node)| (PathSegment::Literal(s.clone()), node)), ), - Some(HttpRouterEdges::Variable(ref varname, ref node)) => Box::new( - std::iter::once((PathSegment::Varname(varname.clone()), node)), - ), + Some(HttpRouterEdges::VariableSingle(varname, node)) => { + Box::new(std::iter::once(( + PathSegment::VarnameSegment(varname.clone()), + node, + ))) + } + Some(HttpRouterEdges::VariableRest(varname, node)) => { + Box::new(std::iter::once(( + PathSegment::VarnameSegment(varname.clone()), + node, + ))) + } None => Box::new(std::iter::empty()), } } @@ -430,7 +628,8 @@ impl<'a, Context: ServerContext> HttpRouterIter<'a, Context> { .iter() .map(|(c, _)| match c { PathSegment::Literal(s) => s.clone(), - PathSegment::Varname(s) => format!("{{{}}}", s), + PathSegment::VarnameSegment(s) => format!("{{{}}}", s), + PathSegment::VarnameWildcard(s) => format!("{{{}:.*}}", s), }) .collect(); @@ -583,12 +782,17 @@ mod test { use super::super::handler::RouteHandler; use super::input_path_to_segments; use super::HttpRouter; + use super::PathSegment; + use crate::from_map::from_map; + use crate::router::VariableValue; use crate::ApiEndpoint; use crate::ApiEndpointResponse; use http::Method; use http::StatusCode; use hyper::Body; use hyper::Response; + use serde::Deserialize; + use std::collections::BTreeMap; use std::sync::Arc; async fn test_handler( @@ -624,13 +828,13 @@ mod test { description: None, tags: vec![], paginated: false, + visible: true, } } #[test] - #[should_panic( - expected = "HTTP URI path segment variable name cannot be empty" - )] + #[should_panic(expected = "HTTP URI path segment variable name must be a \ + valid identifier: ''")] fn test_variable_name_empty() { let mut router = HttpRouter::new(); router.insert(new_endpoint(new_handler(), Method::GET, "/foo/{}")); @@ -757,6 +961,64 @@ mod test { )); } + #[test] + #[should_panic(expected = "URI path \"/projects/default\": attempted to \ + register route for literal path segment \ + \"default\" when a route exists for variable \ + path segment (variable name: \"rest\")")] + fn test_literal_after_regex() { + let mut router = HttpRouter::new(); + router.insert(new_endpoint( + new_handler(), + Method::GET, + "/projects/{rest:.*}", + )); + router.insert(new_endpoint( + new_handler(), + Method::GET, + "/projects/default", + )); + } + + #[test] + #[should_panic(expected = "Only the pattern '.*' is currently supported")] + fn test_bogus_regex() { + let mut router = HttpRouter::new(); + router.insert(new_endpoint( + new_handler(), + Method::GET, + "/word/{rest:[a-z]*}", + )); + } + + #[test] + #[should_panic(expected = "URI path \"/some/{more:.*}/{stuff}\": \ + attempted to match segments after the \ + wildcard variable \"more\"")] + fn test_more_after_regex() { + let mut router = HttpRouter::new(); + router.insert(new_endpoint( + new_handler(), + Method::GET, + "/some/{more:.*}/{stuff}", + )); + } + + /* + * TODO: We allow a trailing slash after the wildcard specifier, but we may + * reconsider this if we decided to distinguish between the presence or + * absence of the trailing slash. + */ + #[test] + fn test_slash_after_wildcard_is_fine_dot_dot_dot_for_now() { + let mut router = HttpRouter::new(); + router.insert(new_endpoint( + new_handler(), + Method::GET, + "/some/{more:.*}/", + )); + } + #[test] fn test_error_cases() { let mut router = HttpRouter::new(); @@ -944,7 +1206,10 @@ mod test { assert_eq!(result.variables.keys().collect::>(), vec![ "project_id" ]); - assert_eq!(result.variables.get("project_id").unwrap(), "p12345"); + assert_eq!( + *result.variables.get("project_id").unwrap(), + VariableValue::String("p12345".to_string()) + ); assert!(router .lookup_route(&Method::GET, "/projects/p12345/child".into()) .is_err()); @@ -952,18 +1217,27 @@ mod test { .lookup_route(&Method::GET, "/projects/p12345/".into()) .unwrap(); assert_eq!(result.handler.label(), "h5"); - assert_eq!(result.variables.get("project_id").unwrap(), "p12345"); + assert_eq!( + *result.variables.get("project_id").unwrap(), + VariableValue::String("p12345".to_string()) + ); let result = router .lookup_route(&Method::GET, "/projects///p12345//".into()) .unwrap(); assert_eq!(result.handler.label(), "h5"); - assert_eq!(result.variables.get("project_id").unwrap(), "p12345"); + assert_eq!( + *result.variables.get("project_id").unwrap(), + VariableValue::String("p12345".to_string()) + ); /* Trick question! */ let result = router .lookup_route(&Method::GET, "/projects/{project_id}".into()) .unwrap(); assert_eq!(result.handler.label(), "h5"); - assert_eq!(result.variables.get("project_id").unwrap(), "{project_id}"); + assert_eq!( + *result.variables.get("project_id").unwrap(), + VariableValue::String("{project_id}".to_string()) + ); } #[test] @@ -990,9 +1264,18 @@ mod test { "instance_id", "project_id" ]); - assert_eq!(result.variables.get("project_id").unwrap(), "p1"); - assert_eq!(result.variables.get("instance_id").unwrap(), "i2"); - assert_eq!(result.variables.get("fwrule_id").unwrap(), "fw3"); + assert_eq!( + *result.variables.get("project_id").unwrap(), + VariableValue::String("p1".to_string()) + ); + assert_eq!( + *result.variables.get("instance_id").unwrap(), + VariableValue::String("i2".to_string()) + ); + assert_eq!( + *result.variables.get("fwrule_id").unwrap(), + VariableValue::String("fw3".to_string()) + ); } #[test] @@ -1022,6 +1305,28 @@ mod test { assert_eq!(result.handler.label(), "h7"); } + #[test] + fn test_variables_glob() { + let mut router = HttpRouter::new(); + router.insert(new_endpoint( + new_handler_named("h8"), + Method::OPTIONS, + "/console/{path:.*}", + )); + + let result = router + .lookup_route(&Method::OPTIONS, "/console/missiles/launch".into()) + .unwrap(); + + assert_eq!( + result.variables.get("path"), + Some(&VariableValue::Components(vec![ + "missiles".to_string(), + "launch".to_string() + ])) + ); + } + #[test] fn test_iter_null() { let router = HttpRouter::<()>::new(); @@ -1075,4 +1380,141 @@ mod test { input_path_to_segments(&"//foo/bar/baz%2fbuzz".into()).unwrap(); assert_eq!(segs, vec!["foo", "bar", "baz/buzz"]); } + + #[test] + fn test_path_segment() { + let seg = PathSegment::from("abc"); + assert_eq!(seg, PathSegment::Literal("abc".to_string())); + + let seg = PathSegment::from("{words}"); + assert_eq!(seg, PathSegment::VarnameSegment("words".to_string())); + + let seg = PathSegment::from("{rest:.*}"); + assert_eq!(seg, PathSegment::VarnameWildcard("rest".to_string()),); + } + + #[test] + #[should_panic] + fn test_bad_path_segment1() { + let _ = PathSegment::from("{foo"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment2() { + let _ = PathSegment::from("bar}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment3() { + let _ = PathSegment::from("{867_5309}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment4() { + let _ = PathSegment::from("{_}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment5() { + let _ = PathSegment::from("{...}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment6() { + let _ = PathSegment::from("{}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment7() { + let _ = PathSegment::from("{}"); + } + + #[test] + #[should_panic] + fn test_bad_path_segment8() { + let _ = PathSegment::from("{varname:abc+}"); + } + + #[test] + fn test_map() { + #[derive(Deserialize)] + struct A { + bbb: String, + ccc: Vec, + } + + let mut map = BTreeMap::new(); + map.insert( + "bbb".to_string(), + VariableValue::String("doggos".to_string()), + ); + map.insert( + "ccc".to_string(), + VariableValue::Components(vec![ + "lizzie".to_string(), + "brickley".to_string(), + ]), + ); + + match from_map::(&map) { + Ok(a) => { + assert_eq!(a.bbb, "doggos"); + assert_eq!(a.ccc, vec!["lizzie", "brickley"]); + } + Err(s) => panic!("unexpected error: {}", s), + } + } + + #[test] + fn test_map_bad_value() { + #[allow(dead_code)] + #[derive(Deserialize)] + struct A { + bbb: String, + } + + let mut map = BTreeMap::new(); + map.insert( + "bbb".to_string(), + VariableValue::Components(vec![ + "lizzie".to_string(), + "brickley".to_string(), + ]), + ); + + match from_map::(&map) { + Ok(_) => panic!("unexpected success"), + Err(s) => { + assert_eq!(s, "cannot deserialize sequence as a single value") + } + } + } + + #[test] + fn test_map_bad_seq() { + #[allow(dead_code)] + #[derive(Deserialize)] + struct A { + bbb: Vec, + } + + let mut map = BTreeMap::new(); + map.insert( + "bbb".to_string(), + VariableValue::String("doggos".to_string()), + ); + + match from_map::(&map) { + Ok(_) => panic!("unexpected success"), + Err(s) => { + assert_eq!(s, "cannot deserialize a single value as a sequence") + } + } + } } diff --git a/dropshot/tests/test_openapi.json b/dropshot/tests/test_openapi.json index 6d3b9a23..a2d0b730 100644 --- a/dropshot/tests/test_openapi.json +++ b/dropshot/tests/test_openapi.json @@ -60,51 +60,9 @@ } } }, - "/dup3": { - "put": { - "operationId": "handler10", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, - "/dup4": { - "put": { - "operationId": "handler11", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, "/dup5": { "put": { - "operationId": "handler12", + "operationId": "handler10", "requestBody": { "content": { "application/json": { @@ -124,7 +82,7 @@ }, "/dup6": { "put": { - "operationId": "handler13", + "operationId": "handler11", "requestBody": { "content": { "application/json": { @@ -144,7 +102,7 @@ }, "/dup7": { "put": { - "operationId": "handler14", + "operationId": "handler12", "requestBody": { "content": { "application/json": { @@ -164,7 +122,7 @@ }, "/dup8": { "get": { - "operationId": "handler15", + "operationId": "handler13", "responses": { "200": { "description": "successful operation", @@ -310,21 +268,7 @@ }, { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -333,7 +277,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -364,21 +308,7 @@ "parameters": [ { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -387,7 +317,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -407,25 +337,25 @@ "BodyParam": { "type": "object", "properties": { - "_any": {}, - "_x": { + "any": {}, + "x": { "type": "string" } }, "required": [ - "_any", - "_x" + "any", + "x" ] }, "NeverDuplicatedBodyNextLevel": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedBodyTopLevel": { @@ -442,12 +372,12 @@ "NeverDuplicatedNext": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedResponseNextLevel": { @@ -475,12 +405,12 @@ "NeverDuplicatedTop": { "type": "object", "properties": { - "_b": { + "b": { "$ref": "#/components/schemas/NeverDuplicatedNext" } }, "required": [ - "_b" + "b" ] }, "Response": { @@ -516,17 +446,6 @@ "required": [ "items" ] - }, - "NeverDuplicatedParamNextLevel": { - "type": "object", - "properties": { - "_v": { - "type": "boolean" - } - }, - "required": [ - "_v" - ] } } } diff --git a/dropshot/tests/test_openapi.rs b/dropshot/tests/test_openapi.rs index 81658056..2d2a2d70 100644 --- a/dropshot/tests/test_openapi.rs +++ b/dropshot/tests/test_openapi.rs @@ -1,4 +1,4 @@ -// Copyright 2020 Oxide Computer Company +// Copyright 2021 Oxide Computer Company use dropshot::{ endpoint, ApiDescription, HttpError, HttpResponseAccepted, @@ -26,9 +26,8 @@ async fn handler1( #[derive(Deserialize, JsonSchema)] #[allow(dead_code)] struct QueryArgs { - _tomax: String, - _xamot: Option, - _destro: Vec, + tomax: String, + xamot: Option, } #[endpoint { @@ -65,9 +64,10 @@ async fn handler3( } #[derive(JsonSchema, Deserialize)] +#[allow(dead_code)] struct BodyParam { - _x: String, - _any: serde_json::Value, + x: String, + any: serde_json::Value, } #[derive(Serialize, JsonSchema)] @@ -172,44 +172,6 @@ async fn handler9( unimplemented!(); } -/* - * Similarly, test that we do not generate duplicate type definitions when the - * same type is accepted as a query parameter to two different handler - * functions. - */ - -#[derive(Deserialize, JsonSchema)] -struct NeverDuplicatedParamTopLevel { - _b: NeverDuplicatedParamNextLevel, -} - -#[derive(Deserialize, JsonSchema)] -struct NeverDuplicatedParamNextLevel { - _v: bool, -} - -#[endpoint { - method = PUT, - path = "/dup3", -}] -async fn handler10( - _rqctx: Arc>, - _q: Query, -) -> Result, HttpError> { - unimplemented!(); -} - -#[endpoint { - method = PUT, - path = "/dup4", -}] -async fn handler11( - _rqctx: Arc>, - _q: Query, -) -> Result, HttpError> { - unimplemented!(); -} - /* * Similarly, test that we do not generate duplicate type definitions when the * same type is accepted as a typed body to two different handler functions. @@ -221,15 +183,16 @@ struct NeverDuplicatedBodyTopLevel { } #[derive(Deserialize, JsonSchema)] +#[allow(dead_code)] struct NeverDuplicatedBodyNextLevel { - _v: bool, + v: bool, } #[endpoint { method = PUT, path = "/dup5", }] -async fn handler12( +async fn handler10( _rqctx: Arc>, _b: TypedBody, ) -> Result, HttpError> { @@ -240,7 +203,7 @@ async fn handler12( method = PUT, path = "/dup6", }] -async fn handler13( +async fn handler11( _rqctx: Arc>, _b: TypedBody, ) -> Result, HttpError> { @@ -253,20 +216,22 @@ async fn handler13( */ #[derive(Deserialize, JsonSchema, Serialize)] +#[allow(dead_code)] struct NeverDuplicatedTop { - _b: NeverDuplicatedNext, + b: NeverDuplicatedNext, } #[derive(Deserialize, JsonSchema, Serialize)] +#[allow(dead_code)] struct NeverDuplicatedNext { - _v: bool, + v: bool, } #[endpoint { method = PUT, path = "/dup7", }] -async fn handler14( +async fn handler12( _rqctx: Arc>, _b: TypedBody, ) -> Result, HttpError> { @@ -277,8 +242,26 @@ async fn handler14( method = GET, path = "/dup8", }] -async fn handler15( +async fn handler13( + _rqctx: Arc>, +) -> Result, HttpError> { + unimplemented!(); +} + +#[allow(dead_code)] +#[derive(JsonSchema, Deserialize)] +struct AllPath { + path: String, +} + +#[endpoint { + method = GET, + path = "/ceci_nes_pas_une_endpoint/{path:.*}", + unpublished = true, +}] +async fn handler14( _rqctx: Arc>, + _path: Path, ) -> Result, HttpError> { unimplemented!(); } @@ -299,7 +282,6 @@ fn make_api() -> Result, String> { api.register(handler12)?; api.register(handler13)?; api.register(handler14)?; - api.register(handler15)?; Ok(api) } diff --git a/dropshot/tests/test_openapi_fuller.json b/dropshot/tests/test_openapi_fuller.json index 9307b8da..9cba67b8 100644 --- a/dropshot/tests/test_openapi_fuller.json +++ b/dropshot/tests/test_openapi_fuller.json @@ -68,51 +68,9 @@ } } }, - "/dup3": { - "put": { - "operationId": "handler10", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, - "/dup4": { - "put": { - "operationId": "handler11", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, "/dup5": { "put": { - "operationId": "handler12", + "operationId": "handler10", "requestBody": { "content": { "application/json": { @@ -132,7 +90,7 @@ }, "/dup6": { "put": { - "operationId": "handler13", + "operationId": "handler11", "requestBody": { "content": { "application/json": { @@ -152,7 +110,7 @@ }, "/dup7": { "put": { - "operationId": "handler14", + "operationId": "handler12", "requestBody": { "content": { "application/json": { @@ -172,7 +130,7 @@ }, "/dup8": { "get": { - "operationId": "handler15", + "operationId": "handler13", "responses": { "200": { "description": "successful operation", @@ -318,21 +276,7 @@ }, { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -341,7 +285,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -372,21 +316,7 @@ "parameters": [ { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -395,7 +325,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -415,25 +345,25 @@ "BodyParam": { "type": "object", "properties": { - "_any": {}, - "_x": { + "any": {}, + "x": { "type": "string" } }, "required": [ - "_any", - "_x" + "any", + "x" ] }, "NeverDuplicatedBodyNextLevel": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedBodyTopLevel": { @@ -450,12 +380,12 @@ "NeverDuplicatedNext": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedResponseNextLevel": { @@ -483,12 +413,12 @@ "NeverDuplicatedTop": { "type": "object", "properties": { - "_b": { + "b": { "$ref": "#/components/schemas/NeverDuplicatedNext" } }, "required": [ - "_b" + "b" ] }, "Response": { @@ -524,17 +454,6 @@ "required": [ "items" ] - }, - "NeverDuplicatedParamNextLevel": { - "type": "object", - "properties": { - "_v": { - "type": "boolean" - } - }, - "required": [ - "_v" - ] } } } diff --git a/dropshot/tests/test_openapi_old.json b/dropshot/tests/test_openapi_old.json index 6d3b9a23..a2d0b730 100644 --- a/dropshot/tests/test_openapi_old.json +++ b/dropshot/tests/test_openapi_old.json @@ -60,51 +60,9 @@ } } }, - "/dup3": { - "put": { - "operationId": "handler10", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, - "/dup4": { - "put": { - "operationId": "handler11", - "parameters": [ - { - "in": "query", - "name": "_b", - "required": true, - "schema": { - "$ref": "#/components/schemas/NeverDuplicatedParamNextLevel" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation" - } - } - } - }, "/dup5": { "put": { - "operationId": "handler12", + "operationId": "handler10", "requestBody": { "content": { "application/json": { @@ -124,7 +82,7 @@ }, "/dup6": { "put": { - "operationId": "handler13", + "operationId": "handler11", "requestBody": { "content": { "application/json": { @@ -144,7 +102,7 @@ }, "/dup7": { "put": { - "operationId": "handler14", + "operationId": "handler12", "requestBody": { "content": { "application/json": { @@ -164,7 +122,7 @@ }, "/dup8": { "get": { - "operationId": "handler15", + "operationId": "handler13", "responses": { "200": { "description": "successful operation", @@ -310,21 +268,7 @@ }, { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -333,7 +277,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -364,21 +308,7 @@ "parameters": [ { "in": "query", - "name": "_destro", - "required": true, - "schema": { - "type": "array", - "items": { - "type": "integer", - "format": "uint16", - "minimum": 0 - } - }, - "style": "form" - }, - { - "in": "query", - "name": "_tomax", + "name": "tomax", "required": true, "schema": { "type": "string" @@ -387,7 +317,7 @@ }, { "in": "query", - "name": "_xamot", + "name": "xamot", "schema": { "type": "string" }, @@ -407,25 +337,25 @@ "BodyParam": { "type": "object", "properties": { - "_any": {}, - "_x": { + "any": {}, + "x": { "type": "string" } }, "required": [ - "_any", - "_x" + "any", + "x" ] }, "NeverDuplicatedBodyNextLevel": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedBodyTopLevel": { @@ -442,12 +372,12 @@ "NeverDuplicatedNext": { "type": "object", "properties": { - "_v": { + "v": { "type": "boolean" } }, "required": [ - "_v" + "v" ] }, "NeverDuplicatedResponseNextLevel": { @@ -475,12 +405,12 @@ "NeverDuplicatedTop": { "type": "object", "properties": { - "_b": { + "b": { "$ref": "#/components/schemas/NeverDuplicatedNext" } }, "required": [ - "_b" + "b" ] }, "Response": { @@ -516,17 +446,6 @@ "required": [ "items" ] - }, - "NeverDuplicatedParamNextLevel": { - "type": "object", - "properties": { - "_v": { - "type": "boolean" - } - }, - "required": [ - "_v" - ] } } } diff --git a/dropshot_endpoint/src/lib.rs b/dropshot_endpoint/src/lib.rs index 6c485c0e..ec7e7248 100644 --- a/dropshot_endpoint/src/lib.rs +++ b/dropshot_endpoint/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright 2020 Oxide Computer Company +// Copyright 2021 Oxide Computer Company //! This package defines macro attributes associated with HTTP handlers. These //! attributes are used both to define an HTTP API and to generate an OpenAPI @@ -49,6 +49,7 @@ struct Metadata { method: MethodType, path: String, tags: Option>, + unpublished: Option, _dropshot_crate: Option, } @@ -80,8 +81,10 @@ fn usage(err_msg: &str, fn_name: &str) -> String { /// method = { DELETE | GET | PATCH | POST | PUT }, /// path = "/path/name/with/{named}/{variables}", /// -/// // Optional fields +/// // Optional tags for the API description /// tags = [ "all", "your", "OpenAPI", "tags" ], +/// // A value of `true` causes the API to be omitted from the API description +/// unpublished = { true | false }, /// }] /// ``` /// @@ -150,6 +153,14 @@ fn do_endpoint( }) .unwrap_or_default(); + let visible = if let Some(true) = metadata.unpublished { + quote! { + .visible(false) + } + } else { + quote! {} + }; + let dropshot = get_crate(metadata._dropshot_crate); let first_arg = ast.sig.inputs.first().ok_or_else(|| { @@ -250,6 +261,7 @@ fn do_endpoint( ) #description #(#tags)* + #visible } } }; @@ -323,12 +335,10 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { pub async fn handler_xyz(_rqctx: Arc>) {} - } - .into(), + }, ); let expected = quote! { #[allow(non_camel_case_types, missing_docs)] @@ -361,12 +371,10 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { pub async fn handler_xyz(_rqctx: std::sync::Arc>) {} - } - .into(), + }, ); let expected = quote! { #[allow(non_camel_case_types, missing_docs)] @@ -399,12 +407,10 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { async fn handler_xyz(_rqctx: Arc>, q: Query) {} - } - .into(), + }, ); let query = quote! { Query @@ -449,12 +455,10 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { pub(crate) async fn handler_xyz(_rqctx: Arc>, q: Query) {} - } - .into(), + }, ); let query = quote! { Query @@ -500,12 +504,10 @@ mod tests { method = GET, path = "/a/b/c", tags = ["stuff", "things"], - } - .into(), + }, quote! { async fn handler_xyz(_rqctx: Arc>) {} - } - .into(), + }, ); let expected = quote! { #[allow(non_camel_case_types, missing_docs)] @@ -538,13 +540,11 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { /** handle "xyz" requests */ async fn handler_xyz(_rqctx: Arc>) {} - } - .into(), + }, ); let expected = quote! { #[allow(non_camel_case_types, missing_docs)] @@ -577,12 +577,10 @@ mod tests { quote! { method = GET, path = "/a/b/c" - } - .into(), + }, quote! { const POTATO = "potato"; - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap()); @@ -595,12 +593,10 @@ mod tests { quote! { method = GET, path = /a/b/c - } - .into(), + }, quote! { const POTATO = "potato"; - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap()); @@ -613,12 +609,10 @@ mod tests { quote! { methud = GET, path = "/a/b/c" - } - .into(), + }, quote! { const POTATO = "potato"; - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap()); @@ -631,12 +625,10 @@ mod tests { quote! { method = GET, path = "/a/b/c", - } - .into(), + }, quote! { fn handler_xyz(_rqctx: Arc) {} - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap()); @@ -649,12 +641,10 @@ mod tests { quote! { method = GET, path = "/a/b/c", - } - .into(), + }, quote! { async fn handler_xyz(&self) {} - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap()); @@ -670,12 +660,10 @@ mod tests { quote! { method = GET, path = "/a/b/c", - } - .into(), + }, quote! { async fn handler_xyz() {} - } - .into(), + }, ); let msg = format!("{}", ret.err().unwrap());