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 1 commit
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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ should change the heading of the (upcoming) version to include a major version b

-->

# 5.18.5

## @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).

abdalla-rko marked this conversation as resolved.
Show resolved Hide resolved
# 5.18.4

## Dev / docs / playground
Expand Down
24 changes: 20 additions & 4 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,16 @@ export default class AJV6Validator<T = any, S extends StrictRJSFSchema = RJSFSch
return validationDataMerge<T>({ errors, errorSchema }, userErrorSchema);
}

/**
* This function is called when the root schema changes. 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
*/
handleRootSchemaChange(rootSchema: RJSFSchema): void {
const rootSchemaId = ROOT_SCHEMA_PREFIX;
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 @@ -169,16 +180,21 @@ 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.
// 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) {
// TODO restore the commented out `if` above when the TODO in the `finally` is completed
this.ajv.addSchema(rootSchema, ROOT_SCHEMA_PREFIX);
} else if (!deepEquals(rootSchema, this.ajv.getSchema(ROOT_SCHEMA_PREFIX)?.schema)) {
this.handleRootSchemaChange(rootSchema);
}
abdalla-rko marked this conversation as resolved.
Show resolved Hide resolved
// 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);
}
}
}
27 changes: 19 additions & 8 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,16 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
return processRawValidationErrors(this, rawErrors, formData, schema, customValidate, transformErrors, uiSchema);
}

/**
* This function is called when the root schema changes. 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
*/
handleRootSchemaChange(rootSchema: S): void {
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
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 @@ -131,13 +142,17 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
try {
// 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(rootSchemaId) === undefined) {
// TODO restore the commented out `if` above when the TODO in the `finally` is completed
this.ajv.addSchema(rootSchema, rootSchemaId);
} else if (!deepEquals(rootSchema, this.ajv.getSchema(rootSchemaId)?.schema)) {
this.handleRootSchemaChange(rootSchema);
}
abdalla-rko marked this conversation as resolved.
Show resolved Hide resolved
// 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 +170,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