Refactored AJV usages with an AJV wrapper that provides better error JSON validation messages.

CHANGELOG.md

@@ -32,6 +32,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Added support for `application/yaml` responses ([#363](https://github.com/opensearch-project/opensearch-api-specification/pull/363))
 - Added test for search with seq_no_primary_term ([#367](https://github.com/opensearch-project/opensearch-api-specification/pull/367))
 - Added a linter for parameter sorting ([#369](https://github.com/opensearch-project/opensearch-api-specification/pull/369))
+- Added AjvErrorsParser to print more informative error messages ([#364](https://github.com/opensearch-project/opensearch-api-specification/issues/364))
+- Added JsonSchemaValidator, a wrapper for AJV ([#364](https://github.com/opensearch-project/opensearch-api-specification/issues/364))
 - Added support for `application/cbor` responses ([#371](https://github.com/opensearch-project/opensearch-api-specification/pull/371))
 - Added support for `application/smile` responses ([#386](https://github.com/opensearch-project/opensearch-api-specification/pull/386))
 - Added `doc_status`, `remote_store`, `segment_replication` and `unreferenced_file_cleanups_performed` to `SegmentStats` ([#395](https://github.com/opensearch-project/opensearch-api-specification/pull/395))

json_schemas/_info.schema.yaml

@@ -40,5 +40,4 @@ properties:
     type: string
 required:
   - title
-  - version
-  - $schema
+  - version

json_schemas/_superseded_operations.schema.yaml

@@ -1,9 +1,10 @@
 $schema: http://json-schema.org/draft-07/schema#
 
 type: object
-patternProperties:
-  ^\$schema$:
+properties:
+  $schema:
     type: string
+patternProperties:
   ^/:
     type: object
     properties:
@@ -17,5 +18,4 @@ patternProperties:
           enum: [GET, POST, PUT, DELETE, HEAD, OPTIONS, PATCH]
     required: [superseded_by, operations]
     additionalProperties: false
-required: [$schema]
 additionalProperties: false

json_schemas/test_story.schema.yaml

@@ -18,7 +18,7 @@ properties:
     type: array
     items:
       $ref: '#/definitions/Chapter'
-required: [$schema, description, chapters]
+required: [description, chapters]
 additionalProperties: false
 
 definitions:

tools/src/_utils/AjvErrorsParser.ts

@@ -0,0 +1,103 @@
+/*
+* 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 { Ajv2019 as AJV, ErrorsTextOptions, ErrorObject } from 'ajv/dist/2019'
+import _ from 'lodash'
+
+interface GroupedErrors {
+  required: ErrorObject[]
+  prohibited: ErrorObject[]
+  enum: ErrorObject[]
+  others: ErrorObject[]
+}
+
+export default class AjvErrorsParser {
+  private readonly ajv: AJV
+  private readonly options: ErrorsTextOptions
+
+  constructor(ajv: AJV, options: ErrorsTextOptions = {}) {
+    this.ajv = ajv
+    this.options = { separator: ' --- ', ...options }
+  }
+
+  parse(errors: ErrorObject[] | undefined | null): string {
+    const error_groups = this.#group_errors(errors ?? [])
+    const parsed_errors = [
+      this.#prohibited_property_error(error_groups.prohibited),
+      this.#required_property_error(error_groups.required),
+      this.#enum_error(error_groups.enum),
+      ...error_groups.others
+    ].filter(e => e != null) as ErrorObject[]
+    return this.ajv.errorsText(parsed_errors, this.options)
+  }
+
+  #group_errors(errors: ErrorObject[]): GroupedErrors {
+    const categories = {
+      required: [] as ErrorObject[],
+      prohibited: [] as ErrorObject[],
+      enum: [] as ErrorObject[],
+      others: [] as ErrorObject[]
+    }
+    _.values(_.groupBy(errors, 'instancePath')).forEach((path_errors) => {
+      for (const error of path_errors) {
+        switch (error.keyword) {
+          case 'required':
+            categories.required.push(error)
+            break
+          case 'unevaluatedProperties':
+          case 'additionalProperties':
+            categories.prohibited.push(error)
+            break
+          case 'enum':
+            categories.enum.push(error)
+            break
+          default:
+            categories.others.push(error)
+        }
+      }
+    })
+    return categories
+  }
+
+  #prohibited_property_error(errors: ErrorObject[]): ErrorObject | undefined {
+    if (errors.length === 0) return
+    const properties = errors.map((error) => error.params.additionalProperty ?? error.params.unevaluatedProperty).join(', ')
+    return {
+      keyword: 'prohibited',
+      instancePath: errors[0].instancePath,
+      schemaPath: errors[0].schemaPath,
+      params: {},
+      message: `contains unsupported properties: ${properties}`
+    }
+  }
+
+  #required_property_error(errors: ErrorObject[]): ErrorObject | undefined {
+    if (errors.length === 0) return
+    const properties = errors.map((error) => error.params.missingProperty).join(', ')
+    return {
+      keyword: 'required',
+      instancePath: errors[0].instancePath,
+      schemaPath: errors[0].schemaPath,
+      params: {},
+      message: `MUST contain the missing properties: ${properties}`
+    }
+  }
+
+  #enum_error(errors: ErrorObject[]): ErrorObject | undefined {
+    if (errors.length === 0) return
+    const allowed_values = errors[0].params.allowedValues.join(', ')
+    return {
+      keyword: 'enum',
+      instancePath: errors[0].instancePath,
+      schemaPath: errors[0].schemaPath,
+      params: {},
+      message: `MUST be equal to one of the allowed values: ${allowed_values}`
+    }
+  }
+}

tools/src/_utils/JsonSchemaValidator.ts

@@ -0,0 +1,61 @@
+/*
+* 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 { Ajv2019 as AJV, ErrorsTextOptions, Options as AjvOptions, ValidateFunction } from 'ajv/dist/2019'
+import ajv_errors, { ErrorMessageOptions } from 'ajv-errors'
+import addFormats from 'ajv-formats';
+import AjvErrorsParser from './AjvErrorsParser';
+
+interface JsonSchemaValidatorOpts {
+  ajv_opts?: AjvOptions,
+  ajv_errors_opts?: ErrorMessageOptions,
+  errors_text_opts?: ErrorsTextOptions
+  additional_keywords?: string[],
+  reference_schemas?: Record<string, Record<any, any>>
+}
+
+const DEFAULT_AJV_OPTS = {
+  strict: true,
+  allErrors: true
+}
+
+// Wrapper for AJV
+export default class JsonSchemaValidator {
+  private readonly ajv: AJV
+  private readonly errors_parser: AjvErrorsParser
+  private readonly _validate: ValidateFunction | undefined
+  message: string | undefined
+
+  constructor(default_schema?: Record<any, any>, options: JsonSchemaValidatorOpts = {}) {
+    this.ajv = new AJV({ ...DEFAULT_AJV_OPTS, ...options.ajv_opts })
+    addFormats(this.ajv);
+    if (options.ajv_errors_opts != null) ajv_errors(this.ajv, options.ajv_errors_opts)
+    for (const keyword of options.additional_keywords ?? []) this.ajv.addKeyword(keyword)
+    Object.entries(options.reference_schemas ?? {}).forEach(([key, schema]) => this.ajv.addSchema(schema, key))
+    this.errors_parser = new AjvErrorsParser(this.ajv, options.errors_text_opts)
+    if (default_schema) this._validate = this.ajv.compile(default_schema)
+  }
+
+  validate_data(data: any, schema?: Record<any, any>): string | undefined {
+    if (schema) return this.#validate(this.ajv.compile(schema), data)
+    if (this._validate) return this.#validate(this._validate, data)
+    throw new Error('No schema provided')
+  }
+
+  validate_schema(schema: any): string | undefined {
+    const validate_func = this.ajv.validateSchema.bind(this.ajv) as ValidateFunction
+    return this.#validate(validate_func, schema ?? {}, true)
+  }
+
+  #validate(validate_func: ValidateFunction, data: any, is_schema: boolean = false): string | undefined {
+    const valid = validate_func(data) as boolean
+    const errors = is_schema ? this.ajv.errors : validate_func.errors
+    return valid ? undefined : this.errors_parser.parse(errors)
+  }
+}

tools/src/linter/SchemasValidator.ts

@@ -7,16 +7,10 @@
 * compatible open source license.
 */
 
-import AJV from 'ajv'
-import addFormats from 'ajv-formats'
 import OpenApiMerger from '../merger/OpenApiMerger'
 import { type ValidationError } from '../types'
-import { type Logger } from '../Logger'
-
-const IGNORED_ERROR_PREFIXES = [
-  'can\'t resolve reference', // errors in referenced schemas will also cause reference errors
-  'discriminator: oneOf subschemas' // known bug in ajv: https://github.com/ajv-validator/ajv/issues/2281
-]
+import { Logger, LogLevel } from '../Logger'
+import JsonSchemaValidator from "../_utils/JsonSchemaValidator";
 
 const ADDITIONAL_KEYWORDS = [
   'x-version-added',
@@ -29,18 +23,16 @@ export default class SchemasValidator {
   logger: Logger
   root_folder: string
   spec: Record<string, any> = {}
-  ajv: AJV
+  json_validator: JsonSchemaValidator
 
   constructor (root_folder: string, logger: Logger) {
     this.logger = logger
     this.root_folder = root_folder
-    this.ajv = new AJV({ strict: true, discriminator: true })
-    addFormats(this.ajv)
-    for (const keyword of ADDITIONAL_KEYWORDS) this.ajv.addKeyword(keyword)
+    this.json_validator = new JsonSchemaValidator(undefined, { additional_keywords: ADDITIONAL_KEYWORDS })
   }
 
   validate (): ValidationError[] {
-    this.spec = new OpenApiMerger(this.root_folder, this.logger).merge().components as Record<string, any>
+    this.spec = new OpenApiMerger(this.root_folder, new Logger(LogLevel.error)).merge().components as Record<string, any>
     const named_schemas_errors = this.validate_named_schemas()
     if (named_schemas_errors.length > 0) return named_schemas_errors
     return [
@@ -52,25 +44,24 @@ export default class SchemasValidator {
 
   validate_named_schemas (): ValidationError[] {
     return Object.entries(this.spec.schemas as Record<string, any>).map(([key, _schema]) => {
-      const schema = _schema as Record<string, any>
-      const error = this.validate_schema(schema, `#/components/schemas/${key}`)
-      if (error == null) return
+      const message = this.json_validator.validate_schema(_schema)
+      if (message == null) return
 
       const file = `schemas/${key.split(':')[0]}.yaml`
       const location = `#/components/schemas/${key.split(':')[1]}`
-      return this.error(file, location, error)
+      return this.error(file, location, message)
     }).filter((error) => error != null) as ValidationError[]
   }
 
   validate_parameter_schemas (): ValidationError[] {
     return Object.entries(this.spec.parameters as Record<string, any>).map(([key, param]) => {
-      const error = this.validate_schema(param.schema as Record<string, any>)
-      if (error == null) return
+      const message = this.json_validator.validate_schema(param.schema)
+      if (message == null) return
 
       const namespace = this.group_to_namespace(key.split('::')[0])
       const file = namespace === '_global' ? '_global_parameters.yaml' : `namespaces/${namespace}.yaml`
       const location = namespace === '_global' ? param.name as string : `#/components/parameters/${key}`
-      return this.error(file, location, error)
+      return this.error(file, location, message)
     }).filter((error) => error != null) as ValidationError[]
   }
 
@@ -94,31 +85,18 @@ export default class SchemasValidator {
 
   validate_content_schemas (file: string, location: string, content: Record<string, any> | undefined): ValidationError[] {
     return Object.entries(content ?? {}).map(([media_type, value]) => {
-      const schema = value.schema as Record<string, any>
-      const error = this.validate_schema(schema)
-      if (error != null) return this.error(file, `${location}/content/${media_type}`, error)
+      const message = this.json_validator.validate_schema(value.schema)
+      if (message != null) return this.error(file, `${location}/content/${media_type}`, message)
     }).filter(e => e != null) as ValidationError[]
   }
 
-  validate_schema (schema: Record<string, any>, key: string | undefined = undefined): Error | undefined {
-    if (schema == null || schema.$ref != null) return
-    try {
-      if (key != null) this.ajv.addSchema(schema, key)
-      this.ajv.compile(schema)
-    } catch (_e: any) {
-      const error = _e as Error
-      for (const prefix of IGNORED_ERROR_PREFIXES) if (error.message.startsWith(prefix)) return
-      return error
-    }
-  }
-
   group_to_namespace (group: string): string {
     if (group === '_global') return '_global'
     const [, namespace] = group.split('.').reverse()
     return namespace ?? '_core'
   }
 
-  error (file: string, location: string, error: Error): ValidationError {
-    return { file, location, message: error.message }
+  error (file: string, location: string, message: string): ValidationError {
+    return { file, location, message }
   }
 }

tools/src/linter/components/base/FileValidator.ts

@@ -10,9 +10,8 @@
 import ValidatorBase from './ValidatorBase'
 import { type ValidationError } from 'types'
 import { type OpenAPIV3 } from 'openapi-types'
-import { read_yaml, to_json } from '../../../helpers'
-import AJV from 'ajv'
-import addFormats from 'ajv-formats'
+import { read_yaml } from '../../../helpers'
+import JsonSchemaValidator from "../../../_utils/JsonSchemaValidator";
 
 export default class FileValidator extends ValidatorBase {
   file_path: string
@@ -61,11 +60,10 @@ export default class FileValidator extends ValidatorBase {
     const json_schema_path: string = (this.spec() as any).$schema ?? ''
     if (json_schema_path === '') return this.error('JSON Schema is required but not found in this file.', '$schema')
     const schema = read_yaml(json_schema_path)
-    const ajv = new AJV({ schemaId: 'id' })
-    addFormats(ajv)
-    const validator = ajv.compile(schema)
-    if (!validator(this.spec())) {
-      return this.error(`File content does not match JSON schema found in '${json_schema_path}':\n ${to_json(validator.errors)}`)
-    }
+    delete schema.$schema
+    const validator = new JsonSchemaValidator()
+    const message = validator.validate_data(this.spec(), schema)
+    if (message != null)
+      return this.error(`File content does not match JSON schema found in '${json_schema_path}': ${message}`)
   }
 }