Skip to content

Commit 7bcfc4a

Browse files
abdalla-rkoAbdallah Al-Soqatriheath-freenome
authored
Bug: Fixed performance issue with large schema dependencies and oneOf (#4203) (#4204)
* Fixed performance issue #4203 * code improvement based on feedback --------- Co-authored-by: Abdallah Al-Soqatri <[email protected]> Co-authored-by: Heath C <[email protected]>
1 parent cbb8fe7 commit 7bcfc4a

File tree

4 files changed

+50
-21
lines changed

4 files changed

+50
-21
lines changed

CHANGELOG.md

+8
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ should change the heading of the (upcoming) version to include a major version b
2222

2323
- 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]
2424

25+
## @rjsf/validator-ajv6
26+
27+
- Improved performance issues with large schema dependencies and oneOf conditions [#4203](https://github.com/rjsf-team/react-jsonschema-form/issues/4203).
28+
29+
## @rjsf/validator-ajv8
30+
31+
- Improved performance issues with large schema dependencies and oneOf conditions [#4203](https://github.com/rjsf-team/react-jsonschema-form/issues/4203).
32+
2533
# 5.18.4
2634

2735
## Dev / docs / playground

packages/validator-ajv6/src/validator.ts

+20-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Ajv, ErrorObject } from 'ajv';
22
import {
33
createErrorHandler,
44
CustomValidator,
5+
deepEquals,
56
ErrorSchema,
67
ErrorTransformer,
78
FormContextType,
@@ -158,6 +159,23 @@ export default class AJV6Validator<T = any, S extends StrictRJSFSchema = RJSFSch
158159
return validationDataMerge<T>({ errors, errorSchema }, userErrorSchema);
159160
}
160161

162+
/**
163+
* 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.
164+
* @param rootSchema - The root schema used to provide $ref resolutions
165+
*/
166+
handleSchemaUpdate(rootSchema: RJSFSchema): void {
167+
const rootSchemaId = ROOT_SCHEMA_PREFIX;
168+
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
169+
// if schema validator instance doesn't exist, add it.
170+
// else 'handleRootSchemaChange' should be called if the root schema changes so we don't have to remove and recompile the schema every run.
171+
if (this.ajv.getSchema(ROOT_SCHEMA_PREFIX) === undefined) {
172+
this.ajv.addSchema(rootSchema, ROOT_SCHEMA_PREFIX);
173+
} else if (!deepEquals(rootSchema, this.ajv.getSchema(ROOT_SCHEMA_PREFIX)?.schema)) {
174+
this.ajv.removeSchema(rootSchemaId);
175+
this.ajv.addSchema(rootSchema, rootSchemaId);
176+
}
177+
}
178+
161179
/** Validates data against a schema, returning true if the data is valid, or
162180
* false otherwise. If the schema is invalid, then this function will return
163181
* false.
@@ -168,17 +186,14 @@ export default class AJV6Validator<T = any, S extends StrictRJSFSchema = RJSFSch
168186
*/
169187
isValid(schema: RJSFSchema, formData: T | undefined, rootSchema: RJSFSchema) {
170188
try {
171-
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
189+
this.handleSchemaUpdate(rootSchema);
172190
// then rewrite the schema ref's to point to the rootSchema
173191
// this accounts for the case where schema have references to models
174192
// that lives in the rootSchema but not in the schema in question.
175-
const result = this.ajv.addSchema(rootSchema, ROOT_SCHEMA_PREFIX).validate(withIdRefPrefix(schema), formData);
193+
const result = this.ajv.validate(withIdRefPrefix(schema), formData);
176194
return result as boolean;
177195
} catch (e) {
178196
return false;
179-
} finally {
180-
// make sure we remove the rootSchema from the global ajv instance
181-
this.ajv.removeSchema(ROOT_SCHEMA_PREFIX);
182197
}
183198
}
184199
}

packages/validator-ajv8/src/validator.ts

+19-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Ajv, { ErrorObject, ValidateFunction } from 'ajv';
22
import {
33
CustomValidator,
4+
deepEquals,
45
ErrorSchema,
56
ErrorTransformer,
67
FormContextType,
@@ -119,6 +120,23 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
119120
return processRawValidationErrors(this, rawErrors, formData, schema, customValidate, transformErrors, uiSchema);
120121
}
121122

