From 5188911d044503c22f2f143bf52e463c7e9fd438 Mon Sep 17 00:00:00 2001 From: Theo Truong Date: Mon, 15 Apr 2024 19:26:21 -0600 Subject: [PATCH] # added validation for global spec Signed-off-by: Theo Truong # updated docs --- DEVELOPER_GUIDE.md | 1 + tools/helpers.ts | 7 +++++- tools/linter/components/RootFile.ts | 18 +++++++++++++-- tools/merger/OpenApiMerger.ts | 21 +++++++++++++++--- tools/test/linter/RootFile.test.ts | 10 +++++++++ tools/test/linter/fixtures/root.yaml | 22 ++++++++++++++++++- tools/test/merger/fixtures/expected.yaml | 8 +++++++ .../fixtures/spec/namespaces/shelter.yaml | 6 +++++ tools/types.ts | 1 + 9 files changed, 87 insertions(+), 7 deletions(-) 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/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 2a075e1ed..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,8 +18,10 @@ 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: this.spec.components?.parameters || {}, + parameters: global_params, requestBodies: {}, responses: {}, schemas: {}, @@ -25,14 +29,25 @@ export default class OpenApiMerger { } merge(output_path?: string): OpenAPIV3.Document { - this.#merge_namespaces(); 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/fixtures/expected.yaml b/tools/test/merger/fixtures/expected.yaml index c37945123..1d28db66d 100644 --- a/tools/test/merger/fixtures/expected.yaml +++ b/tools/test/merger/fixtures/expected.yaml @@ -5,9 +5,17 @@ info: 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: diff --git a/tools/test/merger/fixtures/spec/namespaces/shelter.yaml b/tools/test/merger/fixtures/spec/namespaces/shelter.yaml index 3d85cd359..b91dc827a 100644 --- a/tools/test/merger/fixtures/spec/namespaces/shelter.yaml +++ b/tools/test/merger/fixtures/spec/namespaces/shelter.yaml @@ -5,6 +5,12 @@ info: 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' 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 {