Merge branch 'main' into charlotte-query-api-keys-examples

Author: charlotte-hoblik

.github/validate-pr/index.js

@@ -27,6 +27,7 @@ import * as core from '@actions/core'
 import { copyFile } from 'fs/promises'
 import * as github from '@actions/github'
 import specification from '../../output/schema/schema.json' with { type: 'json' }
+import baselineValidation from '../../../clients-flight-recorder/recordings/types-validation/types-validation.json' with { type: 'json' }
 import { run as getReport } from '../../../clients-flight-recorder/scripts/types-validator/index.js'
 import {
   getNamespace,
@@ -81,7 +82,7 @@ async function run() {
   const specFiles = files.filter(
     (file) => file.includes('specification') && !file.includes('compiler/test')
   )
-  const table = []
+  const reports = new Map()
 
   cd(tsValidationPath)
 
@@ -109,37 +110,47 @@ async function run() {
           ci: false,
           verbose: false
         })
-        table.push(buildTableLine(api, report))
+        const [namespace, _method] = api.split('.')
+        // Asked to validate a specific API, so we only store that one
+        reports.set(api, report.get(namespace)[0])
       }
     } else {
+      const api = getApi(file)
       const report = await getReport({
-        api: getApi(file),
+        api,
         'generate-report': false,
         request: true,
         response: true,
         ci: false,
         verbose: false
       })
-      table.push(buildTableLine(getApi(file), report))
+
+      const [namespace, _method] = api.split('.')
+      // Asked to validate a specific API, so we only store that one
+      reports.set(api, report.get(namespace)[0])
     }
   }
 
   cd(path.join(__dirname, '..', '..'))
 
-  table.sort((a, b) => {
-    if (a < b) return -1
-    if (a > b) return 1
-    return 0
-  })
+  // Compare current reports with baseline and find changes
+  const changedApis = []
+  for (const [apiName, report] of reports) {
+    const baselineReport = findBaselineReport(apiName, baselineValidation)
+    if (baselineReport && hasChanges(baselineReport, report, apiName)) {
+      changedApis.push({ api: apiName, baseline: baselineReport, current: report })
+    }
+  }
+  changedApis.sort((a, b) => a.api.localeCompare(b.api))
 