123+
/**
124+
* 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.
125+
* @param rootSchema - The root schema used to provide $ref resolutions
126+
*/
127+
handleSchemaUpdate(rootSchema: S): void {
128+
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
129+
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
130+
// if schema validator instance doesn't exist, add it.
131+
// 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.
132+
if (this.ajv.getSchema(rootSchemaId) === undefined) {
133+
this.ajv.addSchema(rootSchema, rootSchemaId);
134+
} else if (!deepEquals(rootSchema, this.ajv.getSchema(rootSchemaId)?.schema)) {
135+
this.ajv.removeSchema(rootSchemaId);
136+
this.ajv.addSchema(rootSchema, rootSchemaId);
137+
}
138+
}
139+
122140
/** Validates data against a schema, returning true if the data is valid, or
123141
* false otherwise. If the schema is invalid, then this function will return
124142
* false.
@@ -128,16 +146,11 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
128146
* @param rootSchema - The root schema used to provide $ref resolutions
129147
*/
130148
isValid(schema: S, formData: T | undefined, rootSchema: S) {
131-
const rootSchemaId = rootSchema[ID_KEY] ?? ROOT_SCHEMA_PREFIX;
132149
try {
133-
// add the rootSchema ROOT_SCHEMA_PREFIX as id.
150+
this.handleSchemaUpdate(rootSchema);
134151
// then rewrite the schema ref's to point to the rootSchema
135152
// this accounts for the case where schema have references to models
136153
// that lives in the rootSchema but not in the schema in question.
137-
// if (this.ajv.getSchema(rootSchemaId) === undefined) {
138-
// TODO restore the commented out `if` above when the TODO in the `finally` is completed
139-
this.ajv.addSchema(rootSchema, rootSchemaId);
140-
// }
141154
const schemaWithIdRefPrefix = withIdRefPrefix<S>(schema) as S;
142155
const schemaId = schemaWithIdRefPrefix[ID_KEY] ?? hashForSchema(schemaWithIdRefPrefix);
143156
let compiledValidator: ValidateFunction | undefined;
@@ -155,10 +168,6 @@ export default class AJV8Validator<T = any, S extends StrictRJSFSchema = RJSFSch
155168
} catch (e) {
156169
console.warn('Error encountered compiling schema:', e);
157170
return false;
158-
} finally {
159-
// TODO: A function should be called if the root schema changes so we don't have to remove and recompile the schema every run.
160-
// make sure we remove the rootSchema from the global ajv instance
161-
this.ajv.removeSchema(rootSchemaId);
162171
}
163172
}
164173
}

packages/validator-ajv8/test/validator.test.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,9 @@ describe('AJV8Validator', () => {
9999
validator.isValid(schema, formData, rootSchema);
100100

101101
// Root schema is added twice
102-
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
102+
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
103103
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
104104
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
105-
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
106105
});
107106
it('should fallback to using compile', () => {
108107
const schema: RJSFSchema = {
@@ -594,10 +593,9 @@ describe('AJV8Validator', () => {
594593
validator.isValid(schema, formData, rootSchema);
595594

596595
// Root schema is added twice
597-
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
596+
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
598597
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
599598
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
600-
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
601599
});
602600
});
603601
describe('validator.toErrorList()', () => {
@@ -1050,10 +1048,9 @@ describe('AJV8Validator', () => {
10501048
validator.isValid(schema, formData, rootSchema);
10511049

10521050
// Root schema is added twice
1053-
expect(addSchemaSpy).toHaveBeenCalledTimes(3);
1051+
expect(addSchemaSpy).toHaveBeenCalledTimes(2);
10541052
expect(addSchemaSpy).toHaveBeenNthCalledWith(1, expect.objectContaining(rootSchema), rootSchema.$id);
10551053
expect(addSchemaSpy).toHaveBeenNthCalledWith(2, expect.objectContaining(schema), schema.$id);
1056-
expect(addSchemaSpy).toHaveBeenLastCalledWith(expect.objectContaining(rootSchema), rootSchema.$id);
10571054
});
10581055
});
10591056
describe('validator.toErrorList()', () => {

0 commit comments

Comments
 (0)