Add @ext_doc_id for externalDocs in OpenAPI documents (#3028) (#3032)

compiler-rs/clients_schema/src/lib.rs

@@ -55,6 +55,11 @@ pub trait Documented {
     fn description(&self) -> Option<&str>;
 }
 
+pub trait ExternalDocument {
+    fn ext_doc_id(&self) -> Option<&str>;
+    fn ext_doc_url(&self) -> Option<&str>;
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TypeName {
     // Order is important for Ord implementation
@@ -314,6 +319,12 @@ pub struct Property {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub doc_id: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_url: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_id: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub server_default: Option<ServerDefault>,
 
@@ -355,6 +366,16 @@ impl Documented for Property {
     }
 }
 
+impl ExternalDocument for Property {
+    fn ext_doc_url(&self) -> Option<&str> {
+        self.ext_doc_url.as_deref()
+    }
+
+    fn ext_doc_id(&self) -> Option<&str> {
+        self.ext_doc_id.as_deref()
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum ServerDefault {
@@ -477,6 +498,13 @@ pub struct BaseType {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub doc_id: Option<String>,
 
+    /// Link to public documentation
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_url: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_id: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub deprecation: Option<Deprecation>,
 
@@ -512,6 +540,8 @@ impl BaseType {
             description: None,
             variant_name: None,
             spec_location: None,
+            ext_doc_id: None,
+            ext_doc_url: None,
         }
     }
 }
@@ -530,6 +560,16 @@ impl Documented for BaseType {
     }
 }
 
+impl ExternalDocument for BaseType {
+    fn ext_doc_url(&self) -> Option<&str> {
+        self.ext_doc_url.as_deref()
+    }
+
+    fn ext_doc_id(&self) -> Option<&str> {
+        self.ext_doc_id.as_deref()
+    }
+}
+
 trait WithBaseType {
     fn base(&self) -> &BaseType;
 }
@@ -548,6 +588,16 @@ impl<T: WithBaseType> Documented for T {
     }
 }
 
+impl<T: WithBaseType> ExternalDocument for T {
+    fn ext_doc_url(&self) -> Option<&str> {
+        self.base().doc_url()
+    }
+
+    fn ext_doc_id(&self) -> Option<&str> {
+        self.base().doc_id()
+    }
+}
+
 /// An interface type
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -809,6 +859,12 @@ pub struct Endpoint {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub doc_id: Option<String>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_id: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ext_doc_url: Option<String>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub deprecation: Option<Deprecation>,
 
@@ -854,6 +910,16 @@ impl Documented for Endpoint {
     }
 }
 
+impl ExternalDocument for Endpoint {
+    fn ext_doc_url(&self) -> Option<&str> {
+        self.ext_doc_url.as_deref()
+    }
+
+    fn ext_doc_id(&self) -> Option<&str> {
+        self.ext_doc_id.as_deref()
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Privileges {

compiler-rs/clients_schema_to_openapi/src/paths.rs

@@ -203,8 +203,8 @@ pub fn add_endpoint(
             },
             summary: sum_desc.summary,
             description: sum_desc.description,
-            // external_docs: tac.convert_external_docs(endpoint),
-            external_docs: None, // Need values that differ from client purposes
+            external_docs: tac.convert_external_docs(endpoint),
+            // external_docs: None, // Need values that differ from client purposes
             operation_id: None, // set in clone_operation below with operation_counter
             parameters,
             request_body: request_body.clone(),

compiler-rs/clients_schema_to_openapi/src/schemas.rs

@@ -209,9 +209,9 @@ impl<'a> TypesAndComponents<'a> {
         self.for_body(&response.body)
     }
 
-    pub fn convert_external_docs(&self, obj: &impl clients_schema::Documented) -> Option<ExternalDocumentation> {
+    pub fn convert_external_docs(&self, obj: &impl clients_schema::ExternalDocument) -> Option<ExternalDocumentation> {
         // FIXME: does the model contain resolved doc_id?
-        obj.doc_url().map(|url| {
+        obj.ext_doc_url().map(|url| {
             let branch: &str = self
                 .model
                 .info

compiler-rs/openapi_to_clients_schema/src/types.rs

@@ -121,7 +121,7 @@ fn generate_type_for_schema(
         })
     }
     if let Some(ref docs) = data.external_docs {
-        base.doc_url = Some(docs.url.clone())
+        base.ext_doc_url = Some(docs.ext_docs_url.clone())
     }
 
     // TODO: data.readonly/writeonly -> OverloadOf?

compiler/src/model/build-model.ts

@@ -68,6 +68,7 @@ export function compileEndpoints (): Record<string, model.Endpoint> {
       description: spec.documentation.description,
       docUrl: spec.documentation.url,
       docTag: spec.docTag,
+      extDocUrl: spec.externalDocs?.url,
       // Setting these values by default should be removed
       // when we no longer use rest-api-spec stubs as the
       // source of truth for stability/visibility.

compiler/src/model/json-spec.ts

@@ -62,6 +62,10 @@ export interface JsonSpec {
     required?: boolean
   }
   docTag?: string
+  externalDocs?: {
+    url: string
+    description?: string
+  }
 }
 
 export default function buildJsonSpec (): Map<string, JsonSpec> {

compiler/src/model/metamodel.ts

@@ -126,6 +126,8 @@ export class Property {
   description?: string
   docUrl?: string
   docId?: string
+  extDocId?: string
+  extDocUrl?: string
   serverDefault?: boolean | string | number | string[] | number[]
   deprecation?: Deprecation
   availability?: Availabilities
@@ -158,6 +160,8 @@ export abstract class BaseType {
   /** Link to public documentation */
   docUrl?: string
   docId?: string
+  extDocId?: string
+  extDocUrl?: string
   deprecation?: Deprecation
   /** If this endpoint has a quirk that needs special attention, give a short explanation about it */
   esQuirk?: string
@@ -406,6 +410,8 @@ export class Endpoint {
   description: string
   docUrl: string
   docId?: string
+  extDocId?: string
+  extDocUrl?: string
   deprecation?: Deprecation
   availability: Availabilities
   docTag?: string

compiler/src/model/utils.ts

@@ -625,7 +625,7 @@ export function hoistRequestAnnotations (
   request: model.Request, jsDocs: JSDoc[], mappings: Record<string, model.Endpoint>, response: model.TypeName | null
 ): void {
   const knownRequestAnnotations = [
-    'rest_spec_name', 'behavior', 'class_serializer', 'index_privileges', 'cluster_privileges', 'doc_id', 'availability', 'doc_tag'
+    'rest_spec_name', 'behavior', 'class_serializer', 'index_privileges', 'cluster_privileges', 'doc_id', 'availability', 'doc_tag', 'ext_doc_id'
   ]
   // in most of the cases the jsDocs comes in a single block,
   // but it can happen that the user defines multiple single line jsDoc.
@@ -685,6 +685,12 @@ 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, '')
+    } 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()
+      const docUrl = docIds.find(entry => entry[0] === value.trim())
+      assert(jsDocs, docUrl != null, `The @ext_doc_id '${value.trim()}' is not present in _doc_ids/table.csv`)
+      endpoint.extDocUrl = docUrl[1].replace(/\r/g, '')
     } else if (tag === 'availability') {
       // The @availability jsTag is different than most because it allows
       // multiple values within the same docstring, hence needing to parse
@@ -713,7 +719,7 @@ export function hoistTypeAnnotations (type: model.TypeDefinition, jsDocs: JSDoc[
   assert(jsDocs, jsDocs.length < 2, 'Use a single multiline jsDoc block instead of multiple single line blocks')
 
   const validTags = ['class_serializer', 'doc_url', 'doc_id', 'behavior', 'variants', 'variant', 'shortcut_property',
-    'codegen_names', 'non_exhaustive', 'es_quirk', 'behavior_meta']
+    'codegen_names', 'non_exhaustive', 'es_quirk', 'behavior_meta', 'ext_doc_id']
   const tags = parseJsDocTags(jsDocs)
   if (jsDocs.length === 1) {
     const description = jsDocs[0].getDescription()
@@ -742,6 +748,12 @@ export function hoistTypeAnnotations (type: model.TypeDefinition, jsDocs: JSDoc[
       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`)
       type.docUrl = docUrl[1].replace(/\r/g, '')
+    } else if (tag === 'ext_doc_id') {
+      assert(jsDocs, value.trim() !== '', `Type ${type.name.namespace}.${type.name.name}'s @ext_doc_id cannot be empty`)
+      type.extDocId = value.trim()
+      const docUrl = docIds.find(entry => entry[0] === value.trim())
+      assert(jsDocs, docUrl != null, `The @ext_doc_id '${value.trim()}' is not present in _doc_ids/table.csv`)
+      type.extDocUrl = docUrl[1].replace(/\r/g, '')
     } else if (tag === 'codegen_names') {
       type.codegenNames = parseCommaSeparated(value)
       assert(jsDocs,
@@ -764,7 +776,7 @@ function hoistPropertyAnnotations (property: model.Property, jsDocs: JSDoc[]): v
   assert(jsDocs, jsDocs.length < 2, 'Use a single multiline jsDoc block instead of multiple single line blocks')
 
   const validTags = ['prop_serializer', 'doc_url', 'aliases', 'codegen_name', 'server_default',
-    'variant', 'doc_id', 'es_quirk', 'availability']
+    'variant', 'doc_id', 'es_quirk', 'availability', 'ext_doc_id']
   const tags = parseJsDocTags(jsDocs)
   if (jsDocs.length === 1) {
     const description = jsDocs[0].getDescription()
@@ -808,6 +820,13 @@ function hoistPropertyAnnotations (property: model.Property, jsDocs: JSDoc[]): v
       if (docUrl != null) {
         property.docUrl = docUrl[1].replace(/\r/g, '')
       }
+    } else if (tag === 'ext_doc_id') {
+      assert(jsDocs, value.trim() !== '', `Property ${property.name}'s @ext_doc_id is cannot be empty`)
+      property.extDocId = value
+      const docUrl = docIds.find(entry => entry[0] === value)
+      if (docUrl != null) {
+        property.extDocUrl = docUrl[1].replace(/\r/g, '')
+      }
     } else if (tag === 'server_default') {
       assert(jsDocs, property.type.kind === 'instance_of' || property.type.kind === 'union_of' || property.type.kind === 'array_of', `Default values can only be configured for instance_of or union_of types, you are using ${property.type.kind}`)
       assert(jsDocs, !property.required, 'Default values can only be specified on optional properties')

docs/doc-comments-guide.md

@@ -58,7 +58,7 @@ export interface Request extends RequestBase {
 ([original source code](https://github.com/elastic/elasticsearch-specification/blob/main/specification/_global/rank_eval/RankEvalRequest.ts))
 
 For more information about the tags in this example (and other common tags such
-as `@deprecated` and `@doc_id`), refer to the [Modeling Guide](https://github.com/elastic/elasticsearch-specification/blob/main/docs/modeling-guide.md#additional-information).
+as `@deprecated` and `@ext_doc_id`), refer to the [Modeling Guide](https://github.com/elastic/elasticsearch-specification/blob/main/docs/modeling-guide.md#additional-information).
 
 ## Markup language
 
@@ -76,9 +76,9 @@ GFM also has implementations in most languages, meaning that code generators wil
 
 **Doc comments are reference material**: they should be as succinct as possible while capturing all the necessary information to use the elements they're documenting. Remember that they will often show up in small IDE autocompletion popups!
 
-In particular, doc comments are not the right place for tutorials or examples, which should be in dedicated documentation pages. These pages can of course be linked from the doc comments.
+In particular, doc comments are not the right place for tutorials or extended examples, which should be in dedicated documentation pages. To reduce the risk of broken links, use `@ext_doc_id` to implement a link to additional documentation. 
 
-API endpoints will also have a `@doc_url` JSDoc tag that links to that API's detailed documentation page.
+API endpoints can also have `@doc_id` or `@doc_url` JSDoc tags that enable clients to link to the API docs, for example.
 
 ### Multi-paragraph doc comments
 

docs/modeling-guide.md

@@ -598,37 +598,61 @@ class Foo {
 }
 ```
 
-#### `@doc_url`
+#### `@doc_id`
 
-The documentation url for the parameter or definition.
-If possible, use `@doc_id`.
+An identifier that can be used for generating the doc url in clients.
+The unique id/url pair must exist in `specification/_doc_ids/table.csv`.
+NOTE: This link is *not* included in the OpenAPI output.
 
 ```ts
-class Foo {
-  bar: string
-  /** @doc_url http://localhost:9200 */
-  baz?: string
-  faz: string
+/**
+ * @rest_spec_name api
+ * @doc_id foobar
+ */
+class Request {
+  ...
 }
 ```
 
-#### `@doc_id`
+```csv
+foobar,/guide/en/example
+```
+
+#### `@ext_doc_id`
 
-The documentation id that can be used for generating the doc url.
-You must add the id/url pair in `specification/_doc_ids/table.csv`.
+An identifier for a link.
+The unique id/url pair must exist in `specification/_doc_ids/table.csv`.
+NOTE: This link is included in the OpenAPI output.
 
 ```ts
 /**
- * @rest_spec_name api
- * @doc_id foobar
+ * @variants container
+ * @non_exhaustive
+ * @ext_doc_id query-dsl
  */
-class Request {
+export class QueryContainer {
   ...
 }
 ```
 
 ```csv
-foobar,/guide/en/example
+query-dsl,/guide/en/example
+```
+
+
+#### `@doc_url`
+
+The documentation url for the parameter or definition.
+To reduce the risk of broken links, use `@doc_id` instead.
+NOTE: This link is *not* included in the OpenAPI output.
+
+```ts
+class Foo {
+  bar: string
+  /** @doc_url http://localhost:9200 */
+  baz?: string
+  faz: string
+}
 ```
 
 #### `@doc_tag`

docs/overlays/elasticsearch-shared-overlays.yaml

@@ -240,9 +240,6 @@ actions:
     description: Add x-model and updated externalDocs for the QueryContainer object
     update:
       x-model: true
-      externalDocs:
-        url: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl.html
-        description: Query domain specific language (DSL) reference
   - target: "$.components['schemas']['_types.analysis:CharFilter'].oneOf"
     description: Remove existing oneOf definition for CharFilter
     remove: true