Skip to content
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

Bug: Fixed performance issue with large schema dependencies and oneOf (#4203) #4204

Merged
merged 5 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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