diff --git a/compiler-rs/Cargo.lock b/compiler-rs/Cargo.lock index 25c46faaff..2d50ee7e09 100644 --- a/compiler-rs/Cargo.lock +++ b/compiler-rs/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -199,6 +199,7 @@ dependencies = [ "clap", "derive_more", "indexmap", + "itertools", "once_cell", "serde", "serde_ignored", @@ -332,6 +333,12 @@ dependencies = [ "time", ] +[[package]] +name = "either" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" + [[package]] name = "env_filter" version = "0.1.2" @@ -521,6 +528,15 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" +[[package]] +name = "itertools" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.11" diff --git a/compiler-rs/Cargo.toml b/compiler-rs/Cargo.toml index 2145b82873..294ac02bc0 100644 --- a/compiler-rs/Cargo.toml +++ b/compiler-rs/Cargo.toml @@ -17,6 +17,7 @@ derive_more = "1.0.0-beta.6" either_n = "0.2" icu_segmenter = "1" indexmap = "2" +itertools = "0.14" maplit = "1" once_cell = "1.16" openapiv3 = "2" diff --git a/compiler-rs/clients_schema/Cargo.toml b/compiler-rs/clients_schema/Cargo.toml index ab40e93544..f2caeeaa67 100644 --- a/compiler-rs/clients_schema/Cargo.toml +++ b/compiler-rs/clients_schema/Cargo.toml @@ -11,6 +11,8 @@ serde_json = { workspace = true } once_cell = { workspace = true } anyhow = { workspace = true } indexmap = { workspace = true, features = ["serde"] } +itertools = { workspace = true } + arcstr = { workspace = true, features = ["serde", "substr"] } clap = { workspace = true, features = ["derive"] } diff --git a/compiler-rs/clients_schema/src/bin/add_url_paths.rs b/compiler-rs/clients_schema/src/bin/add_url_paths.rs new file mode 100644 index 0000000000..6d20f7fe18 --- /dev/null +++ b/compiler-rs/clients_schema/src/bin/add_url_paths.rs @@ -0,0 +1,121 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use clap::Parser; +use itertools::Itertools; + + +fn main() -> anyhow::Result<()> { + let cli = Cli::parse(); + cli.run()?; + Ok(()) +} + +// Example usage: +// (cd compiler-rs; find ../specification -name '*Request.ts' | cargo run --bin add_url_paths ../output/schema/schema.json | sh) + +/// Adds url paths to request definitions. Stdin must be a list of files, one per line. +/// Outputs a shell script that uses ast-grep. +#[derive(Debug, Parser)] +#[command(author, version, about, long_about)] +pub struct Cli { + /// input schema file, eg: ../output/schema/schema-no-generics.json + schema: PathBuf, +} + +impl Cli { + pub fn run(&self) -> anyhow::Result<()> { + + // Canonicalize all file names, so that we can do some suffix mapping from the schema locations. + let files: Vec = std::io::read_to_string(std::io::stdin())? + .lines() + .flat_map(|line| std::fs::canonicalize(line) + .map_err(|e| { + eprintln!("File {} not found", line); + Result::::Err(e) + })) // Remove errors + .collect(); + + let json = std::fs::read_to_string(&self.schema)?; + let schema = clients_schema::IndexedModel::from_reader(json.as_bytes())?; + + let mut location_to_request = HashMap::<&Path, &clients_schema::Endpoint>::new(); + for ep in &schema.endpoints { + let Some(req_name) = ep.request.as_ref() else { + //eprintln!("Skipping endpoint {} with no request", ep.name); + continue; + }; + + let type_def = schema.types.get(req_name).unwrap(); + let location = type_def.base().spec_location.as_ref().unwrap(); + let location = Path::new(location.split_once('#').unwrap().0); + + location_to_request.insert(location, ep); + }; + + for file in files { + if let Some((_, endpoint)) = location_to_request.iter().find(|(location, _)| file.ends_with(location)) { + generate_astgrep_command(&file, endpoint); + } else { + eprintln!("No request found for {:?}", file); + } + } + + Ok(()) + } +} + +fn generate_astgrep_command(file: &Path, endpoint: &clients_schema::Endpoint) { + + let text = std::fs::read_to_string(file).unwrap(); + if text.contains("urls:") { + eprintln!("Found an existing 'url' property. Skipping {file:?}"); + return; + } + + // We cannot express conditional parts in the source form of patterns. + + // Requests with generic parameters + let request_expr = if text.contains("Request<") { + "Request<$$$PARAM>" + } else { + "Request" + }; + + // A handful of requests don't have an extends clause + let extends_expr = if text.contains(" extends ") { + "extends $REQBASE" + } else { + "" + }; + + let urls: String = endpoint.urls.iter().map(|url| { + let path = &url.path; + let methods = url.methods.iter().map(|method| format!("\"{}\"", method)).join(", "); + let deprecation = match &url.deprecation { + Some(deprecation) => format!("/** @deprecated {} {} */\n ", deprecation.version, deprecation.description), + None => "".to_string(), + }; + + format!(r#" {{ + {deprecation}path: "{path}", + methods: [{methods}] + }}"#) + }).join(",\n"); + + let pattern = format!(r#"interface {request_expr} {extends_expr} {{ + $$$PROPS +}}"#); + + let fix = format!(r#"interface {request_expr} {extends_expr} {{ + urls: [ +{urls} + ], + $$$PROPS +}}"#); + + let file = file.to_str().unwrap(); + println!("#----- {file}"); + println!(r#"ast-grep --update-all --lang ts --pattern '{pattern}' --rewrite '{fix}' "{file}""#); + + println!(); +} diff --git a/compiler/src/model/build-model.ts b/compiler/src/model/build-model.ts index 5d9187f6c8..96bb7f511b 100644 --- a/compiler/src/model/build-model.ts +++ b/compiler/src/model/build-model.ts @@ -53,7 +53,7 @@ import { verifyUniqueness, parseJsDocTags, deepEqual, - sourceLocation, sortTypeDefinitions + sourceLocation, sortTypeDefinitions, parseDeprecation } from './utils' const jsonSpec = buildJsonSpec() @@ -210,14 +210,6 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int if (mapping == null) { throw new Error(`Cannot find url template for ${namespace}, very likely the specification folder does not follow the rest-api-spec`) } - // list of unique dynamic parameters - const urlTemplateParams = [...new Set( - mapping.urls.flatMap(url => url.path.split('/') - .filter(part => part.includes('{')) - .map(part => part.slice(1, -1)) - ) - )] - const methods = [...new Set(mapping.urls.flatMap(url => url.methods))] let pathMember: Node | null = null let bodyProperties: model.Property[] = [] @@ -226,27 +218,29 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int // collect path/query/body properties for (const member of declaration.getMembers()) { - // we are visiting `path_parts, `query_parameters` or `body` + // we are visiting `urls`, `path_parts, `query_parameters` or `body` assert( member, Node.isPropertyDeclaration(member) || Node.isPropertySignature(member), 'Class and interfaces can only have property declarations or signatures' ) - const property = visitRequestOrResponseProperty(member) - if (property.name === 'path_parts') { + const name = member.getName() + if (name === 'urls') { + // Overwrite the endpoint urls read from the json-rest-spec + // TODO: once all spec files are using it, make it mandatory. + mapping.urls = visitUrls(member) + } else if (name === 'path_parts') { + const property = visitRequestOrResponseProperty(member) assert(member, property.properties.length > 0, 'There is no need to declare an empty object path_parts, just remove the path_parts declaration.') pathMember = member type.path = property.properties - } else if (property.name === 'query_parameters') { + } else if (name === 'query_parameters') { + const property = visitRequestOrResponseProperty(member) assert(member, property.properties.length > 0, 'There is no need to declare an empty object query_parameters, just remove the query_parameters declaration.') type.query = property.properties - } else if (property.name === 'body') { + } else if (name === 'body') { + const property = visitRequestOrResponseProperty(member) bodyMember = member - assert( - member, - methods.some(method => ['POST', 'PUT', 'DELETE'].includes(method)), - `${namespace}.${name} can't have a body, allowed methods: ${methods.join(', ')}` - ) if (property.valueOf != null) { bodyValue = property.valueOf } else { @@ -254,11 +248,20 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int bodyProperties = property.properties } } else { - assert(member, false, `Unknown request property: ${property.name}`) + assert(member, false, `Unknown request property: ${name}`) } } // validate path properties + // list of unique dynamic parameters + const urlTemplateParams = [...new Set( + mapping.urls.flatMap(url => url.path.split('/') + .filter(part => part.includes('{')) + .map(part => part.slice(1, -1)) + ) + )] + const methods = [...new Set(mapping.urls.flatMap(url => url.methods))] + for (const part of type.path) { assert( pathMember as Node, @@ -282,6 +285,13 @@ function compileClassOrInterfaceDeclaration (declaration: ClassDeclaration | Int } // validate body + if (bodyMember != null) { + assert( + bodyMember, + methods.some(method => ['POST', 'PUT', 'DELETE'].includes(method)), + `${namespace}.${name} can't have a body, allowed methods: ${methods.join(', ')}` + ) + } // the body can either be a value (eg Array or an object with properties) if (bodyValue != null) { // Propagate required body value nature based on TS question token being present. @@ -587,3 +597,80 @@ function visitRequestOrResponseProperty (member: PropertyDeclaration | PropertyS return { name, properties, valueOf } } + +/** + * Parse the 'urls' property of a request definition. Format is: + * ``` + * urls: [ + * { + * /** @deprecated 1.2.3 Use something else + * path: '/some/path', + * methods: ["GET", "POST"] + * } + * ] + * ``` + */ +function visitUrls (member: PropertyDeclaration | PropertySignature): model.UrlTemplate[] { + const value = member.getTypeNode() + + // Literal arrays are exposed as tuples by ts-morph + assert(value, Node.isTupleTypeNode(value), '"urls" should be an array') + + const result: model.UrlTemplate[] = [] + + value.forEachChild(urlNode => { + assert(urlNode, Node.isTypeLiteral(urlNode), '"urls" members should be objects') + + const urlTemplate: any = {} + + urlNode.forEachChild(node => { + assert(node, Node.isPropertySignature(node), "Expecting 'path' and 'methods' properties") + + const name = node.getName() + const propValue = node.getTypeNode() + + if (name === 'path') { + assert(propValue, Node.isLiteralTypeNode(propValue), '"path" should be a string') + + const pathLit = propValue.getLiteral() + assert(pathLit, Node.isStringLiteral(pathLit), '"path" should be a string') + + urlTemplate.path = pathLit.getLiteralValue() + + // Deprecation + const jsDoc = node.getJsDocs() + const tags = parseJsDocTags(jsDoc) + const deprecation = parseDeprecation(tags, jsDoc) + if (deprecation != null) { + urlTemplate.deprecation = deprecation + } + if (Object.keys(tags).length > 0) { + assert(jsDoc, false, `Unknown annotations: ${Object.keys(tags).join(', ')}`) + } + } else if (name === 'methods') { + assert(propValue, Node.isTupleTypeNode(propValue), '"methods" should be an array') + + const methods: string[] = [] + propValue.forEachChild(node => { + assert(node, Node.isLiteralTypeNode(node), '"methods" should contain strings') + + const nodeLit = node.getLiteral() + assert(nodeLit, Node.isStringLiteral(nodeLit), '"methods" should contain strings') + + methods.push(nodeLit.getLiteralValue()) + }) + assert(node, methods.length > 0, "'methods' should not be empty") + urlTemplate.methods = methods + } else { + assert(node, false, "Expecting 'path' or 'methods'") + } + }) + + assert(urlTemplate, urlTemplate.path, "Missing required property 'path'") + assert(urlTemplate, urlTemplate.methods, "Missing required property 'methods'") + + result.push(urlTemplate) + }) + + return result +} diff --git a/compiler/src/model/utils.ts b/compiler/src/model/utils.ts index f3452accca..886c96b0fd 100644 --- a/compiler/src/model/utils.ts +++ b/compiler/src/model/utils.ts @@ -576,12 +576,21 @@ export function modelProperty (declaration: PropertySignature | PropertyDeclarat * Pulls @deprecated from types and properties */ function setDeprecated (type: model.BaseType | model.Property | model.EnumMember, tags: Record, jsDocs: JSDoc[]): void { + const deprecation = parseDeprecation(tags, jsDocs) + if (deprecation != null) { + type.deprecation = deprecation + } +} + +export function parseDeprecation (tags: Record, jsDocs: JSDoc[]): model.Deprecation | undefined { if (tags.deprecated !== undefined) { const [version, ...description] = tags.deprecated.split(' ') assert(jsDocs, semver.valid(version), 'Invalid semver value') - type.deprecation = { version, description: description.join(' ') } + delete tags.deprecated + return { version, description: description.join(' ') } + } else { + return undefined } - delete tags.deprecated } /** diff --git a/compiler/src/steps/validate-rest-spec.ts b/compiler/src/steps/validate-rest-spec.ts index 68aab4a61d..ba9e6a024d 100644 --- a/compiler/src/steps/validate-rest-spec.ts +++ b/compiler/src/steps/validate-rest-spec.ts @@ -21,6 +21,7 @@ import assert from 'assert' import * as model from '../model/metamodel' import { JsonSpec } from '../model/json-spec' import { ValidationErrors } from '../validation-errors' +import { deepEqual } from '../model/utils' // This code can be simplified once https://github.com/tc39/proposal-set-methods is available @@ -46,6 +47,30 @@ export default async function validateRestSpec (model: model.Model, jsonSpec: Ma const spec = jsonSpec.get(endpoint.name) assert(spec, `Can't find the json spec for ${endpoint.name}`) + // Check URL paths and methods + if (spec.url.paths.length !== endpoint.urls.length) { + errors.addEndpointError(endpoint.name, 'request', `${endpoint.request.name}: different number of urls in the json spec`) + } else { + for (const modelUrl of endpoint.urls) { + // URL path + const restSpecUrl = spec.url.paths.find(path => path.path === modelUrl.path) + if (restSpecUrl == null) { + errors.addEndpointError(endpoint.name, 'request', `${endpoint.request.name}: url path '${modelUrl.path}' not found in the json spec`) + } else { + // URL methods + if (!deepEqual([...restSpecUrl.methods].sort(), [...modelUrl.methods].sort())) { + errors.addEndpointError(endpoint.name, 'request', `${modelUrl.path}: different http methods in the json spec`) + } + + // Deprecation. + if ((restSpecUrl.deprecated != null) !== (modelUrl.deprecation != null)) { + errors.addEndpointError(endpoint.name, 'request', `${endpoint.request.name}: different deprecation in the json spec`) + } + } + } + } + + // Check url parts const urlParts = Array.from(new Set(spec.url.paths .filter(path => path.parts != null) .flatMap(path => { diff --git a/docs/add-new-api.md b/docs/add-new-api.md index d389ea7353..64c963a0c2 100644 --- a/docs/add-new-api.md +++ b/docs/add-new-api.md @@ -44,6 +44,7 @@ Request definitions are slighly 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: +- `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) @@ -67,6 +68,12 @@ Following you can find a template valid for any request definition. * @availability stack since=1.2.3 stability=stable|beta|experimental */ interface Request extends RequestBase { + urls: [ + { + path: "/path/with/{parts}" + methods: ["POST"] + } + ] path_parts: { } @@ -86,6 +93,12 @@ In some cases, the request could take one or more generics, in such case the def * @availability stack since=1.2.3 stability=stable|beta|experimental */ interface Request extends RequestBase { + urls: [ + { + path: "/path/with/{parts}" + methods: ["POST"] + } + ] path_parts: { } diff --git a/output/schema/schema-serverless.json b/output/schema/schema-serverless.json index ab14f46f43..9f559192f2 100644 --- a/output/schema/schema-serverless.json +++ b/output/schema/schema-serverless.json @@ -11778,7 +11778,7 @@ } } ], - "specLocation": "_global/bulk/BulkRequest.ts#L32-L115" + "specLocation": "_global/bulk/BulkRequest.ts#L32-L125" }, { "body": { @@ -12940,7 +12940,7 @@ } ], "query": [], - "specLocation": "_global/clear_scroll/ClearScrollRequest.ts#L23-L48" + "specLocation": "_global/clear_scroll/ClearScrollRequest.ts#L23-L59" }, { "body": { diff --git a/output/schema/schema.json b/output/schema/schema.json index d76da617a7..c1a1e1c630 100644 --- a/output/schema/schema.json +++ b/output/schema/schema.json @@ -22289,7 +22289,7 @@ } } ], - "specLocation": "_global/bulk/BulkRequest.ts#L32-L115" + "specLocation": "_global/bulk/BulkRequest.ts#L32-L125" }, { "kind": "response", @@ -22790,7 +22790,7 @@ } ], "query": [], - "specLocation": "_global/clear_scroll/ClearScrollRequest.ts#L23-L48" + "specLocation": "_global/clear_scroll/ClearScrollRequest.ts#L23-L59" }, { "kind": "response", diff --git a/specification/_global/bulk/BulkRequest.ts b/specification/_global/bulk/BulkRequest.ts index b53adf3779..0630ce1c10 100644 --- a/specification/_global/bulk/BulkRequest.ts +++ b/specification/_global/bulk/BulkRequest.ts @@ -41,6 +41,16 @@ import { OperationContainer, UpdateAction } from './types' * */ export interface Request extends RequestBase { + urls: [ + { + path: '/_bulk' + methods: ['POST', 'PUT'] + }, + { + path: '/{index}/_bulk' + methods: ['POST', 'PUT'] + } + ] path_parts: { /** * Name of the data stream, index, or index alias to perform bulk actions on. diff --git a/specification/_global/clear_scroll/ClearScrollRequest.ts b/specification/_global/clear_scroll/ClearScrollRequest.ts index 1388e0a1fb..291b5d3941 100644 --- a/specification/_global/clear_scroll/ClearScrollRequest.ts +++ b/specification/_global/clear_scroll/ClearScrollRequest.ts @@ -31,6 +31,17 @@ import { ScrollIds } from '@_types/common' * @doc_tag search */ export interface Request extends RequestBase { + urls: [ + { + path: '/_search/scroll' + methods: ['DELETE'] + }, + { + /** @deprecated 7.0.0 A scroll id can be quite large and should be specified as part of the body */ + path: '/_search/scroll/{scroll_id}' + methods: ['DELETE'] + } + ] path_parts: { /** * Comma-separated list of scroll IDs to clear.