diff --git a/.github/vale/styles/OpenSearch/NumericFormat.yml b/.github/vale/styles/OpenSearch/NumericFormat.yml deleted file mode 100644 index 37a7b58e1..000000000 --- a/.github/vale/styles/OpenSearch/NumericFormat.yml +++ /dev/null @@ -1,8 +0,0 @@ -extends: existence -message: "Missing 'format' after 'type: number'." -level: error -scope: raw -nonword: true -ignorecase: false -tokens: - - 'type:\s+number(?!\n\s+format:)' diff --git a/.vale.ini b/.vale.ini index b14b6f078..9141bb025 100644 --- a/.vale.ini +++ b/.vale.ini @@ -25,7 +25,6 @@ OpenSearch.LoginNoun = YES OpenSearch.LoginVerb = YES OpenSearch.LogoutNoun = YES OpenSearch.LogoutVerb = YES -OpenSearch.NumericFormat = YES OpenSearch.OxfordComma = YES OpenSearch.PassiveVoice = NO OpenSearch.Please = YES diff --git a/CHANGELOG.md b/CHANGELOG.md index 19a7068c8..bc5ca3bf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `GET`, `POST /_plugins/_ml/tasks/_search`, `GET /_plugins/_ml/tools`, `tools/{tool_name}` ([#797](https://github.com/opensearch-project/opensearch-api-specification/pull/797)) - Added `POST /_plugins/_ml/agents/{agent_id}/_execute`, `GET /_plugins/_ml/agents/{agent_id}`, `GET`, `POST /_plugins/_ml/agents/_search` ([#798](https://github.com/opensearch-project/opensearch-api-specification/pull/798)) - Added a warning for test file names that don't match the API being tested ([#793](https://github.com/opensearch-project/opensearch-api-specification/pull/793)) -- Added a vale rule for numeric types ([#800](https://github.com/opensearch-project/opensearch-api-specification/pull/800)) +- Added a linter rule for numeric types ([#800](https://github.com/opensearch-project/opensearch-api-specification/pull/800)) - Added `time` field to the `GetStats` schema in `_common.yml` ([#803](https://github.com/opensearch-project/opensearch-api-specification/pull/803)) - Added version for `POST /_plugins/_ml/_train/{algorithm_name}`, `_predict/{algorithm_name}/{model_id}`, and `_train_predict/{algorithm_name}` ([#763](https://github.com/opensearch-project/opensearch-api-specification/pull/763)) - Added `POST _plugins/_security/api/internalusers/{username}` response `201` ([#810](https://github.com/opensearch-project/opensearch-api-specification/pull/810)) diff --git a/spec/namespaces/_core.yaml b/spec/namespaces/_core.yaml index 1b463ab0a..7e8bcc884 100644 --- a/spec/namespaces/_core.yaml +++ b/spec/namespaces/_core.yaml @@ -2273,7 +2273,6 @@ components: - $ref: '../schemas/_core.bulk.yaml#/components/schemas/OperationContainer' - $ref: '../schemas/_core.bulk.yaml#/components/schemas/UpdateAction' - type: object - - type: number description: The operation definition and data (action-data pairs), separated by newlines required: true bulk_stream: diff --git a/test.yml b/test.yml deleted file mode 100644 index 9456ac79b..000000000 --- a/test.yml +++ /dev/null @@ -1,15 +0,0 @@ -components: - requestBodies: - bulk: - content: - application/x-ndjson: - schema: - type: array - items: - anyOf: - - $ref: '../schemas/_core.bulk.yaml#/components/schemas/OperationContainer' - - $ref: '../schemas/_core.bulk.yaml#/components/schemas/UpdateAction' - - type: object - - type: number - description: The operation definition and data (action-data pairs), separated by newlines - required: true \ No newline at end of file diff --git a/tools/src/linter/SchemaVisitingValidator.ts b/tools/src/linter/SchemaVisitingValidator.ts index bf3beff82..eed8f2ef1 100644 --- a/tools/src/linter/SchemaVisitingValidator.ts +++ b/tools/src/linter/SchemaVisitingValidator.ts @@ -20,6 +20,7 @@ import { SpecificationContext } from '../_utils' import { type OpenAPIV3 } from 'openapi-types' +import { namespace_file } from '../../tests/linter/factories/namespace_file' export default class SchemaVisitingValidator { private readonly _namespaces_folder: NamespacesFolder @@ -52,6 +53,33 @@ export default class SchemaVisitingValidator { return errors } + validateFile(filePath: string): ValidationError[] { + const errors: ValidationError[] = [] + const visitor = this.createVisitor(errors) + + const targetFile = [...this._namespaces_folder.files, ...this._schemas_folder.files].find(f => f.file === filePath) + + if (!targetFile) { + return [{ message: `File not found in namespaces/schemas: ${filePath}`, file: filePath }] + } + + visitor.visit_specification(new SpecificationContext(targetFile.file), targetFile.spec()) + + return errors + } + + private createVisitor(errors: ValidationError[]): SchemaVisitor { + const validating_functions = [ + this.#validate_numeric_schema.bind(this) + ] + + return new SchemaVisitor((ctx, schema) => { + for (const f of validating_functions) { + f(ctx, schema, errors) + } + }) + } + #validate_inline_object_schema (ctx: SpecificationContext, schema: MaybeRef, errors: ValidationError[]): void { if (is_ref(schema) || schema.type !== 'object' || schema.properties === undefined) { return @@ -73,26 +101,16 @@ export default class SchemaVisitingValidator { } if (schema.type === 'number') { - if (schema.format === undefined || SCHEMA_NUMBER_FORMATS.includes(schema.format)) { - return - } - - if (SCHEMA_INTEGER_FORMATS.includes(schema.format)) { - errors.push(ctx.error(`schema of type 'number' with format '${schema.format}' should instead be of type 'integer'`)) - } else { - errors.push(ctx.error(`schema of type 'number' with format '${schema.format}' is invalid, expected one of: ${SCHEMA_NUMBER_FORMATS.join(', ')}`)) + if (schema.format == null || !schema.format) { + errors.push(ctx.error(`Schema of type 'number' must specify a valid format. Allowed formats: ${SCHEMA_NUMBER_FORMATS.join(', ')}`)); + return; } } if (schema.type === 'integer') { - if (schema.format === undefined || SCHEMA_INTEGER_FORMATS.includes(schema.format)) { - return - } - - if (SCHEMA_NUMBER_FORMATS.includes(schema.format)) { - errors.push(ctx.error(`schema of type 'integer' with format '${schema.format}' should instead be of type 'number'`)) - } else { - errors.push(ctx.error(`schema of type 'integer' with format '${schema.format}' is invalid, expected one of: ${SCHEMA_INTEGER_FORMATS.join(', ')}`)) + if (schema.format == null || !schema.format) { + errors.push(ctx.error(`Schema of type 'integer' must specify a valid format. Allowed formats: ${SCHEMA_INTEGER_FORMATS.join(', ')}`)); + return; } } } diff --git a/tools/src/linter/SpecValidator.ts b/tools/src/linter/SpecValidator.ts index 462fbe93b..0dc828fdb 100644 --- a/tools/src/linter/SpecValidator.ts +++ b/tools/src/linter/SpecValidator.ts @@ -16,6 +16,8 @@ import InfoFile from './components/InfoFile' import SchemaVisitingValidator from './SchemaVisitingValidator' import SchemasValidator from './SchemasValidator' import { type Logger } from '../Logger' +import { execSync } from 'child_process' +import { relative, resolve } from 'path' export default class SpecValidator { logger: Logger @@ -26,8 +28,9 @@ export default class SpecValidator { schemas_validator: SchemasValidator schema_refs_validator: SchemaRefsValidator inline_object_schema_validator: SchemaVisitingValidator + changedOnly: boolean - constructor (root_folder: string, logger: Logger) { + constructor (root_folder: string, logger: Logger, changedOnly: boolean) { this.logger = logger this.superseded_ops_file = new SupersededOperationsFile(`${root_folder}/_superseded_operations.yaml`) this.info_file = new InfoFile(`${root_folder}/_info.yaml`) @@ -36,9 +39,48 @@ export default class SpecValidator { this.schemas_validator = new SchemasValidator(root_folder, logger) this.schema_refs_validator = new SchemaRefsValidator(this.namespaces_folder, this.schemas_folder) this.inline_object_schema_validator = new SchemaVisitingValidator(this.namespaces_folder, this.schemas_folder) + this.changedOnly = changedOnly + } + + private getChangedFiles(): string[] { + try { + const output = execSync('git diff --name-only origin/main', { encoding: 'utf-8' }) + return output.split('\n').filter(file => file.endsWith('.yml') || file.endsWith('.yaml')) + } catch (error) { + this.logger.error('Error:' + error) + return [] + } + } + + validateChangedFiles(): ValidationError[] { + const changedFiles = this.getChangedFiles() + + if (changedFiles.length === 0) { + this.logger.log('No valid files to check') + return [] + } + + this.logger.log(`Checking diff files:\n${changedFiles.join('\n')}`) + let errors: ValidationError[] = [] + + for (const file of changedFiles) { + let normalizedFile = file + + if (file.startsWith('spec/')) { + normalizedFile = relative(resolve('spec'), resolve(file)) + } else if (file.startsWith('schemas/')) { + normalizedFile = relative(resolve('schemas'), resolve(file)) + } + + errors = errors.concat(this.inline_object_schema_validator.validateFile(normalizedFile)) + } + + return errors } validate (): ValidationError[] { + if (this.changedOnly) return this.validateChangedFiles() + const component_errors = [ ...this.namespaces_folder.validate(), ...this.schemas_folder.validate() diff --git a/tools/src/linter/lint.ts b/tools/src/linter/lint.ts index af71a5084..9f8a7a242 100644 --- a/tools/src/linter/lint.ts +++ b/tools/src/linter/lint.ts @@ -16,13 +16,14 @@ const command = new Command() .description('Validate the OpenSearch multi-file spec.') .addOption(new Option('-s, --source ', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec'))) .addOption(new Option('--verbose', 'whether to print the full stack trace of errors').default(false)) + .addOption(new Option('--changed-only', 'whether to only validate changed files').default(false)) .allowExcessArguments(false) .parse() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) logger.log(`Validating ${opts.source} ...`) -const validator = new SpecValidator(opts.source, logger) +const validator = new SpecValidator(opts.source, logger, opts.changedOnly) const errors = validator.validate() if (errors.length === 0) { diff --git a/tools/src/prepare-for-vale/KeepDescriptions.ts b/tools/src/prepare-for-vale/KeepDescriptions.ts index bdca1bed3..8d159cc27 100644 --- a/tools/src/prepare-for-vale/KeepDescriptions.ts +++ b/tools/src/prepare-for-vale/KeepDescriptions.ts @@ -12,7 +12,7 @@ import fg from 'fast-glob' import { Logger } from '../Logger' /** - * Keeps only description: field values and type: number with its next line. + * Keeps only description: field values. */ export default class KeepDescriptions { root_folder: string @@ -34,37 +34,28 @@ export default class KeepDescriptions { process_file(filename: string): void { const contents = fs.readFileSync(filename, 'utf-8') - const lines = contents.split(/\r?\n/) - const writer = fs.openSync(filename, 'w+') - - let inside_text = false - for (let i = 0; i < lines.length; i++) { - let line = lines[i] + var writer = fs.openSync(filename, 'w+') + var inside_text = false + contents.split(/\r?\n/).forEach((line) => { if (line.match(/^[\s]+((description|x-deprecation-message): \|)/)) { inside_text = true } else if (line.match(/^[\s]+((description|x-deprecation-message):)[\s]+/)) { let cleaned_line = this.prune(line, /(description|x-deprecation-message):/, ' ') cleaned_line = this.prune_vars(cleaned_line) cleaned_line = this.remove_links(cleaned_line) - fs.writeSync(writer, cleaned_line + "\n") + fs.writeSync(writer, cleaned_line) } else if (inside_text && line.match(/^[\s]*[\w\\$]*:/)) { inside_text = false } else if (inside_text) { let cleaned_line = this.remove_links(line) cleaned_line = this.prune_vars(cleaned_line) - fs.writeSync(writer, cleaned_line + "\n") - } else if (line.includes("type: number")) { - fs.writeSync(writer, line + "\n") - if (i + 1 < lines.length) { - fs.writeSync(writer, lines[i + 1] + "\n") - } + fs.writeSync(writer, cleaned_line) } - if (line.length > 0) { fs.writeSync(writer, "\n") } - } + }) } prune_vars(line: string): string { @@ -83,4 +74,4 @@ export default class KeepDescriptions { return ' ' + p1 + spaces }) } -} +} \ No newline at end of file