From 4299a197341f3e3cbe5be052ae6003c6c82f9ddb Mon Sep 17 00:00:00 2001 From: Theo Nam Truong Date: Tue, 16 Apr 2024 05:13:37 -0600 Subject: [PATCH] Global Params (#241) * Added tests for merger Signed-off-by: Theo Truong * # checking if build step was successful Signed-off-by: Theo Truong * # removed diagnostic code Signed-off-by: Theo Truong * # added validation for global spec Signed-off-by: Theo Truong # updated docs --------- Signed-off-by: Theo Truong --- DEVELOPER_GUIDE.md | 1 + spec/opensearch-openapi.yaml | 44 ++++++++++ tools/helpers.ts | 7 +- tools/linter/components/RootFile.ts | 18 ++++- tools/merger/OpenApiMerger.ts | 23 +++++- tools/test/linter/RootFile.test.ts | 10 +++ tools/test/linter/fixtures/root.yaml | 22 ++++- tools/test/merger/OpenApiMerger.test.ts | 12 +++ tools/test/merger/fixtures/expected.yaml | 81 +++++++++++++++++++ .../fixtures/spec/namespaces/ignored.yaml | 35 ++++++++ .../fixtures/spec/namespaces/shelter.yaml | 36 +++++++++ .../fixtures/spec/opensearch-openapi.yaml | 18 +++++ .../merger/fixtures/spec/schemas/actions.yaml | 11 +++ .../merger/fixtures/spec/schemas/animals.yaml | 21 +++++ tools/types.ts | 1 + 15 files changed, 332 insertions(+), 8 deletions(-) create mode 100644 tools/test/merger/OpenApiMerger.test.ts create mode 100644 tools/test/merger/fixtures/expected.yaml create mode 100644 tools/test/merger/fixtures/spec/namespaces/ignored.yaml create mode 100644 tools/test/merger/fixtures/spec/namespaces/shelter.yaml create mode 100644 tools/test/merger/fixtures/spec/opensearch-openapi.yaml create mode 100644 tools/test/merger/fixtures/spec/schemas/actions.yaml create mode 100644 tools/test/merger/fixtures/spec/schemas/animals.yaml diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index b14d1805a..2b6a5dbfd 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -81,6 +81,7 @@ This repository includes several penAPI Specification Extensions to fill in any - `x-version-removed`: OpenSearch version when the operation/parameter was removed. - `x-deprecation-message`: Reason for deprecation and guidance on how to prepare for the next major version. - `x-ignorable`: Denotes that the operation should be ignored by the client generator. This is used in operation groups where some operations have been replaced by newer ones, but we still keep them in the specs because the server still supports them. +- `x-global`: Denotes that the parameter is a global parameter that is included in every operation. These parameters are listed in the [root file](spec/opensearch-openapi.yaml). ## Linting We have a linter that validates every `yaml` file in the `./spec` folder to assure that they follow the guidelines we have set. Check out the [Linter](tools/README.md#linter) tool for more information on how to run it locally. Make sure to run the linter before submitting a PR. diff --git a/spec/opensearch-openapi.yaml b/spec/opensearch-openapi.yaml index aa652e149..dffbd314c 100644 --- a/spec/opensearch-openapi.yaml +++ b/spec/opensearch-openapi.yaml @@ -496,3 +496,47 @@ paths: $ref: 'namespaces/tasks.yaml#/paths/~1_tasks~1{task_id}' /_tasks/{task_id}/_cancel: $ref: 'namespaces/tasks.yaml#/paths/~1_tasks~1{task_id}~1_cancel' +components: + parameters: + _global::query.pretty: + x-global: true + name: pretty + in: query + description: Whether to pretty format the returned JSON response. + schema: + type: boolean + default: false + _global::query.human: + x-global: true + name: human + in: query + description: Whether to return human readable values for statistics. + schema: + type: boolean + default: true + _global::query.error_trace: + x-global: true + name: error_trace + in: query + description: Whether to include the stack trace of returned errors. + schema: + type: boolean + default: false + _global::query.source: + x-global: true + name: source + in: query + description: The URL-encoded request definition. Useful for libraries that do not accept a request body for non-POST requests. + schema: + type: string + _global::query.filter_path: + x-global: true + name: filter_path + in: query + description: Comma-separated list of filters used to reduce the response. + schema: + oneOf: + - type: string + - type: array + items: + type: string diff --git a/tools/helpers.ts b/tools/helpers.ts index 2abf90984..336ffb68c 100644 --- a/tools/helpers.ts +++ b/tools/helpers.ts @@ -28,7 +28,7 @@ export function sortByKey(obj: Record, priorities: string[] = []) { } export function write2file(file_path: string, content: Record): void { - fs.writeFileSync(file_path, quoteRefs(YAML.stringify(content, {lineWidth: 0, singleQuote: true}))); + fs.writeFileSync(file_path, quoteRefs(YAML.stringify(removeAnchors(content), {lineWidth: 0, singleQuote: true}))); } function quoteRefs(str: string): string { @@ -39,4 +39,9 @@ function quoteRefs(str: string): string { } return line }).join('\n'); +} + +function removeAnchors(content: Record): Record { + const replacer = (key: string, value: any) => key === '$anchor' ? undefined : value; + return JSON.parse(JSON.stringify(content, replacer)); } \ No newline at end of file diff --git a/tools/linter/components/RootFile.ts b/tools/linter/components/RootFile.ts index 5718ce3d4..5e84bac7a 100644 --- a/tools/linter/components/RootFile.ts +++ b/tools/linter/components/RootFile.ts @@ -1,4 +1,4 @@ -import {ValidationError} from "../../types"; +import {ParameterSpec, ValidationError} from "../../types"; import FileValidator from "./base/FileValidator"; export default class RootFile extends FileValidator { @@ -8,7 +8,10 @@ export default class RootFile extends FileValidator { } validate_file(): ValidationError[] { - return this.validate_paths(); + return [ + this.validate_paths(), + this.validate_params(), + ].flat(); } validate_paths(): ValidationError[] { @@ -17,4 +20,15 @@ export default class RootFile extends FileValidator { return this.error(`Every path must be a reference object to a path in a namespace file.`, `Path: ${path}`); }).filter((e) => e) as ValidationError[]; } + + validate_params(): ValidationError[] { + const params = (this.spec().components?.parameters || {}) as Record; + return Object.entries(params).map(([name, param]) => { + const expected = `_global::${param.in}.${param.name}`; + if(name !== expected) + return this.error(`Parameters in root file must be in the format '_global::{in}.{name}'. Expected '${expected}'.`, `#/components/parameters/${name}`); + if(!param['x-global']) + return this.error(`Parameters in root file must have 'x-global' extension set to true.`, `#/components/parameters/${name}`); + }).filter((e) => e) as ValidationError[]; + } } \ No newline at end of file diff --git a/tools/merger/OpenApiMerger.ts b/tools/merger/OpenApiMerger.ts index 1ecda8929..2e108ce55 100644 --- a/tools/merger/OpenApiMerger.ts +++ b/tools/merger/OpenApiMerger.ts @@ -2,12 +2,14 @@ import { OpenAPIV3 } from "openapi-types"; import fs from 'fs'; import _ from 'lodash'; import yaml from 'yaml'; +import { write2file } from '../helpers'; // Create a single-file OpenAPI spec from multiple files for OpenAPI validation and programmatic consumption export default class OpenApiMerger { root_path: string; root_folder: string; spec: Record; + global_param_refs: OpenAPIV3.ReferenceObject[]; paths: Record> = {}; // namespace -> path -> path_item_object schemas: Record> = {}; // category -> schema -> schema_object @@ -16,23 +18,36 @@ export default class OpenApiMerger { this.root_path = fs.realpathSync(root_path); this.root_folder = this.root_path.split('/').slice(0, -1).join('/'); this.spec = yaml.parse(fs.readFileSync(this.root_path, 'utf8')); + const global_params: OpenAPIV3.ParameterObject = this.spec.components?.parameters || {}; + this.global_param_refs = Object.keys(global_params).map(param => ({$ref: `#/components/parameters/${param}`})); this.spec.components = { - parameters: {}, + parameters: global_params, requestBodies: {}, responses: {}, schemas: {}, }; } - merge(output_path: string | null): OpenAPIV3.Document { - this.#merge_namespaces(); + merge(output_path?: string): OpenAPIV3.Document { this.#merge_schemas(); + this.#merge_namespaces(); + this.#apply_global_params(); this.#sort_spec_keys(); - if(output_path) fs.writeFileSync(output_path, yaml.stringify(this.spec, {lineWidth: 0, singleQuote: true})) + if(output_path) write2file(output_path, this.spec); return this.spec as OpenAPIV3.Document; } + // Apply global parameters to all operations in the spec. + #apply_global_params(): void { + Object.entries(this.spec.paths).forEach(([path, pathItem]) => { + Object.entries(pathItem!).forEach(([method, operation]) => { + const params = operation.parameters || []; + operation.parameters = [...params, ...Object.values(this.global_param_refs)]; + }); + }); + } + // Merge files from /namespaces folder. #merge_namespaces(): void { const folder = `${this.root_folder}/namespaces`; diff --git a/tools/test/linter/RootFile.test.ts b/tools/test/linter/RootFile.test.ts index 89749b20f..e27923f69 100644 --- a/tools/test/linter/RootFile.test.ts +++ b/tools/test/linter/RootFile.test.ts @@ -12,6 +12,16 @@ test('validate()', () => { file: "root.yaml", location: "Path: /{index}", message: "Every path must be a reference object to a path in a namespace file." + }, + { + file: "root.yaml", + location: "#/components/parameters/_global::query.pretty", + message: "Parameters in root file must be in the format '_global::{in}.{name}'. Expected '_global::query.beautify'." + }, + { + file: "root.yaml", + location: "#/components/parameters/_global::query.human", + message: "Parameters in root file must have 'x-global' extension set to true." } ]); }); \ No newline at end of file diff --git a/tools/test/linter/fixtures/root.yaml b/tools/test/linter/fixtures/root.yaml index 954d5904a..8c4a08750 100644 --- a/tools/test/linter/fixtures/root.yaml +++ b/tools/test/linter/fixtures/root.yaml @@ -9,4 +9,24 @@ paths: $ref: '#/components/responses/indices.create@200' parameters: - $ref: '#/components/parameters/indices.create::path.index' - - $ref: '#/components/parameters/indices.create::query.pretty' \ No newline at end of file + - $ref: '#/components/parameters/indices.create::query.pretty' +components: + parameters: + _global::query.pretty: + x-global: true + name: beautify + in: query + schema: + type: boolean + _global::query.human: + x-global: false + name: human + in: query + schema: + type: boolean + _global::query.error_trace: + x-global: true + name: error_trace + in: query + schema: + type: boolean \ No newline at end of file diff --git a/tools/test/merger/OpenApiMerger.test.ts b/tools/test/merger/OpenApiMerger.test.ts new file mode 100644 index 000000000..32aec55e8 --- /dev/null +++ b/tools/test/merger/OpenApiMerger.test.ts @@ -0,0 +1,12 @@ +import OpenApiMerger from '../../merger/OpenApiMerger'; +import fs from 'fs'; +import yaml from 'yaml'; + + +test('merge()', async () => { + const merger = new OpenApiMerger('./test/merger/fixtures/spec/opensearch-openapi.yaml'); + merger.merge('./test/merger/opensearch-openapi.yaml'); + expect(fs.readFileSync('./test/merger/fixtures/expected.yaml', 'utf8')) + .toEqual(fs.readFileSync('./test/merger/opensearch-openapi.yaml', 'utf8')); + fs.unlinkSync('./test/merger/opensearch-openapi.yaml'); +}); \ No newline at end of file diff --git a/tools/test/merger/fixtures/expected.yaml b/tools/test/merger/fixtures/expected.yaml new file mode 100644 index 000000000..1d28db66d --- /dev/null +++ b/tools/test/merger/fixtures/expected.yaml @@ -0,0 +1,81 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + /adopt/{animal}: + get: + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/_global::query.human' + responses: + '200': + $ref: '#/components/responses/adopt@200' + post: + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/_global::query.human' + requestBody: + $ref: '#/components/requestBodies/adopt' + responses: + '200': + $ref: '#/components/responses/adopt@200' +components: + parameters: + _global::query.human: + x-global: true + name: human + in: query + description: Whether to return human readable values for statistics. + schema: + type: boolean + default: true + adopt::path.animal: + name: animal + in: path + schema: + $ref: '#/components/schemas/animals:Animal' + indices.create::path.index: + name: index + in: path + schema: + type: string + indices.create::query.pretty: + name: pretty + in: query + schema: + type: boolean + requestBodies: + adopt: {} + indices.create: {} + responses: + adopt@200: + description: '' + application/json: + schema: + type: object + indices.create@200: + description: '' + application/json: + schema: + type: object + schemas: + actions:Bark: + type: string + actions:Meow: + type: string + animals:Animal: + oneOf: + - $ref: '#/components/schemas/animals:Dog' + - $ref: '#/components/schemas/animals:Cat' + animals:Cat: + type: object + properties: + meow: + $ref: '#/components/schemas/actions:Meow' + animals:Dog: + type: object + properties: + bark: + $ref: '#/components/schemas/actions:Bark' diff --git a/tools/test/merger/fixtures/spec/namespaces/ignored.yaml b/tools/test/merger/fixtures/spec/namespaces/ignored.yaml new file mode 100644 index 000000000..f8a7688cd --- /dev/null +++ b/tools/test/merger/fixtures/spec/namespaces/ignored.yaml @@ -0,0 +1,35 @@ +openapi: 3.1.0 +info: + title: Ignored + version: 1.0.0 +paths: + '/{index}': + post: + parameters: + - $ref: '#/components/parameters/indices.create::path.index' + - $ref: '#/components/parameters/indices.create::query.pretty' + requestBody: + $ref: '#/components/requestBodies/indices.create' + responses: + '200': + $ref: '#/components/responses/indices.create@200' +components: + requestBodies: + indices.create: {} + parameters: + indices.create::path.index: + name: index + in: path + schema: + type: string + indices.create::query.pretty: + name: pretty + in: query + schema: + type: boolean + responses: + indices.create@200: + description: '' + application/json: + schema: + type: object \ No newline at end of file diff --git a/tools/test/merger/fixtures/spec/namespaces/shelter.yaml b/tools/test/merger/fixtures/spec/namespaces/shelter.yaml new file mode 100644 index 000000000..b91dc827a --- /dev/null +++ b/tools/test/merger/fixtures/spec/namespaces/shelter.yaml @@ -0,0 +1,36 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + '/adopt/{animal}': + get: + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + responses: + '200': + $ref: '#/components/responses/adopt@200' + post: + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + requestBody: + $ref: '#/components/requestBodies/adopt' + responses: + '200': + $ref: '#/components/responses/adopt@200' +components: + requestBodies: + adopt: {} + parameters: + adopt::path.animal: + name: animal + in: path + schema: + $ref: '../schemas/animals.yaml#/components/schemas/Animal' + responses: + adopt@200: + description: '' + application/json: + schema: + type: object \ No newline at end of file diff --git a/tools/test/merger/fixtures/spec/opensearch-openapi.yaml b/tools/test/merger/fixtures/spec/opensearch-openapi.yaml new file mode 100644 index 000000000..1e9a06617 --- /dev/null +++ b/tools/test/merger/fixtures/spec/opensearch-openapi.yaml @@ -0,0 +1,18 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + '/adopt/{animal}': + $ref: 'namespaces/shelter.yaml#/paths/~1adopt~1{animal}' +components: + parameters: + _global::query.human: + x-global: true + name: human + in: query + description: Whether to return human readable values for statistics. + schema: + type: boolean + default: true \ No newline at end of file diff --git a/tools/test/merger/fixtures/spec/schemas/actions.yaml b/tools/test/merger/fixtures/spec/schemas/actions.yaml new file mode 100644 index 000000000..89bec0877 --- /dev/null +++ b/tools/test/merger/fixtures/spec/schemas/actions.yaml @@ -0,0 +1,11 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +components: + schemas: + Bark: + type: string + Meow: + type: string \ No newline at end of file diff --git a/tools/test/merger/fixtures/spec/schemas/animals.yaml b/tools/test/merger/fixtures/spec/schemas/animals.yaml new file mode 100644 index 000000000..bdb82a16f --- /dev/null +++ b/tools/test/merger/fixtures/spec/schemas/animals.yaml @@ -0,0 +1,21 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +components: + schemas: + Animal: + oneOf: + - $ref: '#/components/schemas/Dog' + - $ref: '#/components/schemas/Cat' + Dog: + type: object + properties: + bark: + $ref: 'actions.yaml#/components/schemas/Bark' + Cat: + type: object + properties: + meow: + $ref: 'actions.yaml#/components/schemas/Meow' \ No newline at end of file diff --git a/tools/types.ts b/tools/types.ts index 04ece7d77..dfd485de8 100644 --- a/tools/types.ts +++ b/tools/types.ts @@ -18,6 +18,7 @@ export interface ParameterSpec extends OpenAPIV3.ParameterObject { 'x-data-type'?: string; 'x-version-deprecated'?: string; 'x-deprecation-message'?: string; + 'x-global'?: boolean; } export interface ValidationError {