-  if (table.length > 0) {
-    let comment = `Following you can find the validation results for the API${table.length === 1 ? '' : 's'} you have changed.\n\n`
+  if (changedApis.length > 0) {
+    let comment = `Following you can find the validation changes for the API${changedApis.length === 1 ? '' : 's'} you have modified.\n\n`
     comment += '| API | Status | Request | Response |\n'
     comment += '| --- | --- | --- | --- |\n'
-    for (const line of [...new Set(table)]) {
-      comment += line
+    for (const change of changedApis) {
+      comment += buildDiffTableLine(change)
     }
-    comment += `\nYou can validate ${table.length === 1 ? 'this' : 'these'} API${table.length === 1 ? '' : 's'} yourself by using the ${tick}make validate${tick} target.\n`
+    comment += `\nYou can validate ${changedApis.length === 1 ? 'this' : 'these'} API${changedApis.length === 1 ? '' : 's'} yourself by using the ${tick}make validate${tick} target.\n`
 
     core.setOutput('has_results', 'true')
     core.setOutput('comment_body', comment)
@@ -154,39 +165,70 @@ function getApi (file) {
   return file.split('/').slice(1, 3).filter(s => !privateNames.includes(s)).filter(Boolean).join('.')
 }
 
-function buildTableLine (api, report) {
-  const apiReport = report.get(getNamespace(api)).find(r => r.api === getName(api))
-  return `| ${tick}${api}${tick} | ${generateStatus(apiReport)} | ${generateRequest(apiReport)} | ${generateResponse(apiReport)} |\n`
+
+function findBaselineReport(apiName, baselineValidation) {
+  const [namespace, method] = apiName.split('.')
+
+  if (!baselineValidation.namespaces[namespace]) {
+    return null
+  }
+
+  return baselineValidation.namespaces[namespace].apis.find(api => api.api === method)
 }
 
-function generateStatus (report) {
-  if (!report.diagnostics.hasRequestType || !report.diagnostics.hasResponseType) {
+function hasChanges(baselineReport, report) {
+  if (!report) return false
+
+  return baselineReport.status !== report.status ||
+         baselineReport.passingRequest !== report.passingRequest ||
+         baselineReport.passingResponse !== report.passingResponse
+}
+
+function buildDiffTableLine(change) {
+  const { api, baseline, current } = change
+
+  const status = generateStatus(current.status)
+  const request = generateRequest(current)
+  const response = generateResponse(current)
+
+  const baselineStatus = generateStatus(baseline.status)
+  const baselineRequest = generateRequest(baseline)
+  const baselineResponse = generateResponse(baseline)
+
+  const statusDiff = status !== baselineStatus ? `${baselineStatus} → ${status}` : status
+  const requestDiff = request !== baselineRequest ? `${baselineRequest} → ${request}` : request
+  const responseDiff = response !== baselineResponse ? `${baselineResponse} → ${response}` : response
+
+  return `| ${tick}${api}${tick} | ${statusDiff} | ${requestDiff} | ${responseDiff} |\n`
+}
+
+
+function generateStatus (status) {
+  if (status === 'missing_types' || status === 'missing_request_type' || status === 'missing_response_type') {
     return ':orange_circle:'
   }
-  if (report.totalRequest <= 0 || report.totalResponse <= 0) {
+  if (status === 'missing_test') {
     return ':white_circle:'
   }
-  if (report.diagnostics.request.length === 0 && report.diagnostics.response.length === 0) {
+  if (status === 'passing') {
     return ':green_circle:'
   }
   return ':red_circle:'
 }
 
 function generateRequest (r) {
-  if (r.totalRequest === -1) return 'Missing recording'
-  if (!r.diagnostics.hasRequestType) return 'Missing type'
-  if (r.totalRequest === 0) return 'Missing test'
+  if (r.status === 'missing_test') return 'Missing test'
+  if (r.status === 'missing_types' || r.status == 'missing_request_type') return 'Missing type'
   return `${r.passingRequest}/${r.totalRequest}`
 }
 
 function generateResponse (r) {
-  if (r.totalResponse === -1) return 'Missing recording'
-  if (!r.diagnostics.hasResponseType) return 'Missing type'
-  if (r.totalResponse === 0) return 'Missing test'
+  if (r.status === 'missing_test') return 'Missing test'
+  if (r.status === 'missing_types' || r.status == 'missing_response_type') return 'Missing type'
   return `${r.passingResponse}/${r.totalResponse}`
 }
 
 run().catch((err) => {
   core.error(err)
   process.exit(1)
-})
+})

compiler-rs/clients_schema/src/lib.rs

@@ -58,6 +58,7 @@ pub trait Documented {
 pub trait ExternalDocument {
     fn ext_doc_id(&self) -> Option<&str>;
     fn ext_doc_url(&self) -> Option<&str>;
+    fn ext_previous_version_doc_url(&self) -> Option<&str>;
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -322,6 +323,9 @@ pub struct Property {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ext_doc_url: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_previous_version_doc_url: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ext_doc_id: Option<String>,
 
@@ -371,6 +375,10 @@ impl ExternalDocument for Property {
         self.ext_doc_url.as_deref()
     }
 
+    fn ext_previous_version_doc_url(&self) -> Option<&str> {
+        self.ext_previous_version_doc_url.as_deref()
+    }
+
     fn ext_doc_id(&self) -> Option<&str> {
         self.ext_doc_id.as_deref()
     }
@@ -528,6 +536,9 @@ pub struct BaseType {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ext_doc_url: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_previous_version_doc_url: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ext_doc_id: Option<String>,
 
@@ -568,6 +579,7 @@ impl BaseType {
             spec_location: None,
             ext_doc_id: None,
             ext_doc_url: None,
+            ext_previous_version_doc_url: None,
         }
     }
 }
@@ -591,6 +603,10 @@ impl ExternalDocument for BaseType {
         self.ext_doc_url.as_deref()
     }
 
+    fn ext_previous_version_doc_url(&self) -> Option<&str> {
+        self.ext_previous_version_doc_url.as_deref()
+    }
+
     fn ext_doc_id(&self) -> Option<&str> {
         self.ext_doc_id.as_deref()
     }
@@ -619,6 +635,10 @@ impl<T: WithBaseType> ExternalDocument for T {
         self.base().doc_url()
     }
 
+    fn ext_previous_version_doc_url(&self) -> Option<&str> {
+        self.base().ext_previous_version_doc_url()
+    }
+
     fn ext_doc_id(&self) -> Option<&str> {
         self.base().doc_id()
     }
@@ -895,6 +915,9 @@ pub struct Endpoint {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ext_doc_url: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_previous_version_doc_url: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub deprecation: Option<Deprecation>,
 
@@ -945,6 +968,10 @@ impl ExternalDocument for Endpoint {
         self.ext_doc_url.as_deref()
     }
 
+    fn ext_previous_version_doc_url(&self) -> Option<&str> {
+        self.ext_previous_version_doc_url.as_deref()
+    }
+
     fn ext_doc_id(&self) -> Option<&str> {
         self.ext_doc_id.as_deref()
     }

compiler-rs/clients_schema_to_openapi/src/schemas.rs

@@ -215,10 +215,14 @@ impl<'a> TypesAndComponents<'a> {
                 .as_ref()
                 .and_then(|i| i.version.as_deref())
                 .unwrap_or("current");
+            let mut extensions: IndexMap<String,serde_json::Value> = Default::default();
+            if let Some(previous_version_doc_url) = obj.ext_previous_version_doc_url() {
+                extensions.insert("x-previousVersionUrl".to_string(), serde_json::json!(previous_version_doc_url));
+            }
             ExternalDocumentation {
                 description: None,
                 url: url.trim().replace("{branch}", branch),
-                extensions: Default::default(),
+                extensions,
             }
         })
     }

compiler-rs/compiler-wasm-lib/pkg/compiler_wasm_lib_bg.wasm

@@ -404,6 +404,7 @@ fn generate_interface_def(
             doc_url: None,
             ext_doc_id: None,
             ext_doc_url: None,
+            ext_previous_version_doc_url: None,
             codegen_name: None, // FIXME: extension in workplace search
             description: None,
             aliases: Vec::default(),

compiler-rs/openapi_to_clients_schema/src/types.rs

@@ -443,6 +443,7 @@ export class Endpoint {
   docId?: string
   extDocId?: string
   extDocUrl?: string
+  extPreviousVersionDocUrl?: string
   deprecation?: Deprecation
   availability: Availabilities
   docTag?: string

compiler/src/model/metamodel.ts

@@ -694,6 +694,9 @@ export function hoistRequestAnnotations (
       const docUrl = docIds.find(entry => entry[0] === value.trim())
       assert(jsDocs, docUrl != null, `The @doc_id '${value.trim()}' is not present in _doc_ids/table.csv`)
       endpoint.docUrl = docUrl[1].replace(/\r/g, '')
+      if (docUrl[2].replace(/\r/g, '') !== '') {
+        endpoint.extPreviousVersionDocUrl = docUrl[2].replace(/\r/g, '')
+      }
     } else if (tag === 'ext_doc_id') {
       assert(jsDocs, value.trim() !== '', `Request ${request.name.name}'s @ext_doc_id cannot be empty`)
       endpoint.extDocId = value.trim()

compiler/src/model/utils.ts

@@ -3,11 +3,11 @@
 It might happen that a new API in Elasticsearch is not yet defined
 in this repository, or we do have an endpoint definition in [`/specification/_json_spec`](../specification/_json_spec)
 but we don't have a type definition for it.
-In this document you will see how to add a new endpopint and how to add a new endpoint definition.
+In this document you will see how to add a new endpoint and how to add a new endpoint definition.
 
 ## How to add a new endpoint
 
-Add a new endpoint is straightforward, you only need to copy-paste the json rest-api-spec defintion
+Add a new endpoint is straightforward, you only need to copy-paste the json rest-api-spec definition
 from the Elasticsearch repository inside [`/specification/_json_spec`](../specification/_json_spec)
 and you are good to go.
 
@@ -17,10 +17,10 @@ or [here](https://github.com/elastic/elasticsearch/tree/7.x/x-pack/plugin/src/te
 ## How to add the definition of an endpoint
 
 Once you have added a new endpoint definition, the next step is to add its type definition.
-First of all, you should find the most approariate place inside [`/specification`](../specification)
+First of all, you should find the most appropriate place inside [`/specification`](../specification)
 where to put the new definition. The content of [`/specification`](../specification)
-tryied to mimic the Elasticsearch online documentation, so you can use it as inspiration.
-For example, the index document defintion can be found in [`/specification/_global/index`](../specification/_global/index).
+tried to mimic the Elasticsearch online documentation, so you can use it as inspiration.
+For example, the index document definition can be found in [`/specification/_global/index`](../specification/_global/index).
 
 Once you have found the best place for the new definition, you should create a new file for it.
 The filename should be the same of the type definition you are writing, for example:
@@ -40,16 +40,16 @@ you can define it in the same file where it's used, unless is a commonly used ty
 
 ### Add the endpoint request definition
 
-Request definitions are slighly different from other definitions.
+Request definitions are slightly different from other definitions.
 It is required that the request definition is named `Request`.
-A request definition is an interface and should contains three top level keys:
+A request definition is an interface and should contain these top level keys:
 
 - `urls`: the URL paths templates and allowed HTTP methods
 - `path_parts`: the path parameters (eg: `indices`, `id`...)
 - `query_parameters`: the query parameters (eg: `timeout`, `pipeline`...)
 - `body`: the body parameters (eg: `query` or user defined entities)
 
-Furthermore, every request definition **must** contain three JS Doc tags:
+Furthermore, every request definition **must** contain these JS Doc tags:
 
 - `@rest_spec_name`: the API name (eg: `search`, `indices.create`...).
 - `@availability` Which flavor of Elasticsearch is this API available for? (eg `stack` or `serverless`)
@@ -130,7 +130,7 @@ class Response {
 ```
 
 As you can see, for responses there are no custom top level keys, as the
-response definition represents the body of a succesful response.
+response definition represents the body of a successful response.
 
 #### Generics
 
@@ -172,7 +172,7 @@ class Response {
 Add an `examples` folder and `request` and `xxx_response` subfolders (where `xxx` is the relevant response code). The file names within these folders should be unique (for example,`DownsampleRequestExample1.yaml` not `RequestExample1.yaml`).
 
 These examples are for use in the API documentation and must adhere to the [OpenAPI 3.0 Example object specification](https://spec.openapis.org/oas/v3.0.3#example-object). They must have a `value` field that contains the request or response body.
-If there are multiple examples for the endpoint, they must each have a brief `summary` field, which is used as the label for the example. You can also optionaly provide an explanation in a `description` field.
+If there are multiple examples for the endpoint, they must each have a brief `summary` field, which is used as the label for the example. You can also optionally provide an explanation in a `description` field.
 In order to generate curl and console examples automatically, the request examples must also contain a `method_request`.
 
 For example:
@@ -197,5 +197,5 @@ value: |-
 ```
 
 NOTE: A good example covers a very common use case or demonstrates a more complex but critical use case.
-It involves realistic data sets ( rather than generic "hello world" values).
+It involves realistic data sets (rather than generic "hello world" values).
 If it requires detailed setup or explanations, however, it is more appropriate for coverage in longer-form narrative documentation.

docs/add-new-api.md

@@ -148,7 +148,7 @@ property: UserDefinedValue
 
 ### Numbers
 
-The numeric type in TypeScript is `number`, but given that this specification targets mutliple languages,
+The numeric type in TypeScript is `number`, but given that this specification targets multiple languages,
 it offers a bunch of aliases that represent the type that should be used if the language supports it:
 
 ```ts
@@ -504,7 +504,7 @@ Code generators should track the `es_quirk` they implement and fail if a new unh
 
 ### Additional information
 
-If needed, you can specify additional information on each type with the approariate JSDoc tag.
+If needed, you can specify additional information on each type with the appropriate JSDoc tag.
 Following you can find a list of the supported tags:
 
 #### `@availability`

docs/modeling-guide.md

@@ -30,6 +30,7 @@ end with `Request` or `Response`.
 ### Request and Response definitions
 
 Request and Response definitions should be placed by strictly following
+the rest-api-spec structure.
 the `rest-api-spec` structure.
 For example, the request and response definition for `indices.put_mapping`
 should go in [`/specification/indices/put_mapping`](../specification/indices/put_mapping).