Global Params (#241)

* Added tests for merger Signed-off-by: Theo Truong <[email protected]> * # checking if build step was successful Signed-off-by: Theo Truong <[email protected]> * # removed diagnostic code Signed-off-by: Theo Truong <[email protected]> * # added validation for global spec Signed-off-by: Theo Truong <[email protected]> # updated docs --------- Signed-off-by: Theo Truong <[email protected]>
opensearch-project · Apr 16, 2024 · 4299a19 · 4299a19
1 parent 198473d
commit 4299a19
Show file tree

Hide file tree

Showing 15 changed files with 332 additions and 8 deletions.
diff --git 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
@@ -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
@@ -28,7 +28,7 @@ export function sortByKey(obj: Record<string, any>, priorities: string[] = []) {
 }
 
 export function write2file(file_path: string, content: Record<string, any>): 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<string, any>): Record<string, any> {
+    const replacer = (key: string, value: any) => key === '$anchor' ? undefined : value;
+    return JSON.parse(JSON.stringify(content, replacer));
 }
diff --git 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<string, ParameterSpec>;
+        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[];
+    }
 }
diff --git 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<string, any>;
+    global_param_refs: OpenAPIV3.ReferenceObject[];
 
     paths: Record<string, Record<string, OpenAPIV3.PathItemObject>> = {}; // namespace -> path -> path_item_object
     schemas: Record<string, Record<string, OpenAPIV3.SchemaObject>> = {}; // 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 <spec_root>/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
@@ -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."
         }
     ]);
 });
diff --git 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'
+        - $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
diff --git a/tools/test/merger/OpenApiMerger.test.ts 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');
+});
diff --git a/tools/test/merger/fixtures/expected.yaml 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
@@ -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
diff --git a/tools/test/merger/fixtures/spec/namespaces/shelter.yaml 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