Fix conflation of () which serializes to the JSON value null and a …

…return that produces no content (#198)

* Fix conflation of () which serializes to the JSON value `null` and a
return that actually produces no content. For the latter, we introduce,
a new type `Empty` which has a custom JsonSchema implementation that
evaluates to the empty schema (i.e. nothing can match it).

CHANGELOG.adoc

@@ -20,6 +20,10 @@ https://github.com/oxidecomputer/dropshot/compare/v0.6.0\...HEAD[Full list of co
 * https://github.com/oxidecomputer/dropshot/pull/197[#197] Endpoints using wildcard path params (i.e. those using the `/foo/{bar:.*}` syntax) previously could be included in OpenAPI output albeit in a form that was invalid. Specifying a wildcard path **without** also specifying `unpublished = true` is now a **compile-time error**.
 * https://github.com/oxidecomputer/dropshot/pull/204[#204] Rust 1.58.0-nightly introduced a new feature `asm_sym` which the `usdt` crate requires on macOS. As of this change 1.58.0-nightly or later is required to build with the `usdt-probes` feature on macOS.
 
+=== Other notable changes
+
+* https://github.com/oxidecomputer/dropshot/pull/198[#198] Responses that used `()` (the unit type) as their `Body` type parameter previously (and inaccurately) were represented in OpenAPI as an empty `responseBody`. They are now more accurately represented as a body whose value is `null` (4 bytes). We encourage those use cases to instead use either `HttpResponseUpdatedNoContent` or `HttpResponseDeleted` both of which have empty response bodies. If there are other situations where you would like a response type with no body, please file an issue.
+
 == 0.6.0 (released 2021-11-18)
 
 https://github.com/oxidecomputer/dropshot/compare/v0.5.1\...v0.6.0[Full list of commits]

dropshot/src/api_description.rs

@@ -55,13 +55,14 @@ impl<'a, Context: ServerContext> ApiEndpoint<Context> {
         ResponseType: HttpResponse + Send + Sync + 'static,
     {
         let func_parameters = FuncParams::metadata();
+        let response = ResponseType::metadata();
         ApiEndpoint {
             operation_id,
             handler: HttpRouteHandler::new(handler),
             method,
             path: path.to_string(),
             parameters: func_parameters.parameters,
-            response: ResponseType::metadata(),
+            response,
             description: None,
             tags: vec![],
             paginated: func_parameters.paginated,
@@ -600,7 +601,8 @@ impl<Context: ServerContext> ApiDescription<Context> {
                     }
                 };
                 let mut content = indexmap::IndexMap::new();
-                if !is_null(&js) {
+                if !is_empty(&js) {
+                    println!("not empty");
                     content.insert(
                         CONTENT_TYPE_JSON.to_string(),
                         openapiv3::MediaType {
@@ -674,18 +676,60 @@ impl<Context: ServerContext> ApiDescription<Context> {
 }
 
 /**
- * Returns true iff the schema represents the null type i.e. the rust type `()`.
+ * Returns true iff the schema represents the void schema that matches no data.
  */
-fn is_null(schema: &schemars::schema::Schema) -> bool {
+fn is_empty(schema: &schemars::schema::Schema) -> bool {
+    if let schemars::schema::Schema::Bool(false) = schema {
+        return true;
+    }
     if let schemars::schema::Schema::Object(schemars::schema::SchemaObject {
-        instance_type: Some(schemars::schema::SingleOrVec::Single(it)),
-        ..
+        metadata: _,
+        instance_type: None,
+        format: None,
+        enum_values: None,
+        const_value: None,
+        subschemas: Some(subschemas),
+        number: None,
+        string: None,
+        array: None,
+        object: None,
+        reference: None,
+        extensions: _,
     }) = schema
     {
-        if let schemars::schema::InstanceType::Null = it.as_ref() {
-            return true;
+        if let schemars::schema::SubschemaValidation {
+            all_of: None,
+            any_of: None,
+            one_of: None,
+            not: Some(not),
+            if_schema: None,
+            then_schema: None,
+            else_schema: None,
+        } = subschemas.as_ref()
+        {
+            match not.as_ref() {
+                schemars::schema::Schema::Bool(true) => return true,
+                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: None,
+                        extensions: _,
+                    },
+                ) => return true,
+                _ => {}
+            }
         }
     }
+
     false
 }
 
@@ -744,7 +788,14 @@ fn j2oas_schema_object(
     };
 
     let kind = match (ty, &obj.subschemas) {
-        (Some(schemars::schema::InstanceType::Null), None) => todo!(),
+        (Some(schemars::schema::InstanceType::Null), None) => {
+            openapiv3::SchemaKind::Type(openapiv3::Type::String(
+                openapiv3::StringType {
+                    enumeration: vec![None],
+                    ..Default::default()
+                },
+            ))
+        }
         (Some(schemars::schema::InstanceType::Boolean), None) => {
             openapiv3::SchemaKind::Type(openapiv3::Type::Boolean {})
         }
@@ -801,25 +852,33 @@ fn j2oas_schema_object(
 fn j2oas_subschemas(
     subschemas: &schemars::schema::SubschemaValidation,
 ) -> openapiv3::SchemaKind {
-    match (&subschemas.all_of, &subschemas.any_of, &subschemas.one_of) {
-        (Some(all_of), None, None) => openapiv3::SchemaKind::AllOf {
+    match (
+        &subschemas.all_of,
+        &subschemas.any_of,
+        &subschemas.one_of,
+        &subschemas.not,
+    ) {
+        (Some(all_of), None, None, None) => openapiv3::SchemaKind::AllOf {
             all_of: all_of
                 .iter()
                 .map(|schema| j2oas_schema(None, schema))
                 .collect::<Vec<_>>(),
         },
-        (None, Some(any_of), None) => openapiv3::SchemaKind::AnyOf {
+        (None, Some(any_of), None, None) => openapiv3::SchemaKind::AnyOf {
             any_of: any_of
                 .iter()
                 .map(|schema| j2oas_schema(None, schema))
                 .collect::<Vec<_>>(),
         },
-        (None, None, Some(one_of)) => openapiv3::SchemaKind::OneOf {
+        (None, None, Some(one_of), None) => openapiv3::SchemaKind::OneOf {
             one_of: one_of
                 .iter()
                 .map(|schema| j2oas_schema(None, schema))
                 .collect::<Vec<_>>(),
         },
+        (None, None, None, Some(not)) => openapiv3::SchemaKind::Not {
+            not: Box::new(j2oas_schema(None, not)),
+        },
         _ => panic!("invalid subschema {:#?}", subschemas),
     }
 }

dropshot/src/from_map.rs

@@ -475,6 +475,8 @@ mod test {
 
     #[test]
     fn test_deep() {
+        #![allow(dead_code)]
+
         #[derive(Deserialize, Debug)]
         struct A {
             b: B,
@@ -494,6 +496,8 @@ mod test {
     }
     #[test]
     fn test_missing_data1() {
+        #![allow(dead_code)]
+
         #[derive(Deserialize, Debug)]
         struct A {
             aaa: String,
@@ -513,6 +517,8 @@ mod test {
     }
     #[test]
     fn test_missing_data2() {
+        #![allow(dead_code)]
+
         #[derive(Deserialize, Debug)]
         struct A {
             aaa: String,
@@ -576,6 +582,8 @@ mod test {
     }
     #[test]
     fn wherefore_art_thou_a_valid_sequence_when_in_fact_you_are_a_lone_value() {
+        #![allow(dead_code)]
+
         #[derive(Deserialize, Debug)]
         struct A {
             b: Vec<String>,

dropshot/src/handler.rs

@@ -1231,14 +1231,47 @@ impl<T: JsonSchema + Serialize + Send + Sync + 'static> From<HttpResponseOk<T>>
     }
 }
 
+/**
+ * An "empty" type used to represent responses that have no associated data
+ * payload. This isn't intended for general use, but must be pub since it's
+ * used as the Body type for certain responses.
+ */
+#[doc(hidden)]
+pub struct Empty;
+
+impl JsonSchema for Empty {
+    fn schema_name() -> String {
+        "Empty".to_string()
+    }
+
+    fn json_schema(
+        _: &mut schemars::gen::SchemaGenerator,
+    ) -> schemars::schema::Schema {
+        schemars::schema::Schema::Bool(false)
+    }
+
+    fn is_referenceable() -> bool {
+        false
+    }
+}
+
+impl Serialize for Empty {
+    fn serialize<S>(&self, _: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        panic!("Empty::serialize() should never be called");
+    }
+}
+
 /**
  * `HttpResponseDeleted` represents an HTTP 204 "No Content" response, intended
  * for use when an API operation has successfully deleted an object.
  */
 pub struct HttpResponseDeleted();
 
 impl HttpTypedResponse for HttpResponseDeleted {
-    type Body = ();
+    type Body = Empty;
     const STATUS_CODE: StatusCode = StatusCode::NO_CONTENT;
     const DESCRIPTION: &'static str = "successful deletion";
 }
@@ -1256,8 +1289,9 @@ impl From<HttpResponseDeleted> for HttpHandlerResult {
  * has nothing to return.
  */
 pub struct HttpResponseUpdatedNoContent();
+
 impl HttpTypedResponse for HttpResponseUpdatedNoContent {
-    type Body = ();
+    type Body = Empty;
     const STATUS_CODE: StatusCode = StatusCode::NO_CONTENT;
     const DESCRIPTION: &'static str = "resource updated";
 }
@@ -1271,6 +1305,8 @@ impl From<HttpResponseUpdatedNoContent> for HttpHandlerResult {
 
 #[cfg(test)]
 mod test {
+    use crate::handler::HttpHandlerResult;
+    use crate::HttpResponseOk;
     use crate::{
         api_description::ApiEndpointParameterMetadata, ApiEndpointParameter,
         ApiEndpointParameterLocation, PaginationParams,
@@ -1279,6 +1315,7 @@ mod test {
     use serde::{Deserialize, Serialize};
 
     use super::get_metadata;
+    use super::Empty;
     use super::ExtractorMetadata;
 
     #[derive(Deserialize, Serialize, JsonSchema)]
@@ -1385,4 +1422,11 @@ mod test {
 
         compare(params, true, expected);
     }
+
+    #[test]
+    #[should_panic]
+    fn test_empty_serialized() {
+        let response = HttpResponseOk::<Empty>(Empty);
+        let _ = HttpHandlerResult::from(response);
+    }
 }

dropshot/src/pagination.rs

@@ -644,6 +644,7 @@ mod test {
         }
 
         #[derive(Debug, Deserialize)]
+        #[allow(dead_code)]
         struct MyOptionalScanParams {
             the_field: Option<String>,
             only_good: Option<String>,
@@ -799,6 +800,7 @@ mod test {
          * precedence (and it's not clear how else this could work).
          */
         #[derive(Debug, Deserialize)]
+        #[allow(dead_code)]
         struct SketchyScanParams {
             page_token: String,
         }