Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete schema elements that don't match target OpenSearch version. #428

Merged
merged 13 commits into from
Jul 26, 2024
5 changes: 4 additions & 1 deletion .github/workflows/test-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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
2 changes: 1 addition & 1 deletion eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
25 changes: 25 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,29 @@
"@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",
"eslint-plugin-license-header": "^0.6.1",
"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",
Expand Down
41 changes: 40 additions & 1 deletion tools/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ export function sort_by_keys (obj: Record<string, any>, 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
})
Expand All @@ -65,6 +64,46 @@ export function sort_array_by_keys (values: any[], priorities: string[] = []): s
})
}

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)) {
delete obj[key]
} else {
delete_matching_keys(item, condition)
}
}
}
}

export function find_refs (current: Record<string, any>, root?: Record<string, any>, call_stack: string[] = []): Set<string> {
var results = new Set<string>()

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 && !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<string, any>, root, call_stack).forEach((ref) => results.add(ref));
})
}

return results
}
dblock marked this conversation as resolved.
Show resolved Hide resolved

export function ensure_parent_dir (file_path: string): void {
fs.mkdirSync(path.dirname(file_path), { recursive: true })
}
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, new Logger(LogLevel.error))
this.spec = merger.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
66 changes: 37 additions & 29 deletions tools/src/merger/OpenApiMerger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@ import { Logger } from '../Logger'
// 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

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()) {
this.logger = logger
this.root_folder = fs.realpathSync(root_folder)
this.spec = {
this._spec = {
openapi: '3.1.0',
info: read_yaml(`${this.root_folder}/_info.yaml`, true),
paths: {},
Expand All @@ -40,17 +42,23 @@ export default class OpenApiMerger {
}
}

merge (output_path: string = ''): OpenAPIV3.Document {
this.#merge_schemas()
this.#merge_namespaces()
this.#sort_spec_keys()
this.#generate_global_params()
this.#generate_superseded_ops()

write_to(output_path: string): OpenApiMerger {
this.logger.info(`Writing ${output_path} ...`)
write_yaml(output_path, this.spec())
return this
}

spec(): OpenAPIV3.Document {
if (!this._merged) {
this.#merge_schemas()
this.#merge_namespaces()
this.#sort_spec_keys()
this.#generate_global_params()
this.#generate_superseded_ops()
this._merged = true
}

if (output_path !== '') write_yaml(output_path, this.spec)
return this.spec as OpenAPIV3.Document
return this._spec as OpenAPIV3.Document
}

// Merge files from <spec_root>/namespaces folder.
Expand All @@ -59,21 +67,21 @@ 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 }
})
}

// 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 +100,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 +123,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)
}
}
Loading
Loading