Skip to content

Commit

Permalink
Bug: Fixed performance issue with large schema dependencies and oneOf (
Browse files Browse the repository at this point in the history
…#4203) (#4204)

* Fixed performance issue #4203

* code improvement based on feedback

---------

Co-authored-by: Abdallah Al-Soqatri <abdallah.al-soqatri@aspentech.com>
Co-authored-by: Heath C <51679588+heath-freenome@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 21, 2024
1 parent cbb8fe7 commit 7bcfc4a
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 21 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ should change the heading of the (upcoming) version to include a major version b

- Fix case where NumberField would not properly reset the field when using programmatic form reset (#4202)[https://github.com/rjsf-team/react-jsonschema-form/issues/4202]

## @rjsf/validator-ajv6

- Improved performance issues with large schema dependencies and oneOf conditions [#4203](https://github.com/rjsf-team/react-jsonschema-form/issues/4203).

## @rjsf/validator-ajv8

- Improved performance issues with large schema dependencies and oneOf conditions [#4203](https://github.com/rjsf-team/react-jsonschema-form/issues/4203).

# 5.18.4

## Dev / docs / playground
Expand Down
25 changes: 20 additions & 5 deletions packages/validator-ajv6/src/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Ajv, ErrorObject } from 'ajv';
import {
createErrorHandler,
CustomValidator,
deepEquals,
ErrorSchema,
ErrorTransformer,
FormContextType,
Expand Down Expand Up @@ -158,6 +159,23 @@ export default class AJV6Validator<T = any, S extends StrictRJSFSchema = RJSFSch
return validationDataMerge<T>({ errors, errorSchema }, userErrorSchema);
}

/**
* This function checks if a schema needs to be added and if the root schemas don't match it removes the old root schema from the ajv instance and adds the new one.
* @param rootSchema - The root schema used to provide $ref resolutions
*/
handleSchemaUpdate(rootSchema: RJSFSchema): void {
const rootSchemaId = ROOT_SCHEMA_PREFIX;
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
// if schema validator instance doesn't exist, add it.
// else 'handleRootSchemaChange' should be called if the root schema changes so we don't have to remove and recompile the schema every run.
if (this.ajv.getSchema(ROOT_SCHEMA_PREFIX) === undefined) {
this.ajv.addSchema(rootSchema, ROOT_SCHEMA_PREFIX);
} else if (!deepEquals(rootSchema, this.ajv.getSchema(ROOT_SCHEMA_PREFIX)?.schema)) {
this.ajv.removeSchema(rootSchemaId);
this.ajv.addSchema(rootSchema, rootSchemaId);
}
}

/** Validates data against a schema, returning true if the data is valid, or
* false otherwise. If the schema is invalid, then this function will return
* false.
Expand All @@ -168,17 +186,14 @@ export default class AJV6Validator<T = any, S extends StrictRJSFSchema = RJSFSch
*/
isValid(schema: RJSFSchema, formData: T | undefined, rootSchema: RJSFSchema) {
try {
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
this.handleSchemaUpdate(rootSchema);
// then rewrite the schema ref's to point to the rootSchema
// this accounts for the case where schema have references to models
// that lives in the rootSchema but not in the schema in question.
const result = this.ajv.addSchema(rootSchema, ROOT_SCHEMA_PREFIX).validate(withIdRefPrefix(schema), formData);
const result = this.ajv.validate(withIdRefPrefix(schema), formData);
return result as boolean;
} catch (e) {
return false;
} finally {
// make sure we remove the rootSchema from the global ajv instance
this.ajv.removeSchema(ROOT_SCHEMA_PREFIX);
}
}
}
29 changes: 19 additions & 10 deletions packages/validator-ajv8/src/validator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Ajv, { ErrorObject, ValidateFunction } from 'ajv';
import {
CustomValidator,
deepEquals,
ErrorSchema,
ErrorTransformer,
FormContextType,
Expand Down Expand Up @@ -119,6 +120,23 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
return processRawValidationErrors(this, rawErrors, formData, schema, customValidate, transformErrors, uiSchema);
}

/**
* This function checks if a schema needs to be added and if the root schemas don't match it removes the old root schema from the ajv instance and adds the new one.
* @param rootSchema - The root schema used to provide $ref resolutions
*/
handleSchemaUpdate(rootSchema: S): void {
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
// if schema validator instance doesn't exist, add it.
// else if the root schemas don't match, we should remove and add the root schema so we don't have to remove and recompile the schema every run.
if (this.ajv.getSchema(rootSchemaId) === undefined) {
this.ajv.addSchema(rootSchema, rootSchemaId);
} else if (!deepEquals(rootSchema, this.ajv.getSchema(rootSchemaId)?.schema)) {
this.ajv.removeSchema(rootSchemaId);
this.ajv.addSchema(rootSchema, rootSchemaId);
}
}

/** Validates data against a schema, returning true if the data is valid, or
* false otherwise. If the schema is invalid, then this function will return
* false.
Expand All @@ -128,16 +146,11 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
* @param rootSchema - The root schema used to provide $ref resolutions
*/
isValid(schema: S, formData: T | undefined, rootSchema: S) {
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
try {
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
this.handleSchemaUpdate(rootSchema);
// then rewrite the schema ref's to point to the rootSchema
// this accounts for the case where schema have references to models
// that lives in the rootSchema but not in the schema in question.
// if (this.ajv.getSchema(rootSchemaId) === undefined) {
// TODO restore the commented out `if` above when the TODO in the `finally` is completed
this.ajv.addSchema(rootSchema, rootSchemaId);
// }
const schemaWithIdRefPrefix = withIdRefPrefix<S>(schema) as S;
const schemaId = schemaWithIdRefPrefix[ID_KEY] ?? hashForSchema(schemaWithIdRefPrefix);
let compiledValidator: ValidateFunction | undefined;
Expand All @@ -155,10 +168,6 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
} catch (e) {
console.warn('Error encountered compiling schema:', e);
return false;
} finally {
// TODO: A function should be called if the root schema changes so we don't have to remove and recompile the schema every run.
// make sure we remove the rootSchema from the global ajv instance
this.ajv.removeSchema(rootSchemaId);
}
}
}
9 changes: 3 additions & 6 deletions packages/validator-ajv8/test/validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,9 @@ describe('AJV8Validator', () => {
validator.isValid(schema, formData, rootSchema);

// Root schema is added twice
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
});
it('should fallback to using compile', () => {
const schema: RJSFSchema = {
Expand Down Expand Up @@ -594,10 +593,9 @@ describe('AJV8Validator', () => {
validator.isValid(schema, formData, rootSchema);

// Root schema is added twice
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
});
});
describe('validator.toErrorList()', () => {
Expand Down Expand Up @@ -1050,10 +1048,9 @@ describe('AJV8Validator', () => {
validator.isValid(schema, formData, rootSchema);

// Root schema is added twice
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
});
});
describe('validator.toErrorList()', () => {
Expand Down

0 comments on commit 7bcfc4a

Please sign in to comment.