-
Notifications
You must be signed in to change notification settings - Fork 11
feat(input_schema): Enable sub-schemas in input-schema #519
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
Changes from 9 commits
2c3e57f
a0ad3d2
f424267
bf0019b
4585ed1
094cf8f
4b91ad2
14c0caf
686e588
0398ea2
37c58d3
58808f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,11 @@ export function parseAjvError( | |
| let fieldKey: string; | ||
| let message: string; | ||
|
|
||
| // remove leading and trailing slashes and replace remaining slashes with dots | ||
| const cleanPropertyName = (name: string) => { | ||
| return name.replace(/^\/|\/$/g, '').replace(/\//g, '.'); | ||
| }; | ||
|
|
||
| // If error is with keyword type, it means that type of input is incorrect | ||
| // this can mean that provided value is null | ||
| if (error.keyword === 'type') { | ||
|
|
@@ -48,20 +53,23 @@ export function parseAjvError( | |
| } | ||
| message = m('inputSchema.validation.generic', { rootName, fieldKey, message: error.message }); | ||
| } else if (error.keyword === 'required') { | ||
| fieldKey = error.params.missingProperty; | ||
| fieldKey = cleanPropertyName(`${error.instancePath}/${error.params.missingProperty}`); | ||
| message = m('inputSchema.validation.required', { rootName, fieldKey }); | ||
| } else if (error.keyword === 'additionalProperties') { | ||
| fieldKey = error.params.additionalProperty; | ||
| fieldKey = cleanPropertyName(`${error.instancePath}/${error.params.additionalProperty}`); | ||
| message = m('inputSchema.validation.additionalProperty', { rootName, fieldKey }); | ||
| } else if (error.keyword === 'unevaluatedProperties') { | ||
| fieldKey = cleanPropertyName(`${error.instancePath}/${error.params.unevaluatedProperty}`); | ||
| message = m('inputSchema.validation.additionalProperty', { rootName, fieldKey }); | ||
| } else if (error.keyword === 'enum') { | ||
| fieldKey = error.instancePath.split('/').pop()!; | ||
| fieldKey = cleanPropertyName(error.instancePath); | ||
| const errorMessage = `${error.message}: "${error.params.allowedValues.join('", "')}"`; | ||
| message = m('inputSchema.validation.generic', { rootName, fieldKey, message: errorMessage }); | ||
| } else if (error.keyword === 'const') { | ||
| fieldKey = error.instancePath.split('/').pop()!; | ||
| fieldKey = cleanPropertyName(error.instancePath); | ||
| message = m('inputSchema.validation.generic', { rootName, fieldKey, message: error.message }); | ||
| } else { | ||
| fieldKey = error.instancePath.split('/').pop()!; | ||
| fieldKey = cleanPropertyName(error.instancePath); | ||
| message = m('inputSchema.validation.generic', { rootName, fieldKey, message: error.message }); | ||
| } | ||
|
|
||
|
|
@@ -92,11 +100,33 @@ function validateBasicStructure(validator: Ajv, obj: Record<string, unknown>): a | |
|
|
||
| /** | ||
| * Validates particular field against it's schema. | ||
| * @param validator An instance of AJV validator (must support draft 2019-09). | ||
| * @param fieldSchema Schema of the field to validate. | ||
| * @param fieldKey Key of the field in the input schema. | ||
| * @param subField If true, the field is a sub-field of another field, so we need to skip some definitions. | ||
| */ | ||
| function validateField(validator: Ajv, fieldSchema: Record<string, unknown>, fieldKey: string): asserts fieldSchema is FieldDefinition { | ||
| function validateField(validator: Ajv, fieldSchema: Record<string, unknown>, fieldKey: string, subField = false): asserts fieldSchema is FieldDefinition { | ||
mfori marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const matchingDefinitions = Object | ||
| .values<any>(definitions) // cast as any, as the code in first branch seems to be invalid | ||
| .filter((definition) => { | ||
| // Because the definitions contains not only the root properties definitions, but also sub-schema definitions | ||
| // and utility definitions, we need to filter them out and validate only against the appropriate ones. | ||
| // We do this by checking prefix of the definition title (Utils: or Sub-schema:) | ||
|
||
|
|
||
| if (definition.title.startsWith('Utils:')) { | ||
| // Utility definitions are not used for property validation. | ||
| // They are used for their internal logic. Filter them out. | ||
| return false; | ||
| } | ||
| if (!subField && definition.title.startsWith('Sub-schema:')) { | ||
| // This is a sub-schema definition, but we are validating a root field, so we skip it. | ||
| return false; | ||
| } | ||
| if (subField && !definition.title.startsWith('Sub-schema:')) { | ||
| // This is a normal definition, but we are validating a sub-field, so we skip it. | ||
| return false; | ||
| } | ||
|
|
||
| return definition.properties.type.enum | ||
| // This is a normal case where fieldSchema.type can be only one possible value matching definition.properties.type.enum.0 | ||
| ? definition.properties.type.enum[0] === fieldSchema.type | ||
|
|
@@ -110,9 +140,19 @@ function validateField(validator: Ajv, fieldSchema: Record<string, unknown>, fie | |
| throw new Error(`Input schema is not valid (${errorMessage})`); | ||
| } | ||
|
|
||
| // We are validating a field schema against one of the definitions, but one definition can reference other definitions. | ||
| // So this basically creates a new JSON Schema with a picked definition at root and puts all definitions from the `schema.json` | ||
| // into the `definitions` property of this final schema. | ||
| const enhanceDefinition = (definition: object) => { | ||
| return { | ||
| ...definition, | ||
| definitions, | ||
| }; | ||
| }; | ||
|
|
||
| // If there is only one matching then we are done and simply compare it. | ||
| if (matchingDefinitions.length === 1) { | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, matchingDefinitions[0], `schema.properties.${fieldKey}`); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, enhanceDefinition(matchingDefinitions[0]), `schema.properties.${fieldKey}`); | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -121,30 +161,49 @@ function validateField(validator: Ajv, fieldSchema: Record<string, unknown>, fie | |
| if ((fieldSchema as StringFieldDefinition).enum) { | ||
| const definition = matchingDefinitions.filter((item) => !!item.properties.enum).pop(); | ||
| if (!definition) throw new Error('Input schema validation failed to find "enum property" definition'); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, definition, `schema.properties.${fieldKey}.enum`); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, enhanceDefinition(definition), `schema.properties.${fieldKey}.enum`); | ||
| return; | ||
| } | ||
| // If the definition contains "resourceType" property then it's resource type. | ||
| if ((fieldSchema as CommonResourceFieldDefinition<unknown>).resourceType) { | ||
| const definition = matchingDefinitions.filter((item) => !!item.properties.resourceType).pop(); | ||
| if (!definition) throw new Error('Input schema validation failed to find "resource property" definition'); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, definition, `schema.properties.${fieldKey}`); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, enhanceDefinition(definition), `schema.properties.${fieldKey}`); | ||
| return; | ||
| } | ||
| // Otherwise we use the other definition. | ||
| const definition = matchingDefinitions.filter((item) => !item.properties.enum && !item.properties.resourceType).pop(); | ||
| if (!definition) throw new Error('Input schema validation failed to find other than "enum property" definition'); | ||
|
|
||
| validateAgainstSchemaOrThrow(validator, fieldSchema, definition, `schema.properties.${fieldKey}`); | ||
| validateAgainstSchemaOrThrow(validator, fieldSchema, enhanceDefinition(definition), `schema.properties.${fieldKey}`); | ||
| } | ||
|
|
||
| /** | ||
| * Validates all subfields (and their subfields) of a given field schema. | ||
| */ | ||
| function validateSubFields(validator: Ajv, fieldSchema: InputSchemaBaseChecked, fieldKey: string) { | ||
| Object.entries(fieldSchema.properties).forEach(([subFieldKey, subFieldSchema]) => { | ||
| // The sub-properties has to be validated first, so we got more relevant error messages. | ||
| if ((subFieldSchema as any).properties) { | ||
| // If the field has sub-fields, we need to validate them as well. | ||
| validateSubFields(validator, subFieldSchema as any as InputSchemaBaseChecked, `${fieldKey}.${subFieldKey}`); | ||
| } | ||
| validateField(validator, subFieldSchema, `${fieldKey}.${subFieldKey}`, true); | ||
| }); | ||
mfori marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * Validates all properties in the input schema | ||
| */ | ||
| function validateProperties(inputSchema: InputSchemaBaseChecked, validator: Ajv): asserts inputSchema is InputSchema { | ||
| Object.entries(inputSchema.properties).forEach(([fieldKey, fieldSchema]) => ( | ||
| validateField(validator, fieldSchema, fieldKey)), | ||
| ); | ||
| Object.entries(inputSchema.properties).forEach(([fieldKey, fieldSchema]) => { | ||
| // The sub-properties has to be validated first, so we got more relevant error messages. | ||
| if ((fieldSchema as any).properties) { | ||
| // If the field has sub-fields, we need to validate them as well. | ||
| validateSubFields(validator, fieldSchema as any as InputSchemaBaseChecked, fieldKey); | ||
| } | ||
| validateField(validator, fieldSchema, fieldKey); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -168,6 +227,10 @@ export function validateExistenceOfRequiredFields(inputSchema: InputSchema) { | |
| * then checks that all required fields are present and finally checks fully against the whole schema. | ||
| * | ||
| * This way we get the most accurate error message for user. | ||
| * | ||
| * @param validator An instance of AJV validator. Important: The JSON Schema that the passed input schema is validated against | ||
| * is using features from JSON Schema 2019 draft, so the AJV instance must support it. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR already more than doubled it's length, but without utilising the But to support this the to It should be these places (we should change it together with bumping version of
Note: the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we maybe add
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added check ( |
||
| * @param inputSchema Input schema to validate. | ||
| */ | ||
| export function validateInputSchema(validator: Ajv, inputSchema: Record<string, unknown>): asserts inputSchema is InputSchema { | ||
| // First validate just basic structure without fields. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed, because now error can be related to sub-properties and we want to show nice path.