Skip to content

Commit

Permalink
AjvErrorsParser
Browse files Browse the repository at this point in the history
- Collapse multiple `required` errors into one
- Collapse multiple `additionalProperties` and `unevaluatedProperties` errors into one, and print out which properties are prohibited.
- Make `enum` errors print out the supported enum values.

Signed-off-by: Theo Truong <[email protected]>
  • Loading branch information
nhtruong committed Jul 5, 2024
1 parent a359a5a commit e31da7f
Show file tree
Hide file tree
Showing 12 changed files with 205 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ 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 support for `application/cbor` responses ([#371](https://github.com/opensearch-project/opensearch-api-specification/pull/371))

### Changed
Expand Down
98 changes: 98 additions & 0 deletions tools/src/_utils/AjvErrorsParser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* 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, ErrorsTextOptions, ErrorObject } from 'ajv/dist/2019'
import _ from 'lodash'

export default class AjvErrorsParser {
private readonly ajv: Ajv2019
private readonly options: ErrorsTextOptions

constructor(ajv: Ajv2019, 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[]): { required: ErrorObject[], prohibited: ErrorObject[], enum: ErrorObject[], others: ErrorObject[] } {
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: { additionalProperty: errors[0].instancePath.split('/').pop() },
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: { missingProperty: errors[0].instancePath.split('/').pop() },
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: errors[0].params,
message: `MUST be equal to one of the allowed values: ${allowed_values}`
}
}


}
32 changes: 12 additions & 20 deletions tools/src/linter/SchemasValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,12 @@
* compatible open source license.
*/

import AJV from 'ajv'
import { Ajv2019 as AJV } from 'ajv/dist/2019'
import addFormats from 'ajv-formats'
import AjvErrorsParser from "../_utils/AjvErrorsParser";
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'

const ADDITIONAL_KEYWORDS = [
'x-version-added',
Expand All @@ -30,17 +26,19 @@ export default class SchemasValidator {
root_folder: string
spec: Record<string, any> = {}
ajv: AJV
errors_parser: AjvErrorsParser

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.errors_parser = new AjvErrorsParser(this.ajv, {})
}

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 [
Expand All @@ -53,7 +51,7 @@ 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}`)
const error = this.validate_schema(schema)
if (error == null) return

const file = `schemas/${key.split(':')[0]}.yaml`
Expand Down Expand Up @@ -100,16 +98,10 @@ export default class SchemasValidator {
}).filter(e => e != null) as ValidationError[]
}

validate_schema (schema: Record<string, any>, key: string | undefined = undefined): Error | undefined {
validate_schema (schema: Record<string, any>): string | 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
}
const valid = this.ajv.validateSchema(schema)
if (valid !== true) return this.errors_parser.parse(this.ajv.errors)
}

group_to_namespace (group: string): string {
Expand All @@ -118,7 +110,7 @@ export default class SchemasValidator {
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 }
}
}
7 changes: 5 additions & 2 deletions tools/src/linter/components/base/FileValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
import ValidatorBase from './ValidatorBase'
import { type ValidationError } from 'types'
import { type OpenAPIV3 } from 'openapi-types'
import { read_yaml, to_json } from '../../../helpers'
import { read_yaml } from '../../../helpers'
import AJV from 'ajv'
import addFormats from 'ajv-formats'
import AjvErrorsParser from '../../../_utils/AjvErrorsParser'

export default class FileValidator extends ValidatorBase {
file_path: string
Expand Down Expand Up @@ -63,9 +64,11 @@ export default class FileValidator extends ValidatorBase {
const schema = read_yaml(json_schema_path)
const ajv = new AJV({ schemaId: 'id' })
addFormats(ajv)
const errors_parser = new AjvErrorsParser(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)}`)
const message = errors_parser.parse(validator.errors)
return this.error(`File content does not match JSON schema found in '${json_schema_path}': ${message}`)
}
}
}
7 changes: 5 additions & 2 deletions tools/src/tester/SchemaValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
* compatible open source license.
*/

import AJV from 'ajv'
import AJV from 'ajv/dist/2019'
import ajv_errors from 'ajv-errors'
import addFormats from 'ajv-formats'
import AjvErrorsParser from '../_utils/AjvErrorsParser'
import { type OpenAPIV3 } from 'openapi-types'
import { type Evaluation, Result } from './types/eval.types'
import { Logger } from 'Logger'
Expand All @@ -25,6 +26,7 @@ const ADDITIONAL_KEYWORDS = [

export default class SchemaValidator {
private readonly ajv: AJV
private readonly errors_parser: AjvErrorsParser
private readonly logger: Logger

constructor (spec: OpenAPIV3.Document, logger: Logger) {
Expand All @@ -35,6 +37,7 @@ export default class SchemaValidator {
ajv_errors(this.ajv, { singleError: true })
const schemas = spec.components?.schemas ?? {}
for (const key in schemas) this.ajv.addSchema(schemas[key], `#/components/schemas/${key}`)
this.errors_parser = new AjvErrorsParser(this.ajv)
}

validate (schema: OpenAPIV3.SchemaObject, data: any): Evaluation {
Expand All @@ -47,7 +50,7 @@ export default class SchemaValidator {
}
return {
result: valid ? Result.PASSED : Result.FAILED,
message: valid ? undefined : this.ajv.errorsText(validate.errors)
message: valid ? undefined : this.errors_parser.parse(validate.errors)
}
}
}
11 changes: 7 additions & 4 deletions tools/src/tester/StoryValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,23 @@

import { Result, StoryEvaluation, StoryFile } from "./types/eval.types";
import * as path from "path";
import { Ajv2019, ValidateFunction } from 'ajv/dist/2019'
import { Ajv2019 as AJV, ValidateFunction } from 'ajv/dist/2019'
import addFormats from 'ajv-formats'
import { read_yaml } from "../helpers";
import AjvErrorsParser from "../_utils/AjvErrorsParser";

export default class StoryValidator {
private static readonly SCHEMA_FILE = path.resolve("./json_schemas/test_story.schema.yaml")
private readonly ajv: Ajv2019
private readonly ajv: AJV
private readonly errors_parser: AjvErrorsParser
private readonly validate_schema: ValidateFunction

constructor() {
this.ajv = new Ajv2019({ allErrors: true, strict: false })
this.ajv = new AJV({ allErrors: true, strict: false })
addFormats(this.ajv)
const schema = read_yaml(StoryValidator.SCHEMA_FILE)
this.validate_schema = this.ajv.compile(schema)
this.errors_parser = new AjvErrorsParser(this.ajv)
}

validate(story_file: StoryFile): StoryEvaluation | undefined {
Expand All @@ -40,7 +43,7 @@ export default class StoryValidator {

#validate_schema(story_file: StoryFile): StoryEvaluation | undefined {
const valid = this.validate_schema(story_file.story)
if (!valid) return this.#invalid(story_file, this.ajv.errorsText(this.validate_schema.errors))
if (!valid) return this.#invalid(story_file, this.errors_parser.parse(this.validate_schema.errors))
}

#invalid({ story, display_path, full_path }: StoryFile, message: string): StoryEvaluation {
Expand Down
62 changes: 62 additions & 0 deletions tools/tests/_utils/AjvErrorsParser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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 } from "ajv/dist/2019";
import AjvErrorsParser from "../../src/_utils/AjvErrorsParser";

describe('AjvErrorsParser', () => {
const ajv = new Ajv2019({ allErrors: true, strict: false })
const parser = new AjvErrorsParser(ajv, { separator: ' | ' , dataVar: 'Obj' })
const schema = {
type: 'object',
additionalProperties: false,
required: [ 'a_boolean', 'a_number', 'an_array', 'an_enum', 'a_compound_object'],
properties: {
a_boolean: { type: 'boolean' },
a_number: { type: 'number' },
a_nullable_string: { type: ['string', 'null'] },
a_non_nullable_string: { type: 'string' },
an_array: { type: 'array', items: { type: 'string' }, minItems: 1 },
an_enum: { type: 'string', enum: ['a', 'b', 'c'] },
a_compound_object: {
unevaluatedProperties: false,
allOf: [
{ type: 'object', properties: { a: { type: 'string' } } },
{ type: 'object', properties: { b: { type: 'number' } } },
],
}
}
}

const data = {
an_array: [],
an_enum: 'd',
a_compound_object: { stranger: 'danger', hello: 'world' },
space_odyssey: 42,
thirteen_sentinels: 426
}

const func = ajv.compile(schema)
func(data)

it('can parse multiple errors', () => {
expect(parser.parse(func.errors)).toEqual(
'Obj contains unsupported properties: space_odyssey, thirteen_sentinels, stranger, hello | ' +
'Obj MUST contain the missing properties: a_boolean, a_number | ' +
'Obj/an_enum MUST be equal to one of the allowed values: a, b, c | ' +
'Obj/an_array must NOT have fewer than 1 items'
)
});

it('can parse empty errors', () => {
const empty_func = ajv.compile({ type: 'object' })
empty_func({})
expect(parser.parse(empty_func.errors)).toEqual(ajv.errorsText(undefined))
});
});
12 changes: 6 additions & 6 deletions tools/tests/linter/SchemasValidator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ test('validate() - named_schemas', () => {
{
file: 'schemas/actions.yaml',
location: '#/components/schemas/Bark',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
},
{
file: 'schemas/animals.yaml',
location: '#/components/schemas/Dog',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
}
])
})
Expand All @@ -32,22 +32,22 @@ test('validate() - anonymous_schemas', () => {
{
file: '_global_parameters.yaml',
location: 'human',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
},
{
file: 'namespaces/_core.yaml',
location: '#/components/parameters/adopt::path.docket',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
},
{
file: 'namespaces/adopt.yaml',
location: '#/components/requestBodies/adopt/content/application/json',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
},
{
file: 'namespaces/_core.yaml',
location: '#/components/responses/adopt@200/content/application/json',
message: 'schema is invalid: data/type must be equal to one of the allowed values, data/type must be array, data/type must match a schema in anyOf'
message: 'data/type MUST be equal to one of the allowed values: array, boolean, integer, null, number, object, string --- data/type must be array --- data/type must match a schema in anyOf'
}
])
})
Loading

0 comments on commit e31da7f

Please sign in to comment.