Skip to content

Commit

Permalink
Add @ext_doc_id for externalDocs in OpenAPI documents (#3028) (#3031)
Browse files Browse the repository at this point in the history
  • Loading branch information
lcawl authored Oct 22, 2024
1 parent 16028e6 commit fe3cffe
Show file tree
Hide file tree
Showing 18 changed files with 158 additions and 587 deletions.
66 changes: 66 additions & 0 deletions compiler-rs/clients_schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>,

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>,

Expand Down Expand Up @@ -512,6 +540,8 @@ impl BaseType {
description: None,
variant_name: None,
spec_location: None,
ext_doc_id: None,
ext_doc_url: None,
}
}
}
Expand All @@ -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;
}
Expand All @@ -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")]
Expand Down Expand Up @@ -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>,

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions compiler-rs/clients_schema_to_openapi/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 2 additions & 2 deletions compiler-rs/clients_schema_to_openapi/src/schemas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Binary file modified compiler-rs/compiler-wasm-lib/pkg/compiler_wasm_lib_bg.wasm
Binary file not shown.
2 changes: 1 addition & 1 deletion compiler-rs/openapi_to_clients_schema/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
1 change: 1 addition & 0 deletions compiler/src/model/build-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/model/json-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ export interface JsonSpec {
required?: boolean
}
docTag?: string
externalDocs?: {
url: string
description?: string
}
}

export default function buildJsonSpec (): Map<string, JsonSpec> {
Expand Down
6 changes: 6 additions & 0 deletions compiler/src/model/metamodel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -406,6 +410,8 @@ export class Endpoint {
description: string
docUrl: string
docId?: string
extDocId?: string
extDocUrl?: string
deprecation?: Deprecation
availability: Availabilities
docTag?: string
Expand Down
25 changes: 22 additions & 3 deletions compiler/src/model/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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')
Expand Down
6 changes: 3 additions & 3 deletions docs/doc-comments-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
54 changes: 39 additions & 15 deletions docs/modeling-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
3 changes: 0 additions & 3 deletions docs/overlays/elasticsearch-shared-overlays.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit fe3cffe

Please sign in to comment.