From d19f3e0552d241862a6b99ecd0f676e3d284a770 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Wed, 2 Oct 2024 16:58:15 -0700 Subject: [PATCH] self-review nits --- dropshot/src/lib.rs | 2 +- dropshot/tests/test_versions.rs | 225 +++++++++++++++++--------------- 2 files changed, 118 insertions(+), 109 deletions(-) diff --git a/dropshot/src/lib.rs b/dropshot/src/lib.rs index cd9a68a54..179ae5a2c 100644 --- a/dropshot/src/lib.rs +++ b/dropshot/src/lib.rs @@ -318,7 +318,7 @@ //! OpenAPI spec output. //! //! The versions field controls which versions of the API this endpoint appears -//! in. See [`API Versioning`] for more on this. +//! in. See "API Versioning" for more on this. //! //! //! ### Function parameters diff --git a/dropshot/tests/test_versions.rs b/dropshot/tests/test_versions.rs index a3b0e5276..18af6d019 100644 --- a/dropshot/tests/test_versions.rs +++ b/dropshot/tests/test_versions.rs @@ -47,6 +47,77 @@ fn api_to_openapi_string( String::from_utf8(contents.get_ref().to_vec()).unwrap() } +// This is just here so that we can tell that types are included in the spec iff +// they are referenced by endpoints in that version of the spec. +#[derive(Deserialize, JsonSchema, Serialize)] +struct EarlyReturn { + real_message: String, +} + +#[endpoint { + method = GET, + path = "/demo", + versions = .."1.0.0", +}] +async fn handler1( + _rqctx: RequestContext<()>, +) -> Result, HttpError> { + Ok(HttpResponseOk(EarlyReturn { real_message: HANDLER1_MSG.to_string() })) +} + +#[endpoint { + method = GET, + path = "/demo", + versions = "1.1.0".."1.3.0", +}] +async fn handler2( + _rqctx: RequestContext<()>, +) -> Result, HttpError> { + Ok(HttpResponseOk(HANDLER2_MSG)) +} + +#[endpoint { + method = GET, + path = "/demo", + versions = "1.4.0".., +}] +async fn handler3( + _rqctx: RequestContext<()>, +) -> Result, HttpError> { + Ok(HttpResponseOk(HANDLER3_MSG)) +} + +#[endpoint { + method = PUT, + path = "/demo", + versions = .. +}] +async fn handler4( + _rqctx: RequestContext<()>, +) -> Result, HttpError> { + Ok(HttpResponseOk(HANDLER4_MSG)) +} + +#[derive(Debug)] +struct TestVersionPolicy(ClientSpecifiesVersionInHeader); +impl dropshot::DynamicVersionPolicy for TestVersionPolicy { + fn request_extract_version( + &self, + request: &http::Request, + log: &slog::Logger, + ) -> Result { + let v = self.0.request_extract_version(request, log)?; + if v.major == 4 && v.minor == 3 && v.patch == 2 { + Err(HttpError::for_bad_request(None, String::from(FORBIDDEN_MSG))) + } else { + Ok(v) + } + } +} + +/// Define an API with different versions and run through an exhaustive battery +/// of tests showing that we use the correct handler for each incoming request +/// based on the version requested. #[tokio::test] async fn test_versions() { let logctx = common::create_log_context("test_versions"); @@ -65,10 +136,16 @@ async fn test_versions() { #[derive(Debug)] struct TestCase<'a> { + /// HTTP method for the request method: Method, + /// HTTP path for the request path: &'a str, + /// Value to pass for our requested version + /// (string supports providing bad (non-semver) input) header: Option<&'a str>, + /// expected HTTP response status code expected_status: StatusCode, + /// expected HTTP response body contents body_contents: String, } @@ -274,54 +351,6 @@ fn test_versions_openapi() { } } -// The contents of this module logically belongs inside -// test_versions_openapi_same_names(). It can't go there due to -// oxidecomputer/dropshot#1128. -mod trait_based { - use super::*; - - #[derive(JsonSchema, Serialize)] - #[schemars(rename = "MyReturn")] - pub struct MyReturnV1 { - #[allow(dead_code)] - q: String, - } - - #[derive(JsonSchema, Serialize)] - #[schemars(rename = "MyReturn")] - pub struct MyReturnV2 { - #[allow(dead_code)] - r: String, - } - - #[dropshot::api_description] - // This `allow(dead_code)` works around oxidecomputer/dropshot#1129. - #[allow(dead_code)] - pub trait MyApi { - type Context; - - #[endpoint { - method = GET, - path = "/demo", - versions = .."1.0.0", - operation_id = "the_operation", - }] - async fn the_operation_v1( - _rqctx: RequestContext, - ) -> Result, HttpError>; - - #[endpoint { - method = GET, - path = "/demo", - versions = "2.0.0".., - operation_id = "the_operation" - }] - async fn the_operation_v2( - _rqctx: RequestContext, - ) -> Result, HttpError>; - } -} - /// Test three different ways to define the same operation in two versions /// (using different handlers). These should all produce the same pair of /// specs. @@ -458,70 +487,50 @@ fn test_versions_openapi_same_names() { assert_eq!(func_mods_v2, traits_v2); } -// This is just here so that we can tell that types are included in the spec iff -// they are referenced by endpoints in that version of the spec. -#[derive(Deserialize, JsonSchema, Serialize)] -struct EarlyReturn { - real_message: String, -} +// The contents of this module logically belongs inside +// test_versions_openapi_same_names(). It can't go there due to +// oxidecomputer/dropshot#1128. +mod trait_based { + use super::*; -#[endpoint { - method = GET, - path = "/demo", - versions = .."1.0.0", -}] -async fn handler1( - _rqctx: RequestContext<()>, -) -> Result, HttpError> { - Ok(HttpResponseOk(EarlyReturn { real_message: HANDLER1_MSG.to_string() })) -} + #[derive(JsonSchema, Serialize)] + #[schemars(rename = "MyReturn")] + pub struct MyReturnV1 { + #[allow(dead_code)] + q: String, + } -#[endpoint { - method = GET, - path = "/demo", - versions = "1.1.0".."1.3.0", -}] -async fn handler2( - _rqctx: RequestContext<()>, -) -> Result, HttpError> { - Ok(HttpResponseOk(HANDLER2_MSG)) -} + #[derive(JsonSchema, Serialize)] + #[schemars(rename = "MyReturn")] + pub struct MyReturnV2 { + #[allow(dead_code)] + r: String, + } -#[endpoint { - method = GET, - path = "/demo", - versions = "1.4.0".., -}] -async fn handler3( - _rqctx: RequestContext<()>, -) -> Result, HttpError> { - Ok(HttpResponseOk(HANDLER3_MSG)) -} + #[dropshot::api_description] + // This `allow(dead_code)` works around oxidecomputer/dropshot#1129. + #[allow(dead_code)] + pub trait MyApi { + type Context; -#[endpoint { - method = PUT, - path = "/demo", - versions = .. -}] -async fn handler4( - _rqctx: RequestContext<()>, -) -> Result, HttpError> { - Ok(HttpResponseOk(HANDLER4_MSG)) -} + #[endpoint { + method = GET, + path = "/demo", + versions = .."1.0.0", + operation_id = "the_operation", + }] + async fn the_operation_v1( + _rqctx: RequestContext, + ) -> Result, HttpError>; -#[derive(Debug)] -struct TestVersionPolicy(ClientSpecifiesVersionInHeader); -impl dropshot::DynamicVersionPolicy for TestVersionPolicy { - fn request_extract_version( - &self, - request: &http::Request, - log: &slog::Logger, - ) -> Result { - let v = self.0.request_extract_version(request, log)?; - if v.major == 4 && v.minor == 3 && v.patch == 2 { - Err(HttpError::for_bad_request(None, String::from(FORBIDDEN_MSG))) - } else { - Ok(v) - } + #[endpoint { + method = GET, + path = "/demo", + versions = "2.0.0".., + operation_id = "the_operation" + }] + async fn the_operation_v2( + _rqctx: RequestContext, + ) -> Result, HttpError>; } }