From f3277f791ceea53e82307a914e73d4eeaf87f789 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 16 Jul 2024 17:17:52 -0400 Subject: [PATCH 01/13] Delete schema elements that don't match target OpenSearch version. Signed-off-by: dblock --- .github/workflows/test-spec.yml | 5 +- CHANGELOG.md | 1 + DEVELOPER_GUIDE.md | 7 + tools/src/linter/SchemasValidator.ts | 3 +- tools/src/merger/OpenApiMerger.ts | 131 ++++++++++++++---- tools/src/merger/merge.ts | 7 +- tools/src/tester/MergedOpenApiSpec.ts | 8 +- tools/src/tester/test.ts | 3 +- tools/tests/merger/OpenApiMerger.test.ts | 59 ++++++-- .../{expected.yaml => expected_1.3.yaml} | 5 + tools/tests/merger/fixtures/expected_2.0.yaml | 128 +++++++++++++++++ .../fixtures/spec/namespaces/indices.yaml | 15 ++ tools/tests/merger/opensearch-openapi.yaml | 118 ++++++++++++++++ tools/tests/tester/MergedOpenApiSpec.test.ts | 95 +++++++++---- .../specs/complete/namespaces/index.yaml | 21 +++ 15 files changed, 535 insertions(+), 71 deletions(-) rename tools/tests/merger/fixtures/{expected.yaml => expected_1.3.yaml} (96%) create mode 100644 tools/tests/merger/fixtures/expected_2.0.yaml create mode 100644 tools/tests/merger/opensearch-openapi.yaml diff --git a/.github/workflows/test-spec.yml b/.github/workflows/test-spec.yml index 809c1a63f..80640fe05 100644 --- a/.github/workflows/test-spec.yml +++ b/.github/workflows/test-spec.yml @@ -55,7 +55,10 @@ jobs: - name: Run Tests run: | - npm run test:spec -- --opensearch-insecure --coverage coverage/test-spec-coverage-${{ matrix.entry.version }}.json + npm run test:spec -- \ + --opensearch-insecure \ + --opensearch-version=${{ matrix.entry.version }} \ + --coverage coverage/test-spec-coverage-${{ matrix.entry.version }}.json - name: Upload Test Coverage Results uses: actions/upload-artifact@v4 diff --git a/CHANGELOG.md b/CHANGELOG.md index d6563e391..19de5fc50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added missing variants of `indices.put_alias` ([#434](https://github.com/opensearch-project/opensearch-api-specification/pull/434)) - Added `plugins` to NodeInfoSettings ([#442](https://github.com/opensearch-project/opensearch-api-specification/pull/442)) - Added test coverage ([#443](https://github.com/opensearch-project/opensearch-api-specification/pull/443)) +- Added `--opensearch-version` to `merger` that excludes schema elements per semver ([#428](https://github.com/opensearch-project/opensearch-api-specification/pull/428)) ### Changed diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index e941171c5..b965e50a1 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -173,6 +173,7 @@ The merger tool merges the multi-file OpenSearch spec into a single file for pro - `--source `: The path to the root folder of the multi-file spec, defaults to `/spec`. - `--output `: The path to write the final merged spec to, defaults to `/build/opensearch-openapi.yaml`. +- `--opensearch-version`: An optional target version of OpenSearch, checking values of `x-version-added` and `x-version-removed`. #### Example @@ -181,6 +182,12 @@ We can take advantage of the default values and simply merge the specification v npm run merge ``` +To generate a spec that does not contain any APIs or fields removed in version 2.0 (e.g. document `_type` fields). + +```bash +npm run merge -- --opensearch-version=2.0 +``` + ### [Spec Linter](tools/src/linter) ```bash diff --git a/tools/src/linter/SchemasValidator.ts b/tools/src/linter/SchemasValidator.ts index 5192595c7..6954d68ab 100644 --- a/tools/src/linter/SchemasValidator.ts +++ b/tools/src/linter/SchemasValidator.ts @@ -32,7 +32,8 @@ export default class SchemasValidator { } validate (): ValidationError[] { - this.spec = new OpenApiMerger(this.root_folder, new Logger(LogLevel.error)).merge().components as Record + const merger = new OpenApiMerger(this.root_folder, undefined, new Logger(LogLevel.error)) + this.spec = merger.merge().spec().components as Record const named_schemas_errors = this.validate_named_schemas() if (named_schemas_errors.length > 0) return named_schemas_errors return [ diff --git a/tools/src/merger/OpenApiMerger.ts b/tools/src/merger/OpenApiMerger.ts index 51907863d..a0f16a5b8 100644 --- a/tools/src/merger/OpenApiMerger.ts +++ b/tools/src/merger/OpenApiMerger.ts @@ -9,25 +9,30 @@ import { type OpenAPIV3 } from 'openapi-types' import fs from 'fs' -import _ from 'lodash' +import _, { isEmpty } from 'lodash' import { read_yaml, write_yaml } from '../helpers' import SupersededOpsGenerator from './SupersededOpsGenerator' import GlobalParamsGenerator from './GlobalParamsGenerator' import { Logger } from '../Logger' +import * as semver from 'semver' // Create a single-file OpenAPI spec from multiple files for OpenAPI validation and programmatic consumption export default class OpenApiMerger { root_folder: string - spec: Record logger: Logger + target_version?: string + + protected _spec: Record + protected _merged: boolean = false paths: Record> = {} // namespace -> path -> path_item_object schemas: Record> = {} // category -> schema -> schema_object - constructor (root_folder: string, logger: Logger = new Logger()) { + constructor (root_folder: string, target_version?: string, logger: Logger = new Logger()) { this.logger = logger this.root_folder = fs.realpathSync(root_folder) - this.spec = { + this.target_version = target_version === undefined ? undefined : semver.coerce(target_version)?.toString() + this._spec = { openapi: '3.1.0', info: read_yaml(`${this.root_folder}/_info.yaml`, true), paths: {}, @@ -40,17 +45,27 @@ export default class OpenApiMerger { } } - merge (output_path: string = ''): OpenAPIV3.Document { + write_to(output_path: string): OpenApiMerger { + if (!this._merged) { throw "Call merge() first." } + this.logger.info(`Writing ${output_path} ...`) + write_yaml(output_path, this._spec) + return this + } + + merge (): OpenApiMerger { + if (this._merged) { throw "Spec already merged." } this.#merge_schemas() this.#merge_namespaces() this.#sort_spec_keys() this.#generate_global_params() this.#generate_superseded_ops() + this._merged = true + return this + } - this.logger.info(`Writing ${output_path} ...`) - - if (output_path !== '') write_yaml(output_path, this.spec) - return this.spec as OpenAPIV3.Document + spec(): OpenAPIV3.Document { + if (!this._merged) { this.merge() } + return this._spec as OpenAPIV3.Document } // Merge files from /namespaces folder. @@ -59,21 +74,83 @@ export default class OpenApiMerger { fs.readdirSync(folder).forEach(file => { this.logger.info(`Merging namespaces in ${folder}/${file} ...`) const spec = read_yaml(`${folder}/${file}`) - this.redirect_refs_in_namespace(spec) - this.spec.paths = { ...this.spec.paths, ...spec.paths } - this.spec.components.parameters = { ...this.spec.components.parameters, ...spec.components.parameters } - this.spec.components.responses = { ...this.spec.components.responses, ...spec.components.responses } - this.spec.components.requestBodies = { ...this.spec.components.requestBodies, ...spec.components.requestBodies } + this.#redirect_refs_in_namespace(spec) + this._spec.paths = { ...this._spec.paths, ...spec.paths } + this._spec.components.parameters = { ...this._spec.components.parameters, ...spec.components.parameters } + this._spec.components.responses = { ...this._spec.components.responses, ...spec.components.responses } + this._spec.components.requestBodies = { ...this._spec.components.requestBodies, ...spec.components.requestBodies } + }) + + this.#remove_refs_per_semver() + } + + // Remove any refs that are x-version-added/removed incompatible with the target server version. + #remove_refs_per_semver() : void { + this.#remove_per_semver(this._spec.paths) + + // parameters + const removed_params = this.#remove_per_semver(this._spec.components.parameters) + const removed_parameter_refs = _.map(removed_params, (ref) => `#/components/parameters/${ref}`) + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + method_item.parameters = _.filter(method_item.parameters, (param) => !_.includes(removed_parameter_refs, param.$ref)) + }) + }) + + // responses + const removed_responses = this.#remove_per_semver(this._spec.components.responses) + const removed_response_refs = _.map(removed_responses, (ref) => `#/components/responses/${ref}`) + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + method_item.responses = _.omitBy(method_item.responses, (param) => _.includes(removed_response_refs, param.$ref)) + }) }) + + this._spec.paths = _.omitBy(this._spec.paths, isEmpty) + } + + #exclude_per_semver(obj: any): boolean { + if (this.target_version === undefined) return false + + const x_version_added = semver.coerce(obj['x-version-added'] as string) + const x_version_removed = semver.coerce(obj['x-version-removed'] as string) + + if (x_version_added && !semver.satisfies(this.target_version, `>=${x_version_added?.toString()}`)) { + return true + } else if (x_version_removed && !semver.satisfies(this.target_version, `<${x_version_removed?.toString()}`)) { + return true + } + + return false + } + + // Remove any elements that are x-version-added/removed incompatible with the target server version. + #remove_per_semver(obj: any): string[] { + let removed: string[] = [] + if (this.target_version === undefined) return removed + + for (const key in obj) { + if (_.isObject(obj[key])) { + if (this.#exclude_per_semver(obj[key])) { + removed.push(key) + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete obj[key] + } else { + removed = _.concat(removed, this.#remove_per_semver(obj[key])) + } + } + } + + return removed } // Redirect schema references in namespace files to local references in single-file spec. - redirect_refs_in_namespace (obj: any): void { + #redirect_refs_in_namespace (obj: any): void { const ref: string = obj?.$ref if (ref?.startsWith('../schemas/')) { obj.$ref = ref.replace('../schemas/', '#/components/schemas/').replace('.yaml#/components/schemas/', ':') } for (const key in obj) { - if (typeof obj[key] === 'object') { this.redirect_refs_in_namespace(obj[key]) } + if (typeof obj[key] === 'object') { this.#redirect_refs_in_namespace(obj[key]) } } } @@ -92,7 +169,7 @@ export default class OpenApiMerger { Object.entries(this.schemas).forEach(([category, schemas]) => { Object.entries(schemas).forEach(([name, schema_obj]) => { - this.spec.components.schemas[`${category}:${name}`] = schema_obj + this._spec.components.schemas[`${category}:${name}`] = schema_obj }) }) } @@ -115,26 +192,26 @@ export default class OpenApiMerger { // Sort keys in the spec to make it easier to read and compare. #sort_spec_keys (): void { - this.spec.components.schemas = _.fromPairs(Object.entries(this.spec.components.schemas as Document).sort((a, b) => a[0].localeCompare(b[0]))) - this.spec.components.parameters = _.fromPairs(Object.entries(this.spec.components.parameters as Document).sort((a, b) => a[0].localeCompare(b[0]))) - this.spec.components.responses = _.fromPairs(Object.entries(this.spec.components.responses as Document).sort((a, b) => a[0].localeCompare(b[0]))) - this.spec.components.requestBodies = _.fromPairs(Object.entries(this.spec.components.requestBodies as Document).sort((a, b) => a[0].localeCompare(b[0]))) - - this.spec.paths = _.fromPairs(Object.entries(this.spec.paths as Document).sort((a, b) => a[0].localeCompare(b[0]))) - Object.entries(this.spec.paths as Document).forEach(([path, path_item]) => { - this.spec.paths[path] = _.fromPairs(Object.entries(path_item as Document).sort((a, b) => a[0].localeCompare(b[0]))) + this._spec.components.schemas = _.fromPairs(Object.entries(this._spec.components.schemas as Document).sort((a, b) => a[0].localeCompare(b[0]))) + this._spec.components.parameters = _.fromPairs(Object.entries(this._spec.components.parameters as Document).sort((a, b) => a[0].localeCompare(b[0]))) + this._spec.components.responses = _.fromPairs(Object.entries(this._spec.components.responses as Document).sort((a, b) => a[0].localeCompare(b[0]))) + this._spec.components.requestBodies = _.fromPairs(Object.entries(this._spec.components.requestBodies as Document).sort((a, b) => a[0].localeCompare(b[0]))) + + this._spec.paths = _.fromPairs(Object.entries(this._spec.paths as Document).sort((a, b) => a[0].localeCompare(b[0]))) + Object.entries(this._spec.paths as Document).forEach(([path, path_item]) => { + this._spec.paths[path] = _.fromPairs(Object.entries(path_item as Document).sort((a, b) => a[0].localeCompare(b[0]))) }) } // Generate global parameters from _global_params.yaml file. #generate_global_params (): void { const gen = new GlobalParamsGenerator(this.root_folder) - gen.generate(this.spec) + gen.generate(this._spec) } // Generate superseded operations from _superseded_operations.yaml file. #generate_superseded_ops (): void { const gen = new SupersededOpsGenerator(this.root_folder, this.logger) - gen.generate(this.spec) + gen.generate(this._spec) } } diff --git a/tools/src/merger/merge.ts b/tools/src/merger/merge.ts index 50af3360b..0ad3d4eed 100644 --- a/tools/src/merger/merge.ts +++ b/tools/src/merger/merge.ts @@ -17,12 +17,13 @@ const command = new Command() .addOption(new Option('-s, --source ', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec'))) .addOption(new Option('-o, --output ', 'output file name').default(resolve(__dirname, '../../../build/opensearch-openapi.yaml'))) .addOption(new Option('--verbose', 'show merge details').default(false)) + .addOption(new Option('--opensearch-version ', 'target OpenSearch schema version').default(undefined)) .allowExcessArguments(false) .parse() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const merger = new OpenApiMerger(opts.source, logger) -logger.log(`Merging ${opts.source} into ${opts.output} ...`) -merger.merge(opts.output) +const merger = new OpenApiMerger(opts.source, opts.opensearchVersion, logger) +logger.log(`Merging ${opts.source} into ${opts.output} (${opts.opensearchVersion}) ...`) +merger.merge().write_to(opts.output) logger.log('Done.') diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index 56ee8b7a9..1b4cb11a6 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -18,16 +18,20 @@ import _ from 'lodash'; export default class MergedOpenApiSpec { logger: Logger file_path: string + target_version?: string + protected _spec: OpenAPIV3.Document | undefined - constructor (spec_path: string, logger: Logger = new Logger()) { + constructor (spec_path: string, target_version?: string, logger: Logger = new Logger()) { this.logger = logger this.file_path = spec_path + this.target_version = target_version } spec (): OpenAPIV3.Document { if (this._spec) return this._spec - const spec = (new OpenApiMerger(this.file_path, this.logger)).merge() + const merger = new OpenApiMerger(this.file_path, this.target_version, this.logger) + const spec = merger.merge().spec() const ctx = new SpecificationContext(this.file_path) this.inject_additional_properties(ctx, spec) this._spec = spec diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 5c55a5446..98a875e09 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -41,6 +41,7 @@ const command = new Command() ) .addOption(new Option('--verbose', 'whether to print the full stack trace of errors').default(false)) .addOption(new Option('--dry-run', 'dry run only, do not make HTTP requests').default(false)) + .addOption(new Option('--opensearch-version ', 'target OpenSearch schema version').default(undefined)) .addOption(OPENSEARCH_URL_OPTION) .addOption(OPENSEARCH_USERNAME_OPTION) .addOption(OPENSEARCH_PASSWORD_OPTION) @@ -52,7 +53,7 @@ const command = new Command() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const spec = new MergedOpenApiSpec(opts.specPath, new Logger(LogLevel.error)) +const spec = new MergedOpenApiSpec(opts.specPath, opts.opensearchVersion, new Logger(LogLevel.error)) const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli({ opensearchResponseType: 'arraybuffer', ...opts })) const chapter_reader = new ChapterReader(http_client, logger) const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec.spec()), chapter_reader, new SchemaValidator(spec.spec(), logger), logger) diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index 72eeef042..1169b2101 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -9,12 +9,55 @@ import OpenApiMerger from 'merger/OpenApiMerger' import fs from 'fs' -import { Logger, LogLevel } from 'Logger' - -test('merge()', () => { - const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', new Logger(LogLevel.error)) - merger.merge('./tools/tests/merger/opensearch-openapi.yaml') - expect(fs.readFileSync('./tools/tests/merger/fixtures/expected.yaml', 'utf8')) - .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) - fs.unlinkSync('./tools/tests/merger/opensearch-openapi.yaml') + +describe('OpenApiMerger', () => { + var merger: OpenApiMerger + + describe('defaults', () => { + beforeEach(() => { + merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/') + }) + + describe('merge()', () => { + test('is not required', () => { + expect(merger.spec()).toBeDefined() + }) + + test('merges spec', () => { + merger.merge() + expect(merger.spec()).toBeDefined() + }) + + test('raises an error when called twice', () => { + merger.merge() + expect(() => { + merger.merge() + }).toThrow('Spec already merged.'); + }) + }) + + describe('write_to()', () => { + afterAll(() => { + fs.unlinkSync('./tools/tests/merger/opensearch-openapi.yaml') + }) + + test('writes a spec', () => { + merger.merge().write_to('./tools/tests/merger/opensearch-openapi.yaml') + expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_2.0.yaml', 'utf8')) + .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) + }) + }) + }) + + describe('1.3', () => { + beforeEach(() => { + merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', '1.3') + }) + + test('writes a spec', () => { + merger.merge().write_to('./tools/tests/merger/opensearch-openapi.yaml') + expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_1.3.yaml', 'utf8')) + .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) + }) + }) }) diff --git a/tools/tests/merger/fixtures/expected.yaml b/tools/tests/merger/fixtures/expected_1.3.yaml similarity index 96% rename from tools/tests/merger/fixtures/expected.yaml rename to tools/tests/merger/fixtures/expected_1.3.yaml index 3c73f80a1..d75111c45 100644 --- a/tools/tests/merger/fixtures/expected.yaml +++ b/tools/tests/merger/fixtures/expected_1.3.yaml @@ -92,6 +92,11 @@ components: application/json: schema: type: object + indices.create@201: + description: Added in 2.0. + application/json: + schema: + type: object schemas: actions:Bark: type: string diff --git a/tools/tests/merger/fixtures/expected_2.0.yaml b/tools/tests/merger/fixtures/expected_2.0.yaml new file mode 100644 index 000000000..3b547b720 --- /dev/null +++ b/tools/tests/merger/fixtures/expected_2.0.yaml @@ -0,0 +1,128 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + /{index}: + post: + parameters: + - $ref: '#/components/parameters/indices.create::path.index' + - $ref: '#/components/parameters/indices.create::query.pretty' + - $ref: '#/components/parameters/indices.create::query.wait_for_active_shards' + - $ref: '#/components/parameters/_global::query.human' + requestBody: + $ref: '#/components/requestBodies/indices.create' + responses: + '200': + $ref: '#/components/responses/indices.create@200' + '201': + $ref: '#/components/responses/indices.create@201' + x-version-added: '2.0' + /adopt/{animal}/dockets/{docket}: + get: + operationId: adopt.0 + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + responses: + '200': + $ref: '#/components/responses/adopt@200' + post: + operationId: adopt.1 + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + requestBody: + $ref: '#/components/requestBodies/adopt' + responses: + '200': + $ref: '#/components/responses/adopt@200' + /replaced/adopting/{animal}/something/{docket}: + get: + operationId: adopt.0_superseded + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + responses: + '200': + $ref: '#/components/responses/adopt@200' + deprecated: true + x-ignorable: true +components: + parameters: + _global::query.human: + name: human + in: query + description: Whether to return human readable values for statistics. + schema: + type: boolean + default: true + x-global: true + adopt::path.animal: + name: animal + in: path + schema: + $ref: '#/components/schemas/animals:Animal' + adopt::path.docket: + name: docket + in: path + schema: + type: number + indices.create::path.index: + name: index + in: path + schema: + type: string + indices.create::query.pretty: + name: pretty + in: query + schema: + type: boolean + indices.create::query.wait_for_active_shards: + name: pretty + in: query + x-version-added: '2.0' + 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 + indices.create@201: + description: Added in 2.0. + 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/tests/merger/fixtures/spec/namespaces/indices.yaml b/tools/tests/merger/fixtures/spec/namespaces/indices.yaml index 966038f3f..0dafa08e7 100644 --- a/tools/tests/merger/fixtures/spec/namespaces/indices.yaml +++ b/tools/tests/merger/fixtures/spec/namespaces/indices.yaml @@ -8,11 +8,15 @@ paths: parameters: - $ref: '#/components/parameters/indices.create::path.index' - $ref: '#/components/parameters/indices.create::query.pretty' + - $ref: '#/components/parameters/indices.create::query.wait_for_active_shards' requestBody: $ref: '#/components/requestBodies/indices.create' responses: '200': $ref: '#/components/responses/indices.create@200' + '201': + $ref: '#/components/responses/indices.create@201' + x-version-added: '2.0' components: requestBodies: indices.create: {} @@ -27,9 +31,20 @@ components: in: query schema: type: boolean + indices.create::query.wait_for_active_shards: + name: pretty + in: query + x-version-added: '2.0' + schema: + type: boolean responses: indices.create@200: description: '' + application/json: + schema: + type: object + indices.create@201: + description: Added in 2.0. application/json: schema: type: object \ No newline at end of file diff --git a/tools/tests/merger/opensearch-openapi.yaml b/tools/tests/merger/opensearch-openapi.yaml new file mode 100644 index 000000000..d75111c45 --- /dev/null +++ b/tools/tests/merger/opensearch-openapi.yaml @@ -0,0 +1,118 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + /{index}: + post: + parameters: + - $ref: '#/components/parameters/indices.create::path.index' + - $ref: '#/components/parameters/indices.create::query.pretty' + - $ref: '#/components/parameters/_global::query.human' + requestBody: + $ref: '#/components/requestBodies/indices.create' + responses: + '200': + $ref: '#/components/responses/indices.create@200' + /adopt/{animal}/dockets/{docket}: + get: + operationId: adopt.0 + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + responses: + '200': + $ref: '#/components/responses/adopt@200' + post: + operationId: adopt.1 + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + requestBody: + $ref: '#/components/requestBodies/adopt' + responses: + '200': + $ref: '#/components/responses/adopt@200' + /replaced/adopting/{animal}/something/{docket}: + get: + operationId: adopt.0_superseded + parameters: + - $ref: '#/components/parameters/adopt::path.animal' + - $ref: '#/components/parameters/adopt::path.docket' + - $ref: '#/components/parameters/_global::query.human' + responses: + '200': + $ref: '#/components/responses/adopt@200' + deprecated: true + x-ignorable: true +components: + parameters: + _global::query.human: + name: human + in: query + description: Whether to return human readable values for statistics. + schema: + type: boolean + default: true + x-global: true + adopt::path.animal: + name: animal + in: path + schema: + $ref: '#/components/schemas/animals:Animal' + adopt::path.docket: + name: docket + in: path + schema: + type: number + 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 + indices.create@201: + description: Added in 2.0. + 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/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index 2c656082d..639f4ce8a 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -8,49 +8,88 @@ */ import { Logger } from 'Logger' +import _ from 'lodash' import MergedOpenApiSpec from "tester/MergedOpenApiSpec" describe('merged API spec', () => { - const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger()) + describe('defaults', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', undefined, new Logger()) + + test('has an api version', () => { + expect(spec.api_version()).toEqual('1.2.3') + }) - test('has an api version', () => { - expect(spec.api_version()).toEqual('1.2.3') - }) + test('paths', () => { + expect(spec.paths()).toEqual({ + '/_nodes/{id}': ['get', 'post'], + '/index': ['get'], + '/nodes': ['get'] + }) + }) + + test('has all responses', () => { + expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' + ]) + }) + + describe('unevaluatedProperties', () => { + const responses: any = spec.spec().components?.responses + + test('is added with required fields', () => { + const schema = responses['info@200'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + }) + + test('is added when no required fields', () => { + const schema = responses['info@500'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + }) - test('paths', () => { - expect(spec.paths()).toEqual({ - '/_nodes/{id}': ['get', 'post'], - '/index': ['get'], - '/nodes': ['get'] + test('is not added to empty object schema', () => { + const schema = responses['info@503'].content['application/json'].schema + expect(schema.unevaluatedProperties).toBeUndefined() + }) + + test('is not added when true', () => { + const schema = responses['info@201'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual(true) + }) + + test('is not added when object', () => { + const schema = responses['info@404'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ type: 'object' }) + }) }) }) - describe('unevaluatedProperties', () => { - const responses: any = spec.spec().components?.responses + describe('1.3', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', '1.3', new Logger()) - test('is added with required fields', () => { - const schema = responses['info@200'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + test('has matching responses', () => { + expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'removed-2.0', 'added-1.3-removed-2.0' + ]) }) + }) - test('is added when no required fields', () => { - const schema = responses['info@500'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) - }) + describe('2.0', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', '2.0', new Logger()) - test('is not added to empty object schema', () => { - const schema = responses['info@503'].content['application/json'].schema - expect(schema.unevaluatedProperties).toBeUndefined() + test('has matching responses', () => { + expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0' + ]) }) + }) - test('is not added when true', () => { - const schema = responses['info@201'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual(true) - }) + describe('2.1', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', '2.1', new Logger()) - test('is not added when object', () => { - const schema = responses['info@404'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ type: 'object' }) + test('has matching responses', () => { + expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0', 'added-2.1' + ]) }) }) }) diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml index 5cf78308e..141ca5ac5 100644 --- a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml @@ -14,6 +14,16 @@ paths: $ref: '#/components/responses/info@201' '404': $ref: '#/components/responses/info@404' + added-2.0: + $ref: '#/components/responses/info@added-2.0' + x-version-added: '2.0' + removed-2.0: + $ref: '#/components/responses/info@removed-2.0' + x-version-removed: '2.0' + added-1.3-removed-2.0: + $ref: '#/components/responses/info@added-1.3-removed-2.0' + added-2.1: + $ref: '#/components/responses/info@added-2.1' '500': $ref: '#/components/responses/info@500' components: @@ -54,6 +64,17 @@ components: - tagline unevaluatedProperties: type: object + info@added-2.0: + description: Added in 2.0 via attribute next to ref. + info@removed-2.0: + description: Removed in 2.0 via attribute next to ref. + info@added-1.3-removed-2.0: + description: 'Added in 1.3, removed in 2.0 via attribute in response body.' + x-version-added: '1.3' + x-version-removed: '2.0' + info@added-2.1: + description: Added in 2.1 via attribute in response body. + x-version-added: '2.1' info@500: description: '' content: From 7508a3d16af72b71219890b270802013cb0c958e Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 18 Jul 2024 16:55:26 -0400 Subject: [PATCH 02/13] Got rid of merge(). Signed-off-by: dblock --- tools/src/linter/SchemasValidator.ts | 2 +- tools/src/merger/OpenApiMerger.ts | 24 ++++++++++-------------- tools/src/merger/merge.ts | 2 +- tools/src/tester/MergedOpenApiSpec.ts | 2 +- tools/tests/merger/OpenApiMerger.test.ts | 12 ++---------- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/tools/src/linter/SchemasValidator.ts b/tools/src/linter/SchemasValidator.ts index 6954d68ab..3c0065d94 100644 --- a/tools/src/linter/SchemasValidator.ts +++ b/tools/src/linter/SchemasValidator.ts @@ -33,7 +33,7 @@ export default class SchemasValidator { validate (): ValidationError[] { const merger = new OpenApiMerger(this.root_folder, undefined, new Logger(LogLevel.error)) - this.spec = merger.merge().spec().components as Record + this.spec = merger.spec().components as Record const named_schemas_errors = this.validate_named_schemas() if (named_schemas_errors.length > 0) return named_schemas_errors return [ diff --git a/tools/src/merger/OpenApiMerger.ts b/tools/src/merger/OpenApiMerger.ts index a0f16a5b8..b914c3be7 100644 --- a/tools/src/merger/OpenApiMerger.ts +++ b/tools/src/merger/OpenApiMerger.ts @@ -46,25 +46,21 @@ export default class OpenApiMerger { } write_to(output_path: string): OpenApiMerger { - if (!this._merged) { throw "Call merge() first." } this.logger.info(`Writing ${output_path} ...`) - write_yaml(output_path, this._spec) - return this - } - - merge (): OpenApiMerger { - if (this._merged) { throw "Spec already merged." } - this.#merge_schemas() - this.#merge_namespaces() - this.#sort_spec_keys() - this.#generate_global_params() - this.#generate_superseded_ops() - this._merged = true + write_yaml(output_path, this.spec()) return this } spec(): OpenAPIV3.Document { - if (!this._merged) { this.merge() } + if (!this._merged) { + this.#merge_schemas() + this.#merge_namespaces() + this.#sort_spec_keys() + this.#generate_global_params() + this.#generate_superseded_ops() + this._merged = true + } + return this._spec as OpenAPIV3.Document } diff --git a/tools/src/merger/merge.ts b/tools/src/merger/merge.ts index 0ad3d4eed..7b8c42903 100644 --- a/tools/src/merger/merge.ts +++ b/tools/src/merger/merge.ts @@ -25,5 +25,5 @@ const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) const merger = new OpenApiMerger(opts.source, opts.opensearchVersion, logger) logger.log(`Merging ${opts.source} into ${opts.output} (${opts.opensearchVersion}) ...`) -merger.merge().write_to(opts.output) +merger.write_to(opts.output) logger.log('Done.') diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index 1b4cb11a6..fe35d216b 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -31,7 +31,7 @@ export default class MergedOpenApiSpec { spec (): OpenAPIV3.Document { if (this._spec) return this._spec const merger = new OpenApiMerger(this.file_path, this.target_version, this.logger) - const spec = merger.merge().spec() + const spec = merger.spec() const ctx = new SpecificationContext(this.file_path) this.inject_additional_properties(ctx, spec) this._spec = spec diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index 1169b2101..2b5904ed6 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -24,16 +24,8 @@ describe('OpenApiMerger', () => { }) test('merges spec', () => { - merger.merge() expect(merger.spec()).toBeDefined() }) - - test('raises an error when called twice', () => { - merger.merge() - expect(() => { - merger.merge() - }).toThrow('Spec already merged.'); - }) }) describe('write_to()', () => { @@ -42,7 +34,7 @@ describe('OpenApiMerger', () => { }) test('writes a spec', () => { - merger.merge().write_to('./tools/tests/merger/opensearch-openapi.yaml') + merger.write_to('./tools/tests/merger/opensearch-openapi.yaml') expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_2.0.yaml', 'utf8')) .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) }) @@ -55,7 +47,7 @@ describe('OpenApiMerger', () => { }) test('writes a spec', () => { - merger.merge().write_to('./tools/tests/merger/opensearch-openapi.yaml') + merger.write_to('./tools/tests/merger/opensearch-openapi.yaml') expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_1.3.yaml', 'utf8')) .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) }) From f39f2e8c8551ad0b773fda1aba00d4f201ab013a Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 18 Jul 2024 18:19:05 -0400 Subject: [PATCH 03/13] Extract delete_matching_keys into a helper. Signed-off-by: dblock --- package-lock.json | 25 ++++ package.json | 6 +- tools/src/helpers.ts | 17 +++ tools/src/merger/OpenApiMerger.ts | 28 ++--- tools/tests/helpers.test.ts | 89 +++++++++++++- tools/tests/merger/OpenApiMerger.test.ts | 32 ++++- tools/tests/merger/opensearch-openapi.yaml | 118 ------------------- tools/tests/tester/MergedOpenApiSpec.test.ts | 4 +- 8 files changed, 169 insertions(+), 150 deletions(-) delete mode 100644 tools/tests/merger/opensearch-openapi.yaml diff --git a/package-lock.json b/package-lock.json index 9387b5d56..190f24271 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,7 @@ "@types/lodash": "^4.14.202", "@types/node": "^20.10.3", "@types/qs": "^6.9.15", + "@types/tmp": "^0.2.6", "@typescript-eslint/eslint-plugin": "^6.21.0", "ajv": "^8.13.0", "ajv-errors": "^3.0.0", @@ -41,6 +42,7 @@ "lodash": "^4.17.21", "qs": "^6.12.1", "smile-js": "^0.7.0", + "tmp": "^0.2.3", "ts-jest": "^29.1.2", "ts-node": "^10.9.1", "typescript": "<5.4.0", @@ -2296,6 +2298,11 @@ "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", "integrity": "sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==" }, + "node_modules/@types/tmp": { + "version": "0.2.6", + "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.6.tgz", + "integrity": "sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA==" + }, "node_modules/@types/yargs": { "version": "17.0.32", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.32.tgz", @@ -7684,6 +7691,14 @@ "node": ">=0.12" } }, + "node_modules/tmp": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.3.tgz", + "integrity": "sha512-nZD7m9iCPC5g0pYmcaxogYKggSfLsdxl8of3Q/oIbqCqLLIO9IAF0GWjX1z9NZRHPiXv8Wex4yDCaZsgEw0Y8w==", + "engines": { + "node": ">=14.14" + } + }, "node_modules/tmpl": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/tmpl/-/tmpl-1.0.5.tgz", @@ -10030,6 +10045,11 @@ "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", "integrity": "sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==" }, + "@types/tmp": { + "version": "0.2.6", + "resolved": "https://registry.npmjs.org/@types/tmp/-/tmp-0.2.6.tgz", + "integrity": "sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA==" + }, "@types/yargs": { "version": "17.0.32", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.32.tgz", @@ -13778,6 +13798,11 @@ "next-tick": "^1.1.0" } }, + "tmp": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.2.3.tgz", + "integrity": "sha512-nZD7m9iCPC5g0pYmcaxogYKggSfLsdxl8of3Q/oIbqCqLLIO9IAF0GWjX1z9NZRHPiXv8Wex4yDCaZsgEw0Y8w==" + }, "tmpl": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/tmpl/-/tmpl-1.0.5.tgz", diff --git a/package.json b/package.json index d433bb108..b574d42df 100644 --- a/package.json +++ b/package.json @@ -30,13 +30,15 @@ "@types/lodash": "^4.14.202", "@types/node": "^20.10.3", "@types/qs": "^6.9.15", + "@types/tmp": "^0.2.6", "@typescript-eslint/eslint-plugin": "^6.21.0", + "ajv": "^8.13.0", "ajv-errors": "^3.0.0", "ajv-formats": "^3.0.1", - "ajv": "^8.13.0", "axios": "^1.7.1", "cbor": "^9.0.2", "commander": "^12.0.0", + "eslint": "^8.57.0", "eslint-config-standard-with-typescript": "^43.0.1", "eslint-plugin-eslint-comments": "^3.2.0", "eslint-plugin-import": "^2.29.1", @@ -44,13 +46,13 @@ "eslint-plugin-n": "^16.6.2", "eslint-plugin-promise": "^6.1.1", "eslint-plugin-yml": "^1.14.0", - "eslint": "^8.57.0", "globals": "^15.0.0", "json-diff-ts": "^4.0.1", "json-schema-to-typescript": "^14.0.4", "lodash": "^4.17.21", "qs": "^6.12.1", "smile-js": "^0.7.0", + "tmp": "^0.2.3", "ts-jest": "^29.1.2", "ts-node": "^10.9.1", "typescript": "<5.4.0", diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 579f11388..3d5ef2e40 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -65,6 +65,23 @@ export function sort_array_by_keys (values: any[], priorities: string[] = []): s }) } +export function delete_matching_keys(obj: any, condition: (obj: any) => boolean): string[] { + let removed: string[] = [] + for (const key in obj) { + var item = obj[key] + if (_.isObject(item)) { + if (condition(item)) { + removed.push(key) + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete obj[key] + } else { + removed = _.concat(removed, delete_matching_keys(item, condition)) + } + } + } + return removed +} + export function ensure_parent_dir (file_path: string): void { fs.mkdirSync(path.dirname(file_path), { recursive: true }) } diff --git a/tools/src/merger/OpenApiMerger.ts b/tools/src/merger/OpenApiMerger.ts index b914c3be7..800c636c2 100644 --- a/tools/src/merger/OpenApiMerger.ts +++ b/tools/src/merger/OpenApiMerger.ts @@ -10,7 +10,7 @@ import { type OpenAPIV3 } from 'openapi-types' import fs from 'fs' import _, { isEmpty } from 'lodash' -import { read_yaml, write_yaml } from '../helpers' +import { delete_matching_keys, read_yaml, write_yaml } from '../helpers' import SupersededOpsGenerator from './SupersededOpsGenerator' import GlobalParamsGenerator from './GlobalParamsGenerator' import { Logger } from '../Logger' @@ -82,10 +82,10 @@ export default class OpenApiMerger { // Remove any refs that are x-version-added/removed incompatible with the target server version. #remove_refs_per_semver() : void { - this.#remove_per_semver(this._spec.paths) + this.#remove_keys_not_matching_semver(this._spec.paths) // parameters - const removed_params = this.#remove_per_semver(this._spec.components.parameters) + const removed_params = this.#remove_keys_not_matching_semver(this._spec.components.parameters) const removed_parameter_refs = _.map(removed_params, (ref) => `#/components/parameters/${ref}`) Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { Object.entries(path_item as Document).forEach(([_method, method_item]) => { @@ -94,7 +94,7 @@ export default class OpenApiMerger { }) // responses - const removed_responses = this.#remove_per_semver(this._spec.components.responses) + const removed_responses = this.#remove_keys_not_matching_semver(this._spec.components.responses) const removed_response_refs = _.map(removed_responses, (ref) => `#/components/responses/${ref}`) Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { Object.entries(path_item as Document).forEach(([_method, method_item]) => { @@ -121,23 +121,9 @@ export default class OpenApiMerger { } // Remove any elements that are x-version-added/removed incompatible with the target server version. - #remove_per_semver(obj: any): string[] { - let removed: string[] = [] - if (this.target_version === undefined) return removed - - for (const key in obj) { - if (_.isObject(obj[key])) { - if (this.#exclude_per_semver(obj[key])) { - removed.push(key) - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete obj[key] - } else { - removed = _.concat(removed, this.#remove_per_semver(obj[key])) - } - } - } - - return removed + #remove_keys_not_matching_semver(obj: any): string[] { + if (this.target_version === undefined) return [] + return delete_matching_keys(obj, this.#exclude_per_semver.bind(this)) } // Redirect schema references in namespace files to local references in single-file spec. diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index 60480af32..5268b55eb 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -7,7 +7,8 @@ * compatible open source license. */ -import { sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' +import _ from 'lodash' +import { delete_matching_keys, sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' describe('helpers', () => { describe('sort_array_by_keys', () => { @@ -36,4 +37,90 @@ describe('helpers', () => { expect(to_ndjson([{ x: 1 }])).toEqual("{\"x\":1}\n") expect(to_ndjson([{ x: 1 }, { y: 'z' }])).toEqual("{\"x\":1}\n{\"y\":\"z\"}\n") }) + + describe('delete_matching_keys', () => { + test('empty collection', () => { + var obj = {} + expect(delete_matching_keys(obj, (_obj) => false)).toEqual([]) + expect(obj).toEqual({}) + }) + + describe('an object', () => { + var obj: object + + beforeEach(() => { + obj = { + foo: { + bar1: { + x: 1 + } + }, + zar: { + bar2: { + y: 2 + } + } + } + }) + + test('removes all keys', () => { + expect(delete_matching_keys(obj, (_item) => true)).toEqual(['foo', 'zar']) + expect(obj).toStrictEqual({}) + }) + + test('removes no keys', () => { + const obj2 = _.cloneDeep(obj) + expect(delete_matching_keys(obj, (_item) => false)).toEqual([]) + expect(obj).toStrictEqual(obj2) + }) + + test('removes a value from a key', () => { + expect(delete_matching_keys(obj, (_item: any) => _item.x == 1)).toEqual(['bar1']) + expect(obj).toStrictEqual({ foo: {}, zar: { bar2: { y: 2 } } }) + }) + + test('removes multiple values from a key', () => { + expect(delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2)).toEqual(['bar1', 'bar2']) + expect(obj).toStrictEqual({ foo: {}, zar: {} }) + }) + }) + + describe('an object with arrays', () => { + var obj: object + + beforeEach(() => { + obj = { + foo: [{ + bar1: { + x: 1 + }, + bar2: { + y: 2 + } + }], + } + }) + + test('removes all keys', () => { + expect(delete_matching_keys(obj, (_item) => true)).toEqual(['foo']) + expect(obj).toStrictEqual({}) + }) + + test('removes no keys', () => { + const obj2 = _.cloneDeep(obj) + expect(delete_matching_keys(obj, (_item) => false)).toEqual([]) + expect(obj).toStrictEqual(obj2) + }) + + test('removes a value from a key', () => { + expect(delete_matching_keys(obj, (_item: any) => _item.x == 1)).toEqual(['bar1']) + expect(obj).toStrictEqual({ foo: [{ bar2: { y: 2 } }] }) + }) + + test('removes multiple values from a key', () => { + expect(delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2)).toEqual(['bar1', 'bar2']) + expect(obj).toStrictEqual({ foo: [{}] }) + }) + }) + }) }) diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index 2b5904ed6..e24d4a657 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -9,6 +9,7 @@ import OpenApiMerger from 'merger/OpenApiMerger' import fs from 'fs' +import tmp from 'tmp' describe('OpenApiMerger', () => { var merger: OpenApiMerger @@ -29,27 +30,46 @@ describe('OpenApiMerger', () => { }) describe('write_to()', () => { - afterAll(() => { - fs.unlinkSync('./tools/tests/merger/opensearch-openapi.yaml') + var temp: tmp.DirResult + var filename: string + + beforeEach(() => { + temp = tmp.dirSync() + filename = `${temp.name}/opensearch-openapi.yaml` + }) + + afterEach(() => { + fs.unlinkSync(filename) + temp.removeCallback() }) test('writes a spec', () => { - merger.write_to('./tools/tests/merger/opensearch-openapi.yaml') + merger.write_to(filename) expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_2.0.yaml', 'utf8')) - .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) + .toEqual(fs.readFileSync(filename, 'utf8')) }) }) }) describe('1.3', () => { + var temp: tmp.DirResult + var filename: string + beforeEach(() => { merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', '1.3') + temp = tmp.dirSync() + filename = `${temp.name}/opensearch-openapi.yaml` + }) + + afterEach(() => { + fs.unlinkSync(filename) + temp.removeCallback() }) test('writes a spec', () => { - merger.write_to('./tools/tests/merger/opensearch-openapi.yaml') + merger.write_to(filename) expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_1.3.yaml', 'utf8')) - .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) + .toEqual(fs.readFileSync(filename, 'utf8')) }) }) }) diff --git a/tools/tests/merger/opensearch-openapi.yaml b/tools/tests/merger/opensearch-openapi.yaml deleted file mode 100644 index d75111c45..000000000 --- a/tools/tests/merger/opensearch-openapi.yaml +++ /dev/null @@ -1,118 +0,0 @@ -openapi: 3.1.0 -info: - title: OpenSearch API - description: OpenSearch API - version: 1.0.0 -paths: - /{index}: - post: - parameters: - - $ref: '#/components/parameters/indices.create::path.index' - - $ref: '#/components/parameters/indices.create::query.pretty' - - $ref: '#/components/parameters/_global::query.human' - requestBody: - $ref: '#/components/requestBodies/indices.create' - responses: - '200': - $ref: '#/components/responses/indices.create@200' - /adopt/{animal}/dockets/{docket}: - get: - operationId: adopt.0 - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - responses: - '200': - $ref: '#/components/responses/adopt@200' - post: - operationId: adopt.1 - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - requestBody: - $ref: '#/components/requestBodies/adopt' - responses: - '200': - $ref: '#/components/responses/adopt@200' - /replaced/adopting/{animal}/something/{docket}: - get: - operationId: adopt.0_superseded - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - responses: - '200': - $ref: '#/components/responses/adopt@200' - deprecated: true - x-ignorable: true -components: - parameters: - _global::query.human: - name: human - in: query - description: Whether to return human readable values for statistics. - schema: - type: boolean - default: true - x-global: true - adopt::path.animal: - name: animal - in: path - schema: - $ref: '#/components/schemas/animals:Animal' - adopt::path.docket: - name: docket - in: path - schema: - type: number - 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 - indices.create@201: - description: Added in 2.0. - 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/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index 639f4ce8a..80211c140 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -14,7 +14,7 @@ import MergedOpenApiSpec from "tester/MergedOpenApiSpec" describe('merged API spec', () => { describe('defaults', () => { const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', undefined, new Logger()) - + test('has an api version', () => { expect(spec.api_version()).toEqual('1.2.3') }) @@ -31,7 +31,7 @@ describe('merged API spec', () => { expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ '200', '201', '404', '500', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' ]) - }) + }) describe('unevaluatedProperties', () => { const responses: any = spec.spec().components?.responses From f77e76c93b3bccd677a008b6c51d322591daa0a7 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 22 Jul 2024 15:54:07 -0400 Subject: [PATCH 04/13] Split up OpenApiVersionExtractor from OpenApiMerger. Signed-off-by: dblock --- tools/src/linter/SchemasValidator.ts | 2 +- tools/src/merger/OpenApiMerger.ts | 57 +------ tools/src/merger/OpenApiVersionExtractor.ts | 102 ++++++++++++ tools/src/merger/merge.ts | 17 +- tools/src/tester/MergedOpenApiSpec.ts | 9 +- tools/tests/merger/OpenApiMerger.test.ts | 24 +-- .../merger/OpenApiVersionExtractor.test.ts | 105 ++++++++++++ tools/tests/merger/fixtures/expected_1.3.yaml | 118 -------------- .../fixtures/extractor/expected_1.3.yaml | 141 +++++++++++++++++ .../fixtures/extractor/expected_2.0.yaml | 149 ++++++++++++++++++ .../expected.yaml} | 0 11 files changed, 521 insertions(+), 203 deletions(-) create mode 100644 tools/src/merger/OpenApiVersionExtractor.ts create mode 100644 tools/tests/merger/OpenApiVersionExtractor.test.ts delete mode 100644 tools/tests/merger/fixtures/expected_1.3.yaml create mode 100644 tools/tests/merger/fixtures/extractor/expected_1.3.yaml create mode 100644 tools/tests/merger/fixtures/extractor/expected_2.0.yaml rename tools/tests/merger/fixtures/{expected_2.0.yaml => merger/expected.yaml} (100%) diff --git a/tools/src/linter/SchemasValidator.ts b/tools/src/linter/SchemasValidator.ts index 3c0065d94..d22c0beaa 100644 --- a/tools/src/linter/SchemasValidator.ts +++ b/tools/src/linter/SchemasValidator.ts @@ -32,7 +32,7 @@ export default class SchemasValidator { } validate (): ValidationError[] { - const merger = new OpenApiMerger(this.root_folder, undefined, new Logger(LogLevel.error)) + const merger = new OpenApiMerger(this.root_folder, new Logger(LogLevel.error)) this.spec = merger.spec().components as Record const named_schemas_errors = this.validate_named_schemas() if (named_schemas_errors.length > 0) return named_schemas_errors diff --git a/tools/src/merger/OpenApiMerger.ts b/tools/src/merger/OpenApiMerger.ts index 800c636c2..ff8cd7d06 100644 --- a/tools/src/merger/OpenApiMerger.ts +++ b/tools/src/merger/OpenApiMerger.ts @@ -9,18 +9,16 @@ import { type OpenAPIV3 } from 'openapi-types' import fs from 'fs' -import _, { isEmpty } from 'lodash' -import { delete_matching_keys, read_yaml, write_yaml } from '../helpers' +import _ from 'lodash' +import { read_yaml, write_yaml } from '../helpers' import SupersededOpsGenerator from './SupersededOpsGenerator' import GlobalParamsGenerator from './GlobalParamsGenerator' import { Logger } from '../Logger' -import * as semver from 'semver' // Create a single-file OpenAPI spec from multiple files for OpenAPI validation and programmatic consumption export default class OpenApiMerger { root_folder: string logger: Logger - target_version?: string protected _spec: Record protected _merged: boolean = false @@ -28,10 +26,9 @@ export default class OpenApiMerger { paths: Record> = {} // namespace -> path -> path_item_object schemas: Record> = {} // category -> schema -> schema_object - constructor (root_folder: string, target_version?: string, logger: Logger = new Logger()) { + constructor (root_folder: string, logger: Logger = new Logger()) { this.logger = logger this.root_folder = fs.realpathSync(root_folder) - this.target_version = target_version === undefined ? undefined : semver.coerce(target_version)?.toString() this._spec = { openapi: '3.1.0', info: read_yaml(`${this.root_folder}/_info.yaml`, true), @@ -76,54 +73,6 @@ export default class OpenApiMerger { this._spec.components.responses = { ...this._spec.components.responses, ...spec.components.responses } this._spec.components.requestBodies = { ...this._spec.components.requestBodies, ...spec.components.requestBodies } }) - - this.#remove_refs_per_semver() - } - - // Remove any refs that are x-version-added/removed incompatible with the target server version. - #remove_refs_per_semver() : void { - this.#remove_keys_not_matching_semver(this._spec.paths) - - // parameters - const removed_params = this.#remove_keys_not_matching_semver(this._spec.components.parameters) - const removed_parameter_refs = _.map(removed_params, (ref) => `#/components/parameters/${ref}`) - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - method_item.parameters = _.filter(method_item.parameters, (param) => !_.includes(removed_parameter_refs, param.$ref)) - }) - }) - - // responses - const removed_responses = this.#remove_keys_not_matching_semver(this._spec.components.responses) - const removed_response_refs = _.map(removed_responses, (ref) => `#/components/responses/${ref}`) - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - method_item.responses = _.omitBy(method_item.responses, (param) => _.includes(removed_response_refs, param.$ref)) - }) - }) - - this._spec.paths = _.omitBy(this._spec.paths, isEmpty) - } - - #exclude_per_semver(obj: any): boolean { - if (this.target_version === undefined) return false - - const x_version_added = semver.coerce(obj['x-version-added'] as string) - const x_version_removed = semver.coerce(obj['x-version-removed'] as string) - - if (x_version_added && !semver.satisfies(this.target_version, `>=${x_version_added?.toString()}`)) { - return true - } else if (x_version_removed && !semver.satisfies(this.target_version, `<${x_version_removed?.toString()}`)) { - return true - } - - return false - } - - // Remove any elements that are x-version-added/removed incompatible with the target server version. - #remove_keys_not_matching_semver(obj: any): string[] { - if (this.target_version === undefined) return [] - return delete_matching_keys(obj, this.#exclude_per_semver.bind(this)) } // Redirect schema references in namespace files to local references in single-file spec. diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts new file mode 100644 index 000000000..c467cbd2d --- /dev/null +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -0,0 +1,102 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { Logger } from '../Logger' +import { delete_matching_keys, write_yaml } from '../helpers' +import _, { isEmpty } from 'lodash' +import { type OpenAPIV3 } from 'openapi-types' +import semver from 'semver' + +// Extract a versioned API +export default class OpenApiVersionExtractor { + private _spec?: Record + private _source_spec: OpenAPIV3.Document + private _target_version?: string + private _logger: Logger + + constructor(source_spec: OpenAPIV3.Document, target_version?: string, logger: Logger = new Logger()) { + this._source_spec = source_spec + this._target_version = target_version !== undefined ? semver.coerce(target_version)?.toString() : undefined + this._logger = logger + this._spec = undefined + } + + extract(): OpenAPIV3.Document { + if (this._spec) return this._spec as OpenAPIV3.Document + if (this._target_version !== undefined) { + this.#extract() + } else { + this._spec = this._source_spec + } + return this._spec as OpenAPIV3.Document + } + + write_to(output_path: string): OpenApiVersionExtractor { + this._logger.info(`Writing ${output_path} ...`) + write_yaml(output_path, this.extract()) + return this + } + + // Remove any refs that are x-version-added/removed incompatible with the target server version. + #extract() : void { + this._logger.info(`Extracting version ${this._target_version} ...`) + + this._spec = _.cloneDeep(this._source_spec) + + this._spec.components = this._spec.components ?? { + parameters: {}, + requestBodies: {}, + responses: {}, + schemas: {} + } + + this.#remove_keys_not_matching_semver(this._spec.paths) + + // parameters + const removed_params = this.#remove_keys_not_matching_semver(this._spec.components.parameters) + const removed_parameter_refs = _.map(removed_params, (ref) => `#/components/parameters/${ref}`) + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + method_item.parameters = _.filter(method_item.parameters, (param) => !_.includes(removed_parameter_refs, param.$ref)) + }) + }) + + // responses + const removed_responses = this.#remove_keys_not_matching_semver(this._spec.components.responses) + const removed_response_refs = _.map(removed_responses, (ref) => `#/components/responses/${ref}`) + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + method_item.responses = _.omitBy(method_item.responses, (param) => _.includes(removed_response_refs, param.$ref)) + }) + }) + + this._spec.paths = _.omitBy(this._spec.paths, isEmpty) + } + + #exclude_per_semver(obj: any): boolean { + if (this._target_version === undefined) return false + + const x_version_added = semver.coerce(obj['x-version-added'] as string) + const x_version_removed = semver.coerce(obj['x-version-removed'] as string) + + if (x_version_added && !semver.satisfies(this._target_version, `>=${x_version_added?.toString()}`)) { + return true + } else if (x_version_removed && !semver.satisfies(this._target_version, `<${x_version_removed?.toString()}`)) { + return true + } + + return false + } + + // Remove any elements that are x-version-added/removed incompatible with the target server version. + #remove_keys_not_matching_semver(obj: any): string[] { + if (this._target_version === undefined) return [] + return delete_matching_keys(obj, this.#exclude_per_semver.bind(this)) + } +} diff --git a/tools/src/merger/merge.ts b/tools/src/merger/merge.ts index 7b8c42903..b2dd16d84 100644 --- a/tools/src/merger/merge.ts +++ b/tools/src/merger/merge.ts @@ -8,9 +8,10 @@ */ import { Command, Option } from '@commander-js/extra-typings' -import OpenApiMerger from './OpenApiMerger' -import { resolve } from 'path' import { Logger, LogLevel } from '../Logger' +import { resolve } from 'path' +import OpenApiMerger from './OpenApiMerger' +import OpenApiVersionExtractor from './OpenApiVersionExtractor' const command = new Command() .description('Merges the multi-file OpenSearch spec into a single file for programmatic use.') @@ -23,7 +24,13 @@ const command = new Command() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const merger = new OpenApiMerger(opts.source, opts.opensearchVersion, logger) -logger.log(`Merging ${opts.source} into ${opts.output} (${opts.opensearchVersion}) ...`) -merger.write_to(opts.output) +const merger = new OpenApiMerger(opts.source, logger) +if (opts.opensearchVersion === undefined) { + logger.log(`Merging ${opts.source} into ${opts.output} ...`) + merger.write_to(opts.output) +} else { + logger.log(`Merging ${opts.source} into ${opts.output} (${opts.opensearchVersion}) ...`) + const extractor = new OpenApiVersionExtractor(merger.spec(), opts.opensearchVersion) + extractor.write_to(opts.output) +} logger.log('Done.') diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index fe35d216b..ee21f55ac 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -13,6 +13,7 @@ import { determine_possible_schema_types, HTTP_METHODS, SpecificationContext } f import { SchemaVisitor } from '../_utils/SpecificationVisitor'; import OpenApiMerger from '../merger/OpenApiMerger'; import _ from 'lodash'; +import OpenApiVersionExtractor from '../merger/OpenApiVersionExtractor'; // An augmented spec with additionalProperties: false. export default class MergedOpenApiSpec { @@ -30,8 +31,12 @@ export default class MergedOpenApiSpec { spec (): OpenAPIV3.Document { if (this._spec) return this._spec - const merger = new OpenApiMerger(this.file_path, this.target_version, this.logger) - const spec = merger.spec() + const merger = new OpenApiMerger(this.file_path, this.logger) + var spec = merger.spec() + if (this.target_version !== undefined) { + const version_extractor = new OpenApiVersionExtractor(spec, this.target_version) + spec = version_extractor.extract() + } const ctx = new SpecificationContext(this.file_path) this.inject_additional_properties(ctx, spec) this._spec = spec diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index e24d4a657..057866dd2 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -45,31 +45,9 @@ describe('OpenApiMerger', () => { test('writes a spec', () => { merger.write_to(filename) - expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_2.0.yaml', 'utf8')) + expect(fs.readFileSync('./tools/tests/merger/fixtures/merger/expected.yaml', 'utf8')) .toEqual(fs.readFileSync(filename, 'utf8')) }) }) }) - - describe('1.3', () => { - var temp: tmp.DirResult - var filename: string - - beforeEach(() => { - merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', '1.3') - temp = tmp.dirSync() - filename = `${temp.name}/opensearch-openapi.yaml` - }) - - afterEach(() => { - fs.unlinkSync(filename) - temp.removeCallback() - }) - - test('writes a spec', () => { - merger.write_to(filename) - expect(fs.readFileSync('./tools/tests/merger/fixtures/expected_1.3.yaml', 'utf8')) - .toEqual(fs.readFileSync(filename, 'utf8')) - }) - }) }) diff --git a/tools/tests/merger/OpenApiVersionExtractor.test.ts b/tools/tests/merger/OpenApiVersionExtractor.test.ts new file mode 100644 index 000000000..d0b8aa6a7 --- /dev/null +++ b/tools/tests/merger/OpenApiVersionExtractor.test.ts @@ -0,0 +1,105 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import _ from 'lodash' +import OpenApiMerger from 'merger/OpenApiMerger' +import OpenApiVersionExtractor from 'merger/OpenApiVersionExtractor' +import fs from 'fs' +import tmp from 'tmp' + +describe('extract() from a merged API spec', () => { + const merger = new OpenApiMerger('tools/tests/tester/fixtures/specs/complete') + + describe('defaults', () => { + const extractor = new OpenApiVersionExtractor(merger.spec()) + + test('has all responses', () => { + const spec = extractor.extract() + + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' + ]) + }) + + describe('write_to()', () => { + var temp: tmp.DirResult + var filename: string + + beforeEach(() => { + temp = tmp.dirSync() + filename = `${temp.name}/opensearch-openapi.yaml` + }) + + afterEach(() => { + fs.unlinkSync(filename) + temp.removeCallback() + }) + + test('writes a spec', () => { + extractor.write_to(filename) + expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_2.0.yaml', 'utf8')) + .toEqual(fs.readFileSync(filename, 'utf8')) + }) + }) + }) + + describe('1.3', () => { + const extractor = new OpenApiVersionExtractor(merger.spec(), '1.3') + + describe('write_to', () => { + var temp: tmp.DirResult + var filename: string + + beforeEach(() => { + temp = tmp.dirSync() + filename = `${temp.name}/opensearch-openapi.yaml` + }) + + afterEach(() => { + fs.unlinkSync(filename) + temp.removeCallback() + }) + + test('writes a spec', () => { + extractor.write_to(filename) + expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_1.3.yaml', 'utf8')) + .toEqual(fs.readFileSync(filename, 'utf8')) + }) + }) + + test('has matching responses', () => { + const spec = extractor.extract() + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'removed-2.0', 'added-1.3-removed-2.0' + ]) + }) + + describe('2.0', () => { + const extractor = new OpenApiVersionExtractor(merger.spec(), '2.0') + + test('has matching responses', () => { + const spec = extractor.extract() + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0' + ]) + }) + }) + + describe('2.1', () => { + const extractor = new OpenApiVersionExtractor(merger.spec(), '2.1') + + test('has matching responses', () => { + const spec = extractor.extract() + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', 'added-2.0', 'added-2.1' + ]) + }) + }) + }) +}) diff --git a/tools/tests/merger/fixtures/expected_1.3.yaml b/tools/tests/merger/fixtures/expected_1.3.yaml deleted file mode 100644 index d75111c45..000000000 --- a/tools/tests/merger/fixtures/expected_1.3.yaml +++ /dev/null @@ -1,118 +0,0 @@ -openapi: 3.1.0 -info: - title: OpenSearch API - description: OpenSearch API - version: 1.0.0 -paths: - /{index}: - post: - parameters: - - $ref: '#/components/parameters/indices.create::path.index' - - $ref: '#/components/parameters/indices.create::query.pretty' - - $ref: '#/components/parameters/_global::query.human' - requestBody: - $ref: '#/components/requestBodies/indices.create' - responses: - '200': - $ref: '#/components/responses/indices.create@200' - /adopt/{animal}/dockets/{docket}: - get: - operationId: adopt.0 - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - responses: - '200': - $ref: '#/components/responses/adopt@200' - post: - operationId: adopt.1 - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - requestBody: - $ref: '#/components/requestBodies/adopt' - responses: - '200': - $ref: '#/components/responses/adopt@200' - /replaced/adopting/{animal}/something/{docket}: - get: - operationId: adopt.0_superseded - parameters: - - $ref: '#/components/parameters/adopt::path.animal' - - $ref: '#/components/parameters/adopt::path.docket' - - $ref: '#/components/parameters/_global::query.human' - responses: - '200': - $ref: '#/components/responses/adopt@200' - deprecated: true - x-ignorable: true -components: - parameters: - _global::query.human: - name: human - in: query - description: Whether to return human readable values for statistics. - schema: - type: boolean - default: true - x-global: true - adopt::path.animal: - name: animal - in: path - schema: - $ref: '#/components/schemas/animals:Animal' - adopt::path.docket: - name: docket - in: path - schema: - type: number - 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 - indices.create@201: - description: Added in 2.0. - 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/tests/merger/fixtures/extractor/expected_1.3.yaml b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml new file mode 100644 index 000000000..3958db67b --- /dev/null +++ b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml @@ -0,0 +1,141 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 + x-api-version: 1.2.3 +paths: + /_nodes/{id}: + get: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + post: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + requestBody: + $ref: '#/components/requestBodies/nodes.info' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + /index: + get: + operationId: get.0 + responses: + '200': + $ref: '#/components/responses/info@200' + '201': + $ref: '#/components/responses/info@201' + '404': + $ref: '#/components/responses/info@404' + '500': + $ref: '#/components/responses/info@500' + removed-2.0: + $ref: '#/components/responses/info@removed-2.0' + x-version-removed: '2.0' + added-1.3-removed-2.0: + $ref: '#/components/responses/info@added-1.3-removed-2.0' + parameters: [] + /nodes: + get: + operationId: nodes.0 + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + parameters: [] +components: + parameters: + nodes.info::path.id: + in: path + name: id + description: Node ID. + required: true + schema: + type: string + requestBodies: + nodes.info: + content: + application/json: + schema: + type: object + properties: + _all: + type: boolean + description: Nodes options. + responses: + info@200: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + info@201: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: true + info@404: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: + type: object + info@500: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + info@503: + description: '' + content: + application/json: + schema: + type: object + info@added-1.3-removed-2.0: + description: Added in 1.3, removed in 2.0 via attribute in response body. + x-version-added: '1.3' + x-version-removed: '2.0' + info@added-2.0: + description: Added in 2.0 via attribute next to ref. + info@removed-2.0: + description: Removed in 2.0 via attribute next to ref. + nodes.info@200: + description: All nodes. + content: + application/json: + schema: + type: object + schemas: {} diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml new file mode 100644 index 000000000..883e167b0 --- /dev/null +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -0,0 +1,149 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 + x-api-version: 1.2.3 +paths: + /_nodes/{id}: + get: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + post: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + requestBody: + $ref: '#/components/requestBodies/nodes.info' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + /index: + get: + operationId: get.0 + responses: + '200': + $ref: '#/components/responses/info@200' + '201': + $ref: '#/components/responses/info@201' + '404': + $ref: '#/components/responses/info@404' + '500': + $ref: '#/components/responses/info@500' + added-2.0: + $ref: '#/components/responses/info@added-2.0' + x-version-added: '2.0' + removed-2.0: + $ref: '#/components/responses/info@removed-2.0' + x-version-removed: '2.0' + added-1.3-removed-2.0: + $ref: '#/components/responses/info@added-1.3-removed-2.0' + added-2.1: + $ref: '#/components/responses/info@added-2.1' + parameters: [] + /nodes: + get: + operationId: nodes.0 + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + parameters: [] +components: + parameters: + nodes.info::path.id: + in: path + name: id + description: Node ID. + required: true + schema: + type: string + requestBodies: + nodes.info: + content: + application/json: + schema: + type: object + properties: + _all: + type: boolean + description: Nodes options. + responses: + info@200: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + info@201: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: true + info@404: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: + type: object + info@500: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + info@503: + description: '' + content: + application/json: + schema: + type: object + info@added-1.3-removed-2.0: + description: Added in 1.3, removed in 2.0 via attribute in response body. + x-version-added: '1.3' + x-version-removed: '2.0' + info@added-2.0: + description: Added in 2.0 via attribute next to ref. + info@added-2.1: + description: Added in 2.1 via attribute in response body. + x-version-added: '2.1' + info@removed-2.0: + description: Removed in 2.0 via attribute next to ref. + nodes.info@200: + description: All nodes. + content: + application/json: + schema: + type: object + schemas: {} diff --git a/tools/tests/merger/fixtures/expected_2.0.yaml b/tools/tests/merger/fixtures/merger/expected.yaml similarity index 100% rename from tools/tests/merger/fixtures/expected_2.0.yaml rename to tools/tests/merger/fixtures/merger/expected.yaml From cdf777d89ca9326b01352a6943d886b237f35db5 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 23 Jul 2024 14:31:53 -0400 Subject: [PATCH 05/13] Removed unused refs. Signed-off-by: dblock --- tools/src/merger/OpenApiVersionExtractor.ts | 64 ++++++++++++++++++- .../merger/OpenApiVersionExtractor.test.ts | 8 +-- .../fixtures/extractor/expected_1.3.yaml | 27 +------- .../fixtures/extractor/expected_2.0.yaml | 21 +++++- .../merger/fixtures/merger/expected.yaml | 4 +- .../fixtures/spec/namespaces/indices.yaml | 4 +- tools/tests/tester/MergedOpenApiSpec.test.ts | 8 +-- .../specs/complete/namespaces/index.yaml | 2 + .../specs/complete/namespaces/nodes.yaml | 19 +++++- 9 files changed, 120 insertions(+), 37 deletions(-) diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index c467cbd2d..560a962bc 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -43,7 +43,8 @@ export default class OpenApiVersionExtractor { return this } - // Remove any refs that are x-version-added/removed incompatible with the target server version. + // Remove any refs and objects that reference them that are x-version-added/removed + // incompatible with the target server version. #extract() : void { this._logger.info(`Extracting version ${this._target_version} ...`) @@ -77,6 +78,10 @@ export default class OpenApiVersionExtractor { }) this._spec.paths = _.omitBy(this._spec.paths, isEmpty) + + this.#remove_unused_bodies() + this.#remove_unused_parameters() + this.#remove_unused_responses() } #exclude_per_semver(obj: any): boolean { @@ -99,4 +104,61 @@ export default class OpenApiVersionExtractor { if (this._target_version === undefined) return [] return delete_matching_keys(obj, this.#exclude_per_semver.bind(this)) } + + #remove_unused_bodies(): void { + if (this._spec === undefined) return + + var used_bodies: string[] = [] + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + if (method_item.requestBody?.$ref !== undefined) { + used_bodies = _.concat(used_bodies, method_item.requestBody.$ref) + } + }) + }) + + this._spec.components.requestBodies = _.pickBy(this._spec.components.requestBodies, (_value, key) => + _.includes(used_bodies, `#/components/requestBodies/${key}`) + ) + } + + + #remove_unused_responses(): void { + if (this._spec === undefined) return + + var used_responses: string[] = [] + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + Object.entries(method_item.responses as Document).forEach(([_response_code, response]) => { + if (response.$ref !== undefined) { + used_responses = _.concat(used_responses, response.$ref) + } + }) + }) + }) + + this._spec.components.responses = _.pickBy(this._spec.components.responses, (_value, key) => + _.includes(used_responses, `#/components/responses/${key}`) + ) + } + + + #remove_unused_parameters(): void { + if (this._spec === undefined) return + + var used_parameters: string[] = [] + Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { + Object.entries(path_item as Document).forEach(([_method, method_item]) => { + Object.entries(method_item.parameters as Document).forEach(([_response_code, parameter]) => { + if (parameter.$ref !== undefined) { + used_parameters = _.concat(used_parameters, parameter.$ref) + } + }) + }) + }) + + this._spec.components.parameters = _.pickBy(this._spec.components.parameters, (_value, key) => + _.includes(used_parameters, `#/components/parameters/${key}`) + ) + } } diff --git a/tools/tests/merger/OpenApiVersionExtractor.test.ts b/tools/tests/merger/OpenApiVersionExtractor.test.ts index d0b8aa6a7..2a5bd28a9 100644 --- a/tools/tests/merger/OpenApiVersionExtractor.test.ts +++ b/tools/tests/merger/OpenApiVersionExtractor.test.ts @@ -23,7 +23,7 @@ describe('extract() from a merged API spec', () => { const spec = extractor.extract() expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' + '200', '201', '404', '500', '503', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' ]) }) @@ -76,7 +76,7 @@ describe('extract() from a merged API spec', () => { test('has matching responses', () => { const spec = extractor.extract() expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'removed-2.0', 'added-1.3-removed-2.0' + '200', '201', '404', '500', '503', 'removed-2.0', 'added-1.3-removed-2.0' ]) }) @@ -86,7 +86,7 @@ describe('extract() from a merged API spec', () => { test('has matching responses', () => { const spec = extractor.extract() expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0' + '200', '201', '404', '500', '503', 'added-2.0' ]) }) }) @@ -97,7 +97,7 @@ describe('extract() from a merged API spec', () => { test('has matching responses', () => { const spec = extractor.extract() expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0', 'added-2.1' + '200', '201', '404', '500', '503', 'added-2.0', 'added-2.1' ]) }) }) diff --git a/tools/tests/merger/fixtures/extractor/expected_1.3.yaml b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml index 3958db67b..42791f2ee 100644 --- a/tools/tests/merger/fixtures/extractor/expected_1.3.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml @@ -16,18 +16,6 @@ paths: responses: '200': $ref: '#/components/responses/nodes.info@200' - post: - operationId: nodes.info.1 - x-operation-group: nodes.info - x-version-added: '1.0' - description: Returns information about nodes in the cluster. - parameters: - - $ref: '#/components/parameters/nodes.info::path.id' - requestBody: - $ref: '#/components/requestBodies/nodes.info' - responses: - '200': - $ref: '#/components/responses/nodes.info@200' /index: get: operationId: get.0 @@ -40,6 +28,8 @@ paths: $ref: '#/components/responses/info@404' '500': $ref: '#/components/responses/info@500' + '503': + $ref: '#/components/responses/info@503' removed-2.0: $ref: '#/components/responses/info@removed-2.0' x-version-removed: '2.0' @@ -62,16 +52,7 @@ components: required: true schema: type: string - requestBodies: - nodes.info: - content: - application/json: - schema: - type: object - properties: - _all: - type: boolean - description: Nodes options. + requestBodies: {} responses: info@200: description: '' @@ -128,8 +109,6 @@ components: description: Added in 1.3, removed in 2.0 via attribute in response body. x-version-added: '1.3' x-version-removed: '2.0' - info@added-2.0: - description: Added in 2.0 via attribute next to ref. info@removed-2.0: description: Removed in 2.0 via attribute next to ref. nodes.info@200: diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml index 883e167b0..6cb54f13b 100644 --- a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -19,15 +19,18 @@ paths: post: operationId: nodes.info.1 x-operation-group: nodes.info - x-version-added: '1.0' + x-version-added: '2.0' description: Returns information about nodes in the cluster. parameters: - $ref: '#/components/parameters/nodes.info::path.id' + - $ref: '#/components/parameters/nodes.info::query.flag' requestBody: $ref: '#/components/requestBodies/nodes.info' responses: '200': $ref: '#/components/responses/nodes.info@200' + '201': + $ref: '#/components/responses/nodes.info@201' /index: get: operationId: get.0 @@ -40,6 +43,8 @@ paths: $ref: '#/components/responses/info@404' '500': $ref: '#/components/responses/info@500' + '503': + $ref: '#/components/responses/info@503' added-2.0: $ref: '#/components/responses/info@added-2.0' x-version-added: '2.0' @@ -67,6 +72,14 @@ components: required: true schema: type: string + nodes.info::query.flag: + in: query + name: flag + description: Flag. + required: false + schema: + type: boolean + default: false requestBodies: nodes.info: content: @@ -146,4 +159,10 @@ components: application/json: schema: type: object + nodes.info@201: + description: All nodes. + content: + application/json: + schema: + type: object schemas: {} diff --git a/tools/tests/merger/fixtures/merger/expected.yaml b/tools/tests/merger/fixtures/merger/expected.yaml index 3b547b720..ef0560ad2 100644 --- a/tools/tests/merger/fixtures/merger/expected.yaml +++ b/tools/tests/merger/fixtures/merger/expected.yaml @@ -90,7 +90,9 @@ components: type: boolean requestBodies: adopt: {} - indices.create: {} + indices.create: + name: + type: string responses: adopt@200: description: '' diff --git a/tools/tests/merger/fixtures/spec/namespaces/indices.yaml b/tools/tests/merger/fixtures/spec/namespaces/indices.yaml index 0dafa08e7..d5f1fcb9c 100644 --- a/tools/tests/merger/fixtures/spec/namespaces/indices.yaml +++ b/tools/tests/merger/fixtures/spec/namespaces/indices.yaml @@ -19,7 +19,9 @@ paths: x-version-added: '2.0' components: requestBodies: - indices.create: {} + indices.create: + name: + type: string parameters: indices.create::path.index: name: index diff --git a/tools/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index 80211c140..e8c78cc65 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -29,7 +29,7 @@ describe('merged API spec', () => { test('has all responses', () => { expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' + '200', '201', '404', '500','503', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' ]) }) @@ -68,7 +68,7 @@ describe('merged API spec', () => { test('has matching responses', () => { expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'removed-2.0', 'added-1.3-removed-2.0' + '200', '201', '404', '500', '503', 'removed-2.0', 'added-1.3-removed-2.0' ]) }) }) @@ -78,7 +78,7 @@ describe('merged API spec', () => { test('has matching responses', () => { expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0' + '200', '201', '404', '500', '503', 'added-2.0' ]) }) }) @@ -88,7 +88,7 @@ describe('merged API spec', () => { test('has matching responses', () => { expect(_.keys(spec.spec().paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', 'added-2.0', 'added-2.1' + '200', '201', '404', '500', '503', 'added-2.0', 'added-2.1' ]) }) }) diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml index 141ca5ac5..e578781f1 100644 --- a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml @@ -26,6 +26,8 @@ paths: $ref: '#/components/responses/info@added-2.1' '500': $ref: '#/components/responses/info@500' + '503': + $ref: '#/components/responses/info@503' components: responses: info@200: diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml index 87abb9dbe..0fae3a8ca 100644 --- a/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml @@ -24,13 +24,16 @@ paths: post: operationId: nodes.info.1 x-operation-group: nodes.info - x-version-added: '1.0' + x-version-added: '2.0' description: Returns information about nodes in the cluster. parameters: - $ref: '#/components/parameters/nodes.info::path.id' + - $ref: '#/components/parameters/nodes.info::query.flag' requestBody: $ref: '#/components/requestBodies/nodes.info' responses: + '201': + $ref: '#/components/responses/nodes.info@201' '200': $ref: '#/components/responses/nodes.info@200' components: @@ -52,6 +55,14 @@ components: required: true schema: type: string + nodes.info::query.flag: + in: query + name: flag + description: Flag. + required: false + schema: + type: boolean + default: false responses: nodes.info@200: description: All nodes. @@ -59,3 +70,9 @@ components: application/json: schema: type: object + nodes.info@201: + description: All nodes. + content: + application/json: + schema: + type: object From 6d176b51d6e0d637231d75e102670026abd771c5 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 23 Jul 2024 18:16:32 -0400 Subject: [PATCH 06/13] Simplify deletes. Signed-off-by: dblock --- eslint.config.mjs | 2 +- tools/src/helpers.ts | 9 +-- tools/src/merger/OpenApiVersionExtractor.ts | 68 ++++++++----------- tools/tests/helpers.test.ts | 18 ++--- .../merger/OpenApiVersionExtractor.test.ts | 32 ++++----- 5 files changed, 55 insertions(+), 74 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index f8b145316..337f14721 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -56,7 +56,7 @@ export default [ { selector: 'typeProperty', format: null } ], '@typescript-eslint/no-confusing-void-expression': 'error', - '@typescript-eslint/no-dynamic-delete': 'error', + '@typescript-eslint/no-dynamic-delete': 'off', '@typescript-eslint/no-invalid-void-type': 'error', '@typescript-eslint/no-non-null-assertion': 'error', '@typescript-eslint/no-unnecessary-type-assertion': 'error', diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 3d5ef2e40..801e49b1f 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -47,7 +47,6 @@ export function sort_by_keys (obj: Record, priorities: string[] = [ return a[0].localeCompare(b[0]) }) sorted.forEach(([k, v]) => { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete obj[k] obj[k] = v }) @@ -65,21 +64,17 @@ export function sort_array_by_keys (values: any[], priorities: string[] = []): s }) } -export function delete_matching_keys(obj: any, condition: (obj: any) => boolean): string[] { - let removed: string[] = [] +export function delete_matching_keys(obj: any, condition: (obj: any) => boolean): void { for (const key in obj) { var item = obj[key] if (_.isObject(item)) { if (condition(item)) { - removed.push(key) - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete obj[key] } else { - removed = _.concat(removed, delete_matching_keys(item, condition)) + delete_matching_keys(item, condition) } } } - return removed } export function ensure_parent_dir (file_path: string): void { diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 560a962bc..9038930f8 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -9,7 +9,7 @@ import { Logger } from '../Logger' import { delete_matching_keys, write_yaml } from '../helpers' -import _, { isEmpty } from 'lodash' +import _ from 'lodash' import { type OpenAPIV3 } from 'openapi-types' import semver from 'semver' @@ -50,34 +50,7 @@ export default class OpenApiVersionExtractor { this._spec = _.cloneDeep(this._source_spec) - this._spec.components = this._spec.components ?? { - parameters: {}, - requestBodies: {}, - responses: {}, - schemas: {} - } - - this.#remove_keys_not_matching_semver(this._spec.paths) - - // parameters - const removed_params = this.#remove_keys_not_matching_semver(this._spec.components.parameters) - const removed_parameter_refs = _.map(removed_params, (ref) => `#/components/parameters/${ref}`) - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - method_item.parameters = _.filter(method_item.parameters, (param) => !_.includes(removed_parameter_refs, param.$ref)) - }) - }) - - // responses - const removed_responses = this.#remove_keys_not_matching_semver(this._spec.components.responses) - const removed_response_refs = _.map(removed_responses, (ref) => `#/components/responses/${ref}`) - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - method_item.responses = _.omitBy(method_item.responses, (param) => _.includes(removed_response_refs, param.$ref)) - }) - }) - - this._spec.paths = _.omitBy(this._spec.paths, isEmpty) + this.#remove_keys_not_matching_semver() this.#remove_unused_bodies() this.#remove_unused_parameters() @@ -100,9 +73,9 @@ export default class OpenApiVersionExtractor { } // Remove any elements that are x-version-added/removed incompatible with the target server version. - #remove_keys_not_matching_semver(obj: any): string[] { - if (this._target_version === undefined) return [] - return delete_matching_keys(obj, this.#exclude_per_semver.bind(this)) + #remove_keys_not_matching_semver(): void { + if (this._target_version === undefined) return + delete_matching_keys(this._spec, this.#exclude_per_semver.bind(this)) } #remove_unused_bodies(): void { @@ -110,9 +83,14 @@ export default class OpenApiVersionExtractor { var used_bodies: string[] = [] Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - if (method_item.requestBody?.$ref !== undefined) { - used_bodies = _.concat(used_bodies, method_item.requestBody.$ref) + Object.entries(path_item as Document).forEach(([method, method_item]) => { + const ref = method_item.requestBody?.$ref + if (ref !== undefined) { + if (this._spec?.components.requestBodies[ref.split('/').pop()] !== undefined) { + used_bodies = _.concat(used_bodies, method_item.requestBody.$ref) + } else { + delete path_item[method] + } } }) }) @@ -122,16 +100,20 @@ export default class OpenApiVersionExtractor { ) } - #remove_unused_responses(): void { if (this._spec === undefined) return var used_responses: string[] = [] Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { Object.entries(path_item as Document).forEach(([_method, method_item]) => { - Object.entries(method_item.responses as Document).forEach(([_response_code, response]) => { + Object.entries(method_item.responses as Document).forEach(([response_code, response]) => { + const ref = response.$ref if (response.$ref !== undefined) { - used_responses = _.concat(used_responses, response.$ref) + if (this._spec?.components.responses[ref.split('/').pop()] !== undefined) { + used_responses = _.concat(used_responses, ref) + } else { + delete method_item.responses[response_code] + } } }) }) @@ -142,16 +124,20 @@ export default class OpenApiVersionExtractor { ) } - #remove_unused_parameters(): void { if (this._spec === undefined) return var used_parameters: string[] = [] Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { Object.entries(path_item as Document).forEach(([_method, method_item]) => { - Object.entries(method_item.parameters as Document).forEach(([_response_code, parameter]) => { + Object.entries(method_item.parameters as Document).forEach(([response_code, parameter]) => { + const ref = parameter.$ref if (parameter.$ref !== undefined) { - used_parameters = _.concat(used_parameters, parameter.$ref) + if (this._spec?.components.parameters[ref.split('/').pop()] !== undefined) { + used_parameters = _.concat(used_parameters, parameter.$ref) + } else { + delete method_item.parameters[response_code] + } } }) }) diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index 5268b55eb..5022a2e44 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -41,7 +41,7 @@ describe('helpers', () => { describe('delete_matching_keys', () => { test('empty collection', () => { var obj = {} - expect(delete_matching_keys(obj, (_obj) => false)).toEqual([]) + delete_matching_keys(obj, (_obj) => false) expect(obj).toEqual({}) }) @@ -64,23 +64,23 @@ describe('helpers', () => { }) test('removes all keys', () => { - expect(delete_matching_keys(obj, (_item) => true)).toEqual(['foo', 'zar']) + delete_matching_keys(obj, (_item) => true) expect(obj).toStrictEqual({}) }) test('removes no keys', () => { const obj2 = _.cloneDeep(obj) - expect(delete_matching_keys(obj, (_item) => false)).toEqual([]) + delete_matching_keys(obj, (_item) => false) expect(obj).toStrictEqual(obj2) }) test('removes a value from a key', () => { - expect(delete_matching_keys(obj, (_item: any) => _item.x == 1)).toEqual(['bar1']) + delete_matching_keys(obj, (_item: any) => _item.x == 1) expect(obj).toStrictEqual({ foo: {}, zar: { bar2: { y: 2 } } }) }) test('removes multiple values from a key', () => { - expect(delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2)).toEqual(['bar1', 'bar2']) + delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2) expect(obj).toStrictEqual({ foo: {}, zar: {} }) }) }) @@ -102,23 +102,23 @@ describe('helpers', () => { }) test('removes all keys', () => { - expect(delete_matching_keys(obj, (_item) => true)).toEqual(['foo']) + delete_matching_keys(obj, (_item) => true) expect(obj).toStrictEqual({}) }) test('removes no keys', () => { const obj2 = _.cloneDeep(obj) - expect(delete_matching_keys(obj, (_item) => false)).toEqual([]) + delete_matching_keys(obj, (_item) => false) expect(obj).toStrictEqual(obj2) }) test('removes a value from a key', () => { - expect(delete_matching_keys(obj, (_item: any) => _item.x == 1)).toEqual(['bar1']) + delete_matching_keys(obj, (_item: any) => _item.x == 1) expect(obj).toStrictEqual({ foo: [{ bar2: { y: 2 } }] }) }) test('removes multiple values from a key', () => { - expect(delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2)).toEqual(['bar1', 'bar2']) + delete_matching_keys(obj, (_item: any) => _item.x == 1 || _item.y == 2) expect(obj).toStrictEqual({ foo: [{}] }) }) }) diff --git a/tools/tests/merger/OpenApiVersionExtractor.test.ts b/tools/tests/merger/OpenApiVersionExtractor.test.ts index 2a5bd28a9..77dcd9a74 100644 --- a/tools/tests/merger/OpenApiVersionExtractor.test.ts +++ b/tools/tests/merger/OpenApiVersionExtractor.test.ts @@ -79,27 +79,27 @@ describe('extract() from a merged API spec', () => { '200', '201', '404', '500', '503', 'removed-2.0', 'added-1.3-removed-2.0' ]) }) + }) - describe('2.0', () => { - const extractor = new OpenApiVersionExtractor(merger.spec(), '2.0') + describe('2.0', () => { + const extractor = new OpenApiVersionExtractor(merger.spec(), '2.0') - test('has matching responses', () => { - const spec = extractor.extract() - expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', '503', 'added-2.0' - ]) - }) + test('has matching responses', () => { + const spec = extractor.extract() + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', '503', 'added-2.0' + ]) }) + }) - describe('2.1', () => { - const extractor = new OpenApiVersionExtractor(merger.spec(), '2.1') + describe('2.1', () => { + const extractor = new OpenApiVersionExtractor(merger.spec(), '2.1') - test('has matching responses', () => { - const spec = extractor.extract() - expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', '503', 'added-2.0', 'added-2.1' - ]) - }) + test('has matching responses', () => { + const spec = extractor.extract() + expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ + '200', '201', '404', '500', '503', 'added-2.0', 'added-2.1' + ]) }) }) }) From d81dec7f03ded54c949ab59b4ac51324904edaf9 Mon Sep 17 00:00:00 2001 From: dblock Date: Wed, 24 Jul 2024 10:17:38 -0400 Subject: [PATCH 07/13] Generalize removing unused references. Signed-off-by: dblock --- tools/src/helpers.ts | 14 ++ tools/src/merger/OpenApiVersionExtractor.ts | 83 +++------ tools/tests/helpers.test.ts | 30 ++- .../merger/OpenApiVersionExtractor.test.ts | 23 ++- .../fixtures/extractor/expected_1.3.yaml | 7 +- .../fixtures/extractor/expected_2.0.yaml | 16 -- .../fixtures/extractor/expected_default.yaml | 173 ++++++++++++++++++ .../specs/complete/namespaces/index.yaml | 2 + .../specs/complete/schemas/_common.yaml | 10 + 9 files changed, 277 insertions(+), 81 deletions(-) create mode 100644 tools/tests/merger/fixtures/extractor/expected_default.yaml create mode 100644 tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 801e49b1f..f584629dd 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -77,6 +77,20 @@ export function delete_matching_keys(obj: any, condition: (obj: any) => boolean) } } +export function find_refs(obj: Record): string[] { + let results: string[] = [] + + if (obj !== undefined && obj !== null && obj.$ref !== undefined) { + results.push(obj.$ref as string) + } + + if (_.isObject(obj)) { + results = _.concat(results, _.flatMap(obj, (v, _k) => find_refs(v as Record))) + } + + return _.uniq(results) +} + export function ensure_parent_dir (file_path: string): void { fs.mkdirSync(path.dirname(file_path), { recursive: true }) } diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 9038930f8..2be36d4e7 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -7,9 +7,9 @@ * compatible open source license. */ -import { Logger } from '../Logger' -import { delete_matching_keys, write_yaml } from '../helpers' import _ from 'lodash' +import { delete_matching_keys, find_refs, write_yaml } from '../helpers' +import { Logger } from '../Logger' import { type OpenAPIV3 } from 'openapi-types' import semver from 'semver' @@ -51,10 +51,7 @@ export default class OpenApiVersionExtractor { this._spec = _.cloneDeep(this._source_spec) this.#remove_keys_not_matching_semver() - - this.#remove_unused_bodies() - this.#remove_unused_parameters() - this.#remove_unused_responses() + this.#remove_unused() } #exclude_per_semver(obj: any): boolean { @@ -78,73 +75,35 @@ export default class OpenApiVersionExtractor { delete_matching_keys(this._spec, this.#exclude_per_semver.bind(this)) } - #remove_unused_bodies(): void { + #remove_unused(): void { if (this._spec === undefined) return - var used_bodies: string[] = [] - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([method, method_item]) => { - const ref = method_item.requestBody?.$ref - if (ref !== undefined) { - if (this._spec?.components.requestBodies[ref.split('/').pop()] !== undefined) { - used_bodies = _.concat(used_bodies, method_item.requestBody.$ref) - } else { - delete path_item[method] - } - } - }) - }) + // remove anything that's not referenced + var references: string[] = find_refs(this._spec) - this._spec.components.requestBodies = _.pickBy(this._spec.components.requestBodies, (_value, key) => - _.includes(used_bodies, `#/components/requestBodies/${key}`) + this._spec.components.schemas = _.pickBy(this._spec.components.schemas, (_value, key) => + _.includes(references, `#/components/schemas/${key}`) ) - } - - #remove_unused_responses(): void { - if (this._spec === undefined) return - var used_responses: string[] = [] - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - Object.entries(method_item.responses as Document).forEach(([response_code, response]) => { - const ref = response.$ref - if (response.$ref !== undefined) { - if (this._spec?.components.responses[ref.split('/').pop()] !== undefined) { - used_responses = _.concat(used_responses, ref) - } else { - delete method_item.responses[response_code] - } - } - }) - }) - }) + this._spec.components.parameters = _.pickBy(this._spec.components.parameters, (_value, key) => + _.includes(references, `#/components/parameters/${key}`) + ) this._spec.components.responses = _.pickBy(this._spec.components.responses, (_value, key) => - _.includes(used_responses, `#/components/responses/${key}`) + _.includes(references, `#/components/responses/${key}`) ) - } - #remove_unused_parameters(): void { - if (this._spec === undefined) return + this._spec.components.requestBodies = _.pickBy(this._spec.components.requestBodies, (_value, key) => + _.includes(references, `#/components/requestBodies/${key}`) + ) - var used_parameters: string[] = [] - Object.entries(this._spec.paths as Document).forEach(([_path, path_item]) => { - Object.entries(path_item as Document).forEach(([_method, method_item]) => { - Object.entries(method_item.parameters as Document).forEach(([response_code, parameter]) => { - const ref = parameter.$ref - if (parameter.$ref !== undefined) { - if (this._spec?.components.parameters[ref.split('/').pop()] !== undefined) { - used_parameters = _.concat(used_parameters, parameter.$ref) - } else { - delete method_item.parameters[response_code] - } - } - }) - }) - }) + // collect what's left + var remaining = _.flatMap(['schemas', 'parameters', 'responses', 'requestBodies'], (key) => + _.keys(this._spec?.components?.[key]).map((ref) => `#/components/${key}/${ref}`) + ) - this._spec.components.parameters = _.pickBy(this._spec.components.parameters, (_value, key) => - _.includes(used_parameters, `#/components/parameters/${key}`) + delete_matching_keys(this._spec, (obj) => + obj.$ref !== undefined && !_.includes(remaining, obj.$ref) ) } } diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index 5022a2e44..1991d21b6 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -8,7 +8,7 @@ */ import _ from 'lodash' -import { delete_matching_keys, sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' +import { delete_matching_keys, find_refs, sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' describe('helpers', () => { describe('sort_array_by_keys', () => { @@ -123,4 +123,32 @@ describe('helpers', () => { }) }) }) + + describe('find_refs', () => { + test('empty collection', () => { + expect(find_refs({})).toEqual([]) + }) + + test('with refs', () => { + expect(find_refs({ + $ref: 1, + null: null, + undefined, + obj: { + $ref: 2, + obj: { + $ref: 3 + } + }, + arr: [{ + obj1: { + $ref: 'dup', + }, + obj2: { + $ref: 'dup', + }, + }] + })).toEqual([1, 2, 3, 'dup']) + }) + }) }) diff --git a/tools/tests/merger/OpenApiVersionExtractor.test.ts b/tools/tests/merger/OpenApiVersionExtractor.test.ts index 77dcd9a74..508f29a07 100644 --- a/tools/tests/merger/OpenApiVersionExtractor.test.ts +++ b/tools/tests/merger/OpenApiVersionExtractor.test.ts @@ -43,7 +43,7 @@ describe('extract() from a merged API spec', () => { test('writes a spec', () => { extractor.write_to(filename) - expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_2.0.yaml', 'utf8')) + expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_default.yaml', 'utf8')) .toEqual(fs.readFileSync(filename, 'utf8')) }) }) @@ -90,6 +90,27 @@ describe('extract() from a merged API spec', () => { '200', '201', '404', '500', '503', 'added-2.0' ]) }) + + describe('write_to()', () => { + var temp: tmp.DirResult + var filename: string + + beforeEach(() => { + temp = tmp.dirSync() + filename = `${temp.name}/opensearch-openapi.yaml` + }) + + afterEach(() => { + fs.unlinkSync(filename) + temp.removeCallback() + }) + + test('writes a spec', () => { + extractor.write_to(filename) + expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_2.0.yaml', 'utf8')) + .toEqual(fs.readFileSync(filename, 'utf8')) + }) + }) }) describe('2.1', () => { diff --git a/tools/tests/merger/fixtures/extractor/expected_1.3.yaml b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml index 42791f2ee..81eecb289 100644 --- a/tools/tests/merger/fixtures/extractor/expected_1.3.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_1.3.yaml @@ -61,6 +61,8 @@ components: schema: type: object properties: + _type: + $ref: '#/components/schemas/_common:Type' tagline: type: string required: @@ -117,4 +119,7 @@ components: application/json: schema: type: object - schemas: {} + schemas: + _common:Type: + type: string + x-version-removed: '2.0' diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml index 6cb54f13b..796499e6b 100644 --- a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -48,13 +48,6 @@ paths: added-2.0: $ref: '#/components/responses/info@added-2.0' x-version-added: '2.0' - removed-2.0: - $ref: '#/components/responses/info@removed-2.0' - x-version-removed: '2.0' - added-1.3-removed-2.0: - $ref: '#/components/responses/info@added-1.3-removed-2.0' - added-2.1: - $ref: '#/components/responses/info@added-2.1' parameters: [] /nodes: get: @@ -142,17 +135,8 @@ components: application/json: schema: type: object - info@added-1.3-removed-2.0: - description: Added in 1.3, removed in 2.0 via attribute in response body. - x-version-added: '1.3' - x-version-removed: '2.0' info@added-2.0: description: Added in 2.0 via attribute next to ref. - info@added-2.1: - description: Added in 2.1 via attribute in response body. - x-version-added: '2.1' - info@removed-2.0: - description: Removed in 2.0 via attribute next to ref. nodes.info@200: description: All nodes. content: diff --git a/tools/tests/merger/fixtures/extractor/expected_default.yaml b/tools/tests/merger/fixtures/extractor/expected_default.yaml new file mode 100644 index 000000000..ebc5054d7 --- /dev/null +++ b/tools/tests/merger/fixtures/extractor/expected_default.yaml @@ -0,0 +1,173 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 + x-api-version: 1.2.3 +paths: + /_nodes/{id}: + get: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + post: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '2.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + - $ref: '#/components/parameters/nodes.info::query.flag' + requestBody: + $ref: '#/components/requestBodies/nodes.info' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + '201': + $ref: '#/components/responses/nodes.info@201' + /index: + get: + operationId: get.0 + responses: + '200': + $ref: '#/components/responses/info@200' + '201': + $ref: '#/components/responses/info@201' + '404': + $ref: '#/components/responses/info@404' + '500': + $ref: '#/components/responses/info@500' + '503': + $ref: '#/components/responses/info@503' + added-2.0: + $ref: '#/components/responses/info@added-2.0' + x-version-added: '2.0' + removed-2.0: + $ref: '#/components/responses/info@removed-2.0' + x-version-removed: '2.0' + added-1.3-removed-2.0: + $ref: '#/components/responses/info@added-1.3-removed-2.0' + added-2.1: + $ref: '#/components/responses/info@added-2.1' + parameters: [] + /nodes: + get: + operationId: nodes.0 + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + parameters: [] +components: + parameters: + nodes.info::path.id: + in: path + name: id + description: Node ID. + required: true + schema: + type: string + nodes.info::query.flag: + in: query + name: flag + description: Flag. + required: false + schema: + type: boolean + default: false + requestBodies: + nodes.info: + content: + application/json: + schema: + type: object + properties: + _all: + type: boolean + description: Nodes options. + responses: + info@200: + description: '' + content: + application/json: + schema: + type: object + properties: + _type: + $ref: '#/components/schemas/_common:Type' + tagline: + type: string + required: + - tagline + info@201: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: true + info@404: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + required: + - tagline + unevaluatedProperties: + type: object + info@500: + description: '' + content: + application/json: + schema: + type: object + properties: + tagline: + type: string + info@503: + description: '' + content: + application/json: + schema: + type: object + info@added-1.3-removed-2.0: + description: Added in 1.3, removed in 2.0 via attribute in response body. + x-version-added: '1.3' + x-version-removed: '2.0' + info@added-2.0: + description: Added in 2.0 via attribute next to ref. + info@added-2.1: + description: Added in 2.1 via attribute in response body. + x-version-added: '2.1' + info@removed-2.0: + description: Removed in 2.0 via attribute next to ref. + nodes.info@200: + description: All nodes. + content: + application/json: + schema: + type: object + nodes.info@201: + description: All nodes. + content: + application/json: + schema: + type: object + schemas: + _common:Type: + type: string + x-version-removed: '2.0' diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml index e578781f1..a2402b356 100644 --- a/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/index.yaml @@ -37,6 +37,8 @@ components: schema: type: object properties: + _type: + $ref: '../schemas/_common.yaml#/components/schemas/Type' tagline: type: string required: diff --git a/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml new file mode 100644 index 000000000..fcaf29113 --- /dev/null +++ b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml @@ -0,0 +1,10 @@ +openapi: 3.1.0 +info: + title: Schemas of _common category + description: Schemas of _common category + version: 1.0.0 +components: + schemas: + Type: + type: string + x-version-removed: '2.0' From 02eb4094c418cdf564c529cddad2be52593253b5 Mon Sep 17 00:00:00 2001 From: dblock Date: Wed, 24 Jul 2024 10:44:00 -0400 Subject: [PATCH 08/13] Pick and extend to dry. Signed-off-by: dblock --- tools/src/merger/OpenApiVersionExtractor.ts | 25 ++++++++------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 2be36d4e7..05290c289 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -7,7 +7,7 @@ * compatible open source license. */ -import _ from 'lodash' +import _, { extend } from 'lodash' import { delete_matching_keys, find_refs, write_yaml } from '../helpers' import { Logger } from '../Logger' import { type OpenAPIV3 } from 'openapi-types' @@ -81,21 +81,14 @@ export default class OpenApiVersionExtractor { // remove anything that's not referenced var references: string[] = find_refs(this._spec) - this._spec.components.schemas = _.pickBy(this._spec.components.schemas, (_value, key) => - _.includes(references, `#/components/schemas/${key}`) - ) - - this._spec.components.parameters = _.pickBy(this._spec.components.parameters, (_value, key) => - _.includes(references, `#/components/parameters/${key}`) - ) - - this._spec.components.responses = _.pickBy(this._spec.components.responses, (_value, key) => - _.includes(references, `#/components/responses/${key}`) - ) - - this._spec.components.requestBodies = _.pickBy(this._spec.components.requestBodies, (_value, key) => - _.includes(references, `#/components/requestBodies/${key}`) - ) + this._spec.components = _.reduce(_.map(['parameters', 'requestBodies', 'responses', 'schemas'], (p) => + { + return { + [p]: _.pickBy(this._spec?.components?.[p], (_value, key) => + _.includes(references, `#/components/${p}/${key}`)) + } + } + ), extend) // collect what's left var remaining = _.flatMap(['schemas', 'parameters', 'responses', 'requestBodies'], (key) => From 203550b319ff3dfe7d9aecd0a2bf71c3d63424a5 Mon Sep 17 00:00:00 2001 From: dblock Date: Wed, 24 Jul 2024 10:53:55 -0400 Subject: [PATCH 09/13] Remove any paths that had all their verbs removed. Signed-off-by: dblock --- tools/src/merger/OpenApiVersionExtractor.ts | 4 +++- .../merger/fixtures/extractor/expected_2.0.yaml | 9 +++++++++ .../fixtures/extractor/expected_default.yaml | 9 +++++++++ tools/tests/tester/MergedOpenApiSpec.test.ts | 1 + tools/tests/tester/ResultLogger.test.ts | 2 +- tools/tests/tester/TestResults.test.ts | 6 +++--- .../complete/namespaces/cluster_manager.yaml | 17 +++++++++++++++++ 7 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 tools/tests/tester/fixtures/specs/complete/namespaces/cluster_manager.yaml diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 05290c289..02b634994 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -7,7 +7,7 @@ * compatible open source license. */ -import _, { extend } from 'lodash' +import _, { extend, isEmpty } from 'lodash' import { delete_matching_keys, find_refs, write_yaml } from '../helpers' import { Logger } from '../Logger' import { type OpenAPIV3 } from 'openapi-types' @@ -98,5 +98,7 @@ export default class OpenApiVersionExtractor { delete_matching_keys(this._spec, (obj) => obj.$ref !== undefined && !_.includes(remaining, obj.$ref) ) + + this._spec.paths = _.omitBy(this._spec.paths, isEmpty) } } diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml index 796499e6b..e32ad78a4 100644 --- a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -31,6 +31,15 @@ paths: $ref: '#/components/responses/nodes.info@200' '201': $ref: '#/components/responses/nodes.info@201' + /cluster_manager: + get: + operationId: cluster_manager.0 + x-version-added: '2.0' + parameters: [] + post: + operationId: cluster_manager.0 + x-version-added: '2.0' + parameters: [] /index: get: operationId: get.0 diff --git a/tools/tests/merger/fixtures/extractor/expected_default.yaml b/tools/tests/merger/fixtures/extractor/expected_default.yaml index ebc5054d7..158e00020 100644 --- a/tools/tests/merger/fixtures/extractor/expected_default.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_default.yaml @@ -31,6 +31,15 @@ paths: $ref: '#/components/responses/nodes.info@200' '201': $ref: '#/components/responses/nodes.info@201' + /cluster_manager: + get: + operationId: cluster_manager.0 + x-version-added: '2.0' + parameters: [] + post: + operationId: cluster_manager.0 + x-version-added: '2.0' + parameters: [] /index: get: operationId: get.0 diff --git a/tools/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index e8c78cc65..7e6c64641 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -22,6 +22,7 @@ describe('merged API spec', () => { test('paths', () => { expect(spec.paths()).toEqual({ '/_nodes/{id}': ['get', 'post'], + '/cluster_manager': ['get', 'post'], '/index': ['get'], '/nodes': ['get'] }) diff --git a/tools/tests/tester/ResultLogger.test.ts b/tools/tests/tester/ResultLogger.test.ts index 2eea82b2d..1d2392bbd 100644 --- a/tools/tests/tester/ResultLogger.test.ts +++ b/tools/tests/tester/ResultLogger.test.ts @@ -75,7 +75,7 @@ describe('ConsoleResultLogger', () => { expect(log.mock.calls).toEqual([ [], - ['Tested 1/4 paths.'] + ['Tested 1/6 paths.'] ]) }) }) diff --git a/tools/tests/tester/TestResults.test.ts b/tools/tests/tester/TestResults.test.ts index fb8b25853..ac06a6350 100644 --- a/tools/tests/tester/TestResults.test.ts +++ b/tools/tests/tester/TestResults.test.ts @@ -39,7 +39,7 @@ describe('TestResults', () => { }) test('spec_paths_count', () => { - expect(test_results.spec_paths_count()).toEqual(4) + expect(test_results.spec_paths_count()).toEqual(6) }) test('write_coverage', () => { @@ -47,8 +47,8 @@ describe('TestResults', () => { test_results.write_coverage(filename) expect(JSON.parse(fs.readFileSync(filename, 'utf8'))).toEqual({ evaluated_paths_count: 1, - evaluated_paths_pct: 25, - paths_count: 4 + evaluated_paths_pct: 16.67, + paths_count: 6 }) fs.unlinkSync(filename) }) diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/cluster_manager.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/cluster_manager.yaml new file mode 100644 index 000000000..680169ad6 --- /dev/null +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/cluster_manager.yaml @@ -0,0 +1,17 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + /cluster_manager: + get: + operationId: cluster_manager.0 + x-version-added: '2.0' + post: + operationId: cluster_manager.0 + x-version-added: '2.0' +components: + requestBodies: [] + parameters: [] + responses: [] From 74cdcde8b61a5fef162b71050b58f749b2e743c0 Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 25 Jul 2024 17:20:02 -0400 Subject: [PATCH 10/13] Require target_version. Signed-off-by: dblock --- tools/src/merger/OpenApiVersionExtractor.ts | 19 +- .../merger/OpenApiVersionExtractor.test.ts | 33 ---- .../fixtures/extractor/expected_default.yaml | 182 ------------------ 3 files changed, 5 insertions(+), 229 deletions(-) delete mode 100644 tools/tests/merger/fixtures/extractor/expected_default.yaml diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 02b634994..2c0ce0f57 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -17,23 +17,20 @@ import semver from 'semver' export default class OpenApiVersionExtractor { private _spec?: Record private _source_spec: OpenAPIV3.Document - private _target_version?: string + private _target_version: string private _logger: Logger - constructor(source_spec: OpenAPIV3.Document, target_version?: string, logger: Logger = new Logger()) { + constructor(source_spec: OpenAPIV3.Document, target_version: string, logger: Logger = new Logger()) { this._source_spec = source_spec - this._target_version = target_version !== undefined ? semver.coerce(target_version)?.toString() : undefined + this._target_version = semver.coerce(target_version)?.toString() ?? target_version this._logger = logger this._spec = undefined } extract(): OpenAPIV3.Document { if (this._spec) return this._spec as OpenAPIV3.Document - if (this._target_version !== undefined) { - this.#extract() - } else { - this._spec = this._source_spec - } + this._spec = _.cloneDeep(this._source_spec) + this.#extract() return this._spec as OpenAPIV3.Document } @@ -47,16 +44,11 @@ export default class OpenApiVersionExtractor { // incompatible with the target server version. #extract() : void { this._logger.info(`Extracting version ${this._target_version} ...`) - - this._spec = _.cloneDeep(this._source_spec) - this.#remove_keys_not_matching_semver() this.#remove_unused() } #exclude_per_semver(obj: any): boolean { - if (this._target_version === undefined) return false - const x_version_added = semver.coerce(obj['x-version-added'] as string) const x_version_removed = semver.coerce(obj['x-version-removed'] as string) @@ -71,7 +63,6 @@ export default class OpenApiVersionExtractor { // Remove any elements that are x-version-added/removed incompatible with the target server version. #remove_keys_not_matching_semver(): void { - if (this._target_version === undefined) return delete_matching_keys(this._spec, this.#exclude_per_semver.bind(this)) } diff --git a/tools/tests/merger/OpenApiVersionExtractor.test.ts b/tools/tests/merger/OpenApiVersionExtractor.test.ts index 508f29a07..61bc42622 100644 --- a/tools/tests/merger/OpenApiVersionExtractor.test.ts +++ b/tools/tests/merger/OpenApiVersionExtractor.test.ts @@ -16,39 +16,6 @@ import tmp from 'tmp' describe('extract() from a merged API spec', () => { const merger = new OpenApiMerger('tools/tests/tester/fixtures/specs/complete') - describe('defaults', () => { - const extractor = new OpenApiVersionExtractor(merger.spec()) - - test('has all responses', () => { - const spec = extractor.extract() - - expect(_.keys(spec.paths['/index']?.get?.responses)).toEqual([ - '200', '201', '404', '500', '503', 'added-2.0', 'removed-2.0', 'added-1.3-removed-2.0', 'added-2.1' - ]) - }) - - describe('write_to()', () => { - var temp: tmp.DirResult - var filename: string - - beforeEach(() => { - temp = tmp.dirSync() - filename = `${temp.name}/opensearch-openapi.yaml` - }) - - afterEach(() => { - fs.unlinkSync(filename) - temp.removeCallback() - }) - - test('writes a spec', () => { - extractor.write_to(filename) - expect(fs.readFileSync('./tools/tests/merger/fixtures/extractor/expected_default.yaml', 'utf8')) - .toEqual(fs.readFileSync(filename, 'utf8')) - }) - }) - }) - describe('1.3', () => { const extractor = new OpenApiVersionExtractor(merger.spec(), '1.3') diff --git a/tools/tests/merger/fixtures/extractor/expected_default.yaml b/tools/tests/merger/fixtures/extractor/expected_default.yaml deleted file mode 100644 index 158e00020..000000000 --- a/tools/tests/merger/fixtures/extractor/expected_default.yaml +++ /dev/null @@ -1,182 +0,0 @@ -openapi: 3.1.0 -info: - title: OpenSearch API - description: OpenSearch API - version: 1.0.0 - x-api-version: 1.2.3 -paths: - /_nodes/{id}: - get: - operationId: nodes.info.1 - x-operation-group: nodes.info - x-version-added: '1.0' - description: Returns information about nodes in the cluster. - parameters: - - $ref: '#/components/parameters/nodes.info::path.id' - responses: - '200': - $ref: '#/components/responses/nodes.info@200' - post: - operationId: nodes.info.1 - x-operation-group: nodes.info - x-version-added: '2.0' - description: Returns information about nodes in the cluster. - parameters: - - $ref: '#/components/parameters/nodes.info::path.id' - - $ref: '#/components/parameters/nodes.info::query.flag' - requestBody: - $ref: '#/components/requestBodies/nodes.info' - responses: - '200': - $ref: '#/components/responses/nodes.info@200' - '201': - $ref: '#/components/responses/nodes.info@201' - /cluster_manager: - get: - operationId: cluster_manager.0 - x-version-added: '2.0' - parameters: [] - post: - operationId: cluster_manager.0 - x-version-added: '2.0' - parameters: [] - /index: - get: - operationId: get.0 - responses: - '200': - $ref: '#/components/responses/info@200' - '201': - $ref: '#/components/responses/info@201' - '404': - $ref: '#/components/responses/info@404' - '500': - $ref: '#/components/responses/info@500' - '503': - $ref: '#/components/responses/info@503' - added-2.0: - $ref: '#/components/responses/info@added-2.0' - x-version-added: '2.0' - removed-2.0: - $ref: '#/components/responses/info@removed-2.0' - x-version-removed: '2.0' - added-1.3-removed-2.0: - $ref: '#/components/responses/info@added-1.3-removed-2.0' - added-2.1: - $ref: '#/components/responses/info@added-2.1' - parameters: [] - /nodes: - get: - operationId: nodes.0 - responses: - '200': - $ref: '#/components/responses/nodes.info@200' - parameters: [] -components: - parameters: - nodes.info::path.id: - in: path - name: id - description: Node ID. - required: true - schema: - type: string - nodes.info::query.flag: - in: query - name: flag - description: Flag. - required: false - schema: - type: boolean - default: false - requestBodies: - nodes.info: - content: - application/json: - schema: - type: object - properties: - _all: - type: boolean - description: Nodes options. - responses: - info@200: - description: '' - content: - application/json: - schema: - type: object - properties: - _type: - $ref: '#/components/schemas/_common:Type' - tagline: - type: string - required: - - tagline - info@201: - description: '' - content: - application/json: - schema: - type: object - properties: - tagline: - type: string - required: - - tagline - unevaluatedProperties: true - info@404: - description: '' - content: - application/json: - schema: - type: object - properties: - tagline: - type: string - required: - - tagline - unevaluatedProperties: - type: object - info@500: - description: '' - content: - application/json: - schema: - type: object - properties: - tagline: - type: string - info@503: - description: '' - content: - application/json: - schema: - type: object - info@added-1.3-removed-2.0: - description: Added in 1.3, removed in 2.0 via attribute in response body. - x-version-added: '1.3' - x-version-removed: '2.0' - info@added-2.0: - description: Added in 2.0 via attribute next to ref. - info@added-2.1: - description: Added in 2.1 via attribute in response body. - x-version-added: '2.1' - info@removed-2.0: - description: Removed in 2.0 via attribute next to ref. - nodes.info@200: - description: All nodes. - content: - application/json: - schema: - type: object - nodes.info@201: - description: All nodes. - content: - application/json: - schema: - type: object - schemas: - _common:Type: - type: string - x-version-removed: '2.0' From e661b32e78b2d0b9606e183f3d5636244ef47513 Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 25 Jul 2024 17:34:24 -0400 Subject: [PATCH 11/13] Return a Set from find_refs. Signed-off-by: dblock --- tools/src/helpers.ts | 17 +++++------------ tools/src/merger/OpenApiVersionExtractor.ts | 12 +++++++----- tools/tests/helpers.test.ts | 4 ++-- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index f584629dd..aabff55bf 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -77,18 +77,11 @@ export function delete_matching_keys(obj: any, condition: (obj: any) => boolean) } } -export function find_refs(obj: Record): string[] { - let results: string[] = [] - - if (obj !== undefined && obj !== null && obj.$ref !== undefined) { - results.push(obj.$ref as string) - } - - if (_.isObject(obj)) { - results = _.concat(results, _.flatMap(obj, (v, _k) => find_refs(v as Record))) - } - - return _.uniq(results) +export function find_refs (obj: any): Set { + var results = new Set() + if (obj?.$ref != null) results.add(obj.$ref as string) + if (_.isObject(obj)) _.forEach(obj, (v) => { find_refs(v).forEach((ref) => results.add(ref)); }) + return results } export function ensure_parent_dir (file_path: string): void { diff --git a/tools/src/merger/OpenApiVersionExtractor.ts b/tools/src/merger/OpenApiVersionExtractor.ts index 2c0ce0f57..5d4164b1c 100644 --- a/tools/src/merger/OpenApiVersionExtractor.ts +++ b/tools/src/merger/OpenApiVersionExtractor.ts @@ -52,9 +52,9 @@ export default class OpenApiVersionExtractor { const x_version_added = semver.coerce(obj['x-version-added'] as string) const x_version_removed = semver.coerce(obj['x-version-removed'] as string) - if (x_version_added && !semver.satisfies(this._target_version, `>=${x_version_added?.toString()}`)) { + if (x_version_added && !semver.satisfies(this._target_version, `>=${x_version_added.toString()}`)) { return true - } else if (x_version_removed && !semver.satisfies(this._target_version, `<${x_version_removed?.toString()}`)) { + } else if (x_version_removed && !semver.satisfies(this._target_version, `<${x_version_removed.toString()}`)) { return true } @@ -70,13 +70,15 @@ export default class OpenApiVersionExtractor { if (this._spec === undefined) return // remove anything that's not referenced - var references: string[] = find_refs(this._spec) + var references = find_refs(this._spec) this._spec.components = _.reduce(_.map(['parameters', 'requestBodies', 'responses', 'schemas'], (p) => { return { - [p]: _.pickBy(this._spec?.components?.[p], (_value, key) => - _.includes(references, `#/components/${p}/${key}`)) + [p]: _.pickBy( + this._spec?.components?.[p], (_value, key) => + references.has(`#/components/${p}/${key}`) + ) } } ), extend) diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index 1991d21b6..185ca6415 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -126,7 +126,7 @@ describe('helpers', () => { describe('find_refs', () => { test('empty collection', () => { - expect(find_refs({})).toEqual([]) + expect(find_refs({})).toEqual(new Set()) }) test('with refs', () => { @@ -148,7 +148,7 @@ describe('helpers', () => { $ref: 'dup', }, }] - })).toEqual([1, 2, 3, 'dup']) + })).toEqual(new Set([1, 2, 3, 'dup'])) }) }) }) From ed18c4d145a39c8a3c3cf2894dcfa66175102df8 Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 25 Jul 2024 19:25:19 -0400 Subject: [PATCH 12/13] Traverse $ref's recursively. Signed-off-by: dblock --- tools/src/helpers.ts | 23 +++++++- tools/tests/helpers.test.ts | 55 ++++++++++++++----- .../fixtures/extractor/expected_2.0.yaml | 15 ++++- .../specs/complete/namespaces/nodes.yaml | 12 ++++ .../specs/complete/schemas/_common.yaml | 10 ++++ 5 files changed, 96 insertions(+), 19 deletions(-) diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index aabff55bf..3a7bfacf6 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -77,10 +77,27 @@ export function delete_matching_keys(obj: any, condition: (obj: any) => boolean) } } -export function find_refs (obj: any): Set { +export function find_refs (current: Record, root?: Record): Set { var results = new Set() - if (obj?.$ref != null) results.add(obj.$ref as string) - if (_.isObject(obj)) _.forEach(obj, (v) => { find_refs(v).forEach((ref) => results.add(ref)); }) + + if (root === undefined) { + root = current + current = current.paths + } + + if (current?.$ref != null) { + const ref = current.$ref as string + results.add(ref) + const ref_node = resolve_ref(ref, root) + if (ref_node !== undefined) find_refs(ref_node, root).forEach((ref) => results.add(ref)) + } + + if (_.isObject(current)) { + _.forEach(current, (v) => { + find_refs(v as Record, root).forEach((ref) => results.add(ref)); + }) + } + return results } diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index 185ca6415..54773d226 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -131,24 +131,49 @@ describe('helpers', () => { test('with refs', () => { expect(find_refs({ - $ref: 1, - null: null, - undefined, - obj: { - $ref: 2, + paths: { + $ref: '#1', + null: null, + undefined, obj: { - $ref: 3 - } - }, - arr: [{ - obj1: { - $ref: 'dup', + $ref: '#2', + obj: { + $ref: '#3' + }, + schema_obj: { + $ref: '#/schemas/schema1' + } }, - obj2: { - $ref: 'dup', + arr: [{ + obj1: { + $ref: '#dup', + }, + obj2: { + $ref: '#dup', + }, + }] + }, + schemas: { + schema1: { + items: { + $ref: '#/schemas/schema2' + } }, - }] - })).toEqual(new Set([1, 2, 3, 'dup'])) + schema2: { + x: 1 + } + }, + unused1: { + $ref: '#unused2', + obj: { + $ref: '#unused3' + } + }, + '#1': {}, + '#2': {}, + '#3': {}, + '#dup': {} + })).toEqual(new Set(['#1', '#2', '#3', '#dup', '#/schemas/schema1', '#/schemas/schema2'])) }) }) }) diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml index e32ad78a4..d9c539fcf 100644 --- a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -91,6 +91,9 @@ components: properties: _all: type: boolean + ids: + $ref: '#/components/schemas/_common:Ids' + x-version-added: '2.0' description: Nodes options. responses: info@200: @@ -158,4 +161,14 @@ components: application/json: schema: type: object - schemas: {} + schemas: + _common:Ids: + oneOf: + - $ref: '#/components/schemas/_common:OldId' + - type: array + items: + $ref: '#/components/schemas/_common:NewId' + _common:NewId: + type: string + _common:OldId: + type: string diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml index 0fae3a8ca..2b39054ea 100644 --- a/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml @@ -46,6 +46,9 @@ components: properties: _all: type: boolean + ids: + $ref: '../schemas/_common.yaml#/components/schemas/Ids' + x-version-added: '2.0' description: Nodes options. parameters: nodes.info::path.id: @@ -63,6 +66,15 @@ components: schema: type: boolean default: false + nodes.info::query.new: + in: query + name: new + description: New option added in 2.1. + required: false + x-version-added: '2.1' + schema: + type: array + default: false responses: nodes.info@200: description: All nodes. diff --git a/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml index fcaf29113..8b6aae7ac 100644 --- a/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml +++ b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml @@ -8,3 +8,13 @@ components: Type: type: string x-version-removed: '2.0' + OldId: + type: string + NewId: + type: string + Ids: + oneOf: + - $ref: '#/components/schemas/OldId' + - type: array + items: + $ref: '#/components/schemas/NewId' From 7cf2517b10214bbcbdf02c33633160728e55538d Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 25 Jul 2024 21:20:11 -0400 Subject: [PATCH 13/13] Fix: circular references. Signed-off-by: dblock --- tools/src/helpers.ts | 9 ++++++--- tools/tests/merger/fixtures/extractor/expected_2.0.yaml | 4 +--- .../tester/fixtures/specs/complete/schemas/_common.yaml | 4 +--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 3a7bfacf6..97cee98f3 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -77,7 +77,7 @@ export function delete_matching_keys(obj: any, condition: (obj: any) => boolean) } } -export function find_refs (current: Record, root?: Record): Set { +export function find_refs (current: Record, root?: Record, call_stack: string[] = []): Set { var results = new Set() if (root === undefined) { @@ -89,12 +89,15 @@ export function find_refs (current: Record, root?: Record results.add(ref)) + if (ref_node !== undefined && !call_stack.includes(ref)) { + call_stack.push(ref) + find_refs(ref_node, root, call_stack).forEach((ref) => results.add(ref)) + } } if (_.isObject(current)) { _.forEach(current, (v) => { - find_refs(v as Record, root).forEach((ref) => results.add(ref)); + find_refs(v as Record, root, call_stack).forEach((ref) => results.add(ref)); }) } diff --git a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml index d9c539fcf..95e119868 100644 --- a/tools/tests/merger/fixtures/extractor/expected_2.0.yaml +++ b/tools/tests/merger/fixtures/extractor/expected_2.0.yaml @@ -167,8 +167,6 @@ components: - $ref: '#/components/schemas/_common:OldId' - type: array items: - $ref: '#/components/schemas/_common:NewId' - _common:NewId: - type: string + $ref: '#/components/schemas/_common:Ids' _common:OldId: type: string diff --git a/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml index 8b6aae7ac..c3a4e41f3 100644 --- a/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml +++ b/tools/tests/tester/fixtures/specs/complete/schemas/_common.yaml @@ -10,11 +10,9 @@ components: x-version-removed: '2.0' OldId: type: string - NewId: - type: string Ids: oneOf: - $ref: '#/components/schemas/OldId' - type: array items: - $ref: '#/components/schemas/NewId' + $ref: '#/components/schemas/Ids'