Skip to content

Commit

Permalink
Delete schema elements that don't match target OpenSearch version.
Browse files Browse the repository at this point in the history
Signed-off-by: dblock <[email protected]>
  • Loading branch information
dblock committed Jul 18, 2024
1 parent e1cd369 commit 262e1e0
Show file tree
Hide file tree
Showing 15 changed files with 534 additions and 63 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,4 @@ jobs:
run: docker-compose up -d

- name: Run Tests
run: npm run test:spec -- --opensearch-insecure
run: npm run test:spec -- --opensearch-insecure --opensearch-version=${{ matrix.entry.version }}
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `is_hidden` to `/{index}/_alias/{name}` and `/{index}/_aliases/{name}` ([#429](https://github.com/opensearch-project/opensearch-api-specification/pull/429))
- Added `ignore_unmapped` to `GeoDistanceQuery` ([#427](https://github.com/opensearch-project/opensearch-api-specification/pull/427))
- Added missing variants of `indices.put_alias` ([#434](https://github.com/opensearch-project/opensearch-api-specification/pull/434))
- Added `--opensearch-version` to `merger` that excludes schema elements per semver ([#428](https://github.com/opensearch-project/opensearch-api-specification/pull/428))

### Changed

Expand Down
7 changes: 7 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ The merger tool merges the multi-file OpenSearch spec into a single file for pro

- `--source <path>`: The path to the root folder of the multi-file spec, defaults to `<repository-root>/spec`.
- `--output <path>`: The path to write the final merged spec to, defaults to `<repository-root>/build/opensearch-openapi.yaml`.
- `--opensearch-version`: An optional target version of OpenSearch, checking values of `x-version-added` and `x-version-removed`.

#### Example

Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion tools/src/linter/SchemasValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>
const merger = new OpenApiMerger(this.root_folder, undefined, new Logger(LogLevel.error))
this.spec = merger.merge().spec().components as Record<string, any>
const named_schemas_errors = this.validate_named_schemas()
if (named_schemas_errors.length > 0) return named_schemas_errors
return [
Expand Down
131 changes: 104 additions & 27 deletions tools/src/merger/OpenApiMerger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>
logger: Logger
target_version?: string

protected _spec: Record<string, any>
protected _merged: boolean = false

paths: Record<string, Record<string, OpenAPIV3.PathItemObject>> = {} // namespace -> path -> path_item_object
schemas: Record<string, Record<string, OpenAPIV3.SchemaObject>> = {} // 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: {},
Expand All @@ -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 <spec_root>/namespaces folder.
Expand All @@ -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]) }
}
}

Expand All @@ -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
})
})
}
Expand All @@ -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)
}
}
7 changes: 4 additions & 3 deletions tools/src/merger/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ const command = new Command()
.addOption(new Option('-s, --source <path>', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec')))
.addOption(new Option('-o, --output <path>', 'output file name').default(resolve(__dirname, '../../../build/opensearch-openapi.yaml')))
.addOption(new Option('--verbose', 'show merge details').default(false))
.addOption(new Option('--opensearch-version <number>', '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.')
8 changes: 6 additions & 2 deletions tools/src/tester/MergedOpenApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@ import OpenApiMerger from '../merger/OpenApiMerger';
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
Expand Down
3 changes: 2 additions & 1 deletion tools/src/tester/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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 <number>', 'target OpenSearch schema version').default(undefined))
.addOption(OPENSEARCH_URL_OPTION)
.addOption(OPENSEARCH_USERNAME_OPTION)
.addOption(OPENSEARCH_PASSWORD_OPTION)
Expand All @@ -50,7 +51,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)
Expand Down
59 changes: 51 additions & 8 deletions tools/tests/merger/OpenApiMerger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 262e1e0

Please sign in to comment.