OpenAPI: update path separator (#4199) (#4249)

Author: swallez

compiler-rs/Cargo.lock

@@ -7,4 +7,7 @@ rules:
   oas3-api-servers: off      # OpenAPI "servers" must be present and non-empty array.
   info-contact: off          # Info object must have "contact" object.
   info-description: off      # Info "description" must be present and non-empty string.
+  operation-description: off # Info "description" must be present and non-empty string.
   operation-tag-defined: off # Operation tags must be defined in global tags.
+
+#  oas3-valid-media-example: off

compiler-rs/clients_schema_to_openapi/.spectral.yaml

@@ -11,7 +11,7 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
-    "@stoplight/spectral-cli": "^6.10.1",
+    "@stoplight/spectral-cli": "^6.14.3",
     "json-diff": "^1.0.6"
   }
 }

compiler-rs/clients_schema_to_openapi/package-lock.json

@@ -21,8 +21,12 @@ use openapiv3::{Components, Parameter, ReferenceOr, RequestBody, Response, Schem
 use crate::utils::SchemaName;
 
 // Separator used to combine parts of a component path.
-// See https://github.com/elastic/elasticsearch-specification/issues/4183
-pub const SEPARATOR: char = ':';
+// OpenAPI says `$ref` must comply with RFC 3968 (escaping reserved chars),
+// but also restricts the keys in `components` to match `^[a-zA-Z0-9\.\-_]+$`.
+//
+// See https://spec.openapis.org/oas/v3.1.1.html#reference-object
+// and https://spec.openapis.org/oas/v3.1.1.html#fixed-fields-5
+pub const SEPARATOR: char = '-';
 
 pub struct TypesAndComponents<'a> {
     pub model: &'a clients_schema::IndexedModel,

compiler-rs/clients_schema_to_openapi/package.json

@@ -28,7 +28,6 @@ use openapiv3::{
     RequestBody, Response, Responses, StatusCode, Example
 };
 use clients_schema::SchemaExample;
-use serde_json::json;
 
 use crate::components::TypesAndComponents;
 
@@ -40,12 +39,12 @@ pub fn add_endpoint(
     out: &mut Paths,
 ) -> anyhow::Result<()> {
     if endpoint.request.is_none() {
-        tracing::warn!("Endpoint {} is missing a request -- ignored", &endpoint.name);
+        // tracing::warn!("Endpoint {} is missing a request -- ignored", &endpoint.name);
         return Ok(());
     }
 
     if endpoint.response.is_none() {
-        tracing::warn!("Endpoint {} is missing a response -- ignored", &endpoint.name);
+        // tracing::warn!("Endpoint {} is missing a response -- ignored", &endpoint.name);
         return Ok(());
     }
 
@@ -122,28 +121,48 @@ pub fn add_endpoint(
     // This function converts the IndexMap<String, SchemaExample> examples of
     // schema.json to IndexMap<String, ReferenceOr<Example>> which is the format
     // that OpenAPI expects.
-    fn get_openapi_examples(schema_examples: IndexMap<String, SchemaExample>) -> IndexMap<String, ReferenceOr<Example>> {
+    fn get_openapi_examples(schema_examples: &IndexMap<String, SchemaExample>) -> IndexMap<String, ReferenceOr<Example>> {
         let mut openapi_examples = indexmap! {};
         for (name, schema_example) in schema_examples {
+            let example = match &schema_example.value {
+                None => None,
+                // Examples should be objects - https://spec.openapis.org/oas/v3.1.1.html#example-object
+                // Disabled for now, as some examples use multi-line json as in the Kibana console.
+                Some(text) => Some(serde_json::Value::String(text.clone())),
+                // Some(text) => {
+                //     match serde_json::from_str::<serde_json::Value>(&text) {
+                //         Ok(json) => {
+                //             Some(json)
+                //         }
+                //         // Cannot parse json: assume it's text (e.g. cat requests)
+                //         // but should be validated by looking at the media-type
+                //         Err(err) => {
+                //             tracing::warn!("Cannot parse example: {}\n{}", err, text);
+                //             Some(serde_json::Value::String(text.clone()))
+                //         }
+                //     }
+                // }
+            };
+
             let openapi_example = Example {
-                value: Some(json!(schema_example.value)),
+                value: example,
                 description: schema_example.description.clone(),
                 summary: schema_example.summary.clone(),
                 external_value: None,
                 extensions: Default::default(),
             };
             openapi_examples.insert(name.clone(), ReferenceOr::Item(openapi_example));
         }
-        return openapi_examples;
+        openapi_examples
     }
 
-
-    let mut request_examples: IndexMap<String, ReferenceOr<Example>> = indexmap! {};
     // If this endpoint request has examples in schema.json, convert them to the
     // OpenAPI format and add them to the endpoint request in the OpenAPI document.
-    if request.examples.is_some() {
-        request_examples = get_openapi_examples(request.examples.as_ref().unwrap().clone());
-    }
+    let request_examples = if let Some(examples) = &request.examples {
+        get_openapi_examples(examples)
+    } else {
+        IndexMap::new()
+    };
 
     let request_body = tac.convert_request(request)?.map(|schema| {
         let media = MediaType {
@@ -176,12 +195,13 @@ pub fn add_endpoint(
     // FIXME: buggy for responses with no body
     // TODO: handle binary responses
     let response_def = tac.model.get_response(endpoint.response.as_ref().unwrap())?;
-    let mut response_examples: IndexMap<String, ReferenceOr<Example>> = indexmap! {};
     // If this endpoint response has examples in schema.json, convert them to the
     // OpenAPI format and add them to the endpoint response in the OpenAPI document.
-    if response_def.examples.is_some() {
-        response_examples = get_openapi_examples(response_def.examples.as_ref().unwrap().clone());
-    }
+    let response_examples = if let Some(examples) = &response_def.examples {
+         get_openapi_examples(examples)
+    } else {
+        IndexMap::new()
+    };
     let response = Response {
         description: "".to_string(),
         headers: Default::default(),

compiler-rs/clients_schema_to_openapi/src/components.rs

@@ -43,13 +43,15 @@ pub trait SchemaName {
 }
 
 impl SchemaName for TypeName {
+    // Use '.' as the separator: names and paths must be RFC 3986 compliant,
+    // and ':' output by `TypeName.toString()` is a reserved character.
     fn schema_name(&self) -> String {
-        format!("{}", self)
+        format!("{}.{}", self.namespace, self.name)
     }
 
     fn schema_ref(&self) -> ReferenceOr<Schema> {
         ReferenceOr::Reference {
-            reference: format!("#/components/schemas/{}", self),
+            reference: format!("#/components/schemas/{}.{}", self.namespace, self.name),
         }
     }
 }

compiler-rs/clients_schema_to_openapi/src/paths.rs

@@ -19,6 +19,7 @@ serde_json = { workspace = true }
 anyhow = { workspace = true }
 
 console_error_panic_hook = { workspace = true, optional = true }
+tracing-wasm = "0.2.1"
 
 [dev-dependencies]
 wasm-bindgen-test = { workspace = true }

compiler-rs/clients_schema_to_openapi/src/utils.rs

@@ -149,11 +149,68 @@ module.exports.convert_schema_to_openapi = function(json, flavor) {
     }
 };
 
+function handleError(f, args) {
+    try {
+        return f.apply(this, args);
+    } catch (e) {
+        wasm.__wbindgen_exn_store(addHeapObject(e));
+    }
+}
+
 module.exports.__wbindgen_string_new = function(arg0, arg1) {
     const ret = getStringFromWasm0(arg0, arg1);
     return addHeapObject(ret);
 };
 
+module.exports.__wbg_log_c9486ca5d8e2cbe8 = function(arg0, arg1) {
+    let deferred0_0;
+    let deferred0_1;
+    try {
+        deferred0_0 = arg0;
+        deferred0_1 = arg1;
+        console.log(getStringFromWasm0(arg0, arg1));
+    } finally {
+        wasm.__wbindgen_free(deferred0_0, deferred0_1, 1);
+    }
+};
+
+module.exports.__wbg_log_aba5996d9bde071f = function(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) {
+    let deferred0_0;
+    let deferred0_1;
+    try {
+        deferred0_0 = arg0;
+        deferred0_1 = arg1;
+        console.log(getStringFromWasm0(arg0, arg1), getStringFromWasm0(arg2, arg3), getStringFromWasm0(arg4, arg5), getStringFromWasm0(arg6, arg7));
+    } finally {
+        wasm.__wbindgen_free(deferred0_0, deferred0_1, 1);
+    }
+};
+
+module.exports.__wbg_mark_40e050a77cc39fea = function(arg0, arg1) {
+    performance.mark(getStringFromWasm0(arg0, arg1));
+};
+
+module.exports.__wbindgen_object_drop_ref = function(arg0) {
+    takeObject(arg0);
+};
+
+module.exports.__wbg_measure_aa7a73f17813f708 = function() { return handleError(function (arg0, arg1, arg2, arg3) {
+    let deferred0_0;
+    let deferred0_1;
+    let deferred1_0;
+    let deferred1_1;
+    try {
+        deferred0_0 = arg0;
+        deferred0_1 = arg1;
+        deferred1_0 = arg2;
+        deferred1_1 = arg3;
+        performance.measure(getStringFromWasm0(arg0, arg1), getStringFromWasm0(arg2, arg3));
+    } finally {
+        wasm.__wbindgen_free(deferred0_0, deferred0_1, 1);
+        wasm.__wbindgen_free(deferred1_0, deferred1_1, 1);
+    }
+}, arguments) };
+
 module.exports.__wbg_new_abda76e883ba8a5f = function() {
     const ret = new Error();
     return addHeapObject(ret);
@@ -179,10 +236,6 @@ module.exports.__wbg_error_f851667af71bcfc6 = function(arg0, arg1) {
     }
 };
 
-module.exports.__wbindgen_object_drop_ref = function(arg0) {
-    takeObject(arg0);
-};
-
 const path = require('path').join(__dirname, 'compiler_wasm_lib_bg.wasm');
 const bytes = require('fs').readFileSync(path);