diff --git a/CHANGELOG.md b/CHANGELOG.md index 3532e3d369..a37377c983 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,10 +21,13 @@ should change the heading of the (upcoming) version to include a major version b - Fix Vite development server [#3228](https://github.com/rjsf-team/react-jsonschema-form/issues/3228) ## @rjsf/validator-ajv8 +- BREAKING CHANGE: Disable form data validation for invalid JSON Schemas. Use @rjsf/validator-ajv6 if you need to validate against invalid schemas. - Fix additionalProperties validation [#3213](https://github.com/rjsf-team/react-jsonschema-form/issues/3213) - Report all schema errors thrown by Ajv. Previously, we would only report errors thrown for a missing meta-schema. This behavior is unchanged for @rjsf/validator-ajv6. - Disable Ajv strict mode by default. - Add RJSF-specific additional properties keywords to Ajv to prevent errors from being reported in strict mode. +- For JSON Schemas with `$id`s, use a pre-compiled Ajv validation function when available. +- No longer fail to validate inner schemas with `$id`s, fixing [#2821](https://github.com/rjsf-team/react-jsonschema-form/issues/2181). # 5.0.0-beta.12 diff --git a/packages/validator-ajv8/src/validator.ts b/packages/validator-ajv8/src/validator.ts index 6d36ed16cf..957e47b363 100644 --- a/packages/validator-ajv8/src/validator.ts +++ b/packages/validator-ajv8/src/validator.ts @@ -1,4 +1,4 @@ -import Ajv, { ErrorObject } from "ajv8"; +import Ajv, { ErrorObject, ValidateFunction } from "ajv8"; import toPath from "lodash/toPath"; import isObject from "lodash/isObject"; import clone from "lodash/clone"; @@ -245,22 +245,35 @@ export default class AJV8Validator< schema: RJSFSchema, formData?: T ): { errors?: Result[]; validationError?: Error } { - let validationError: Error | undefined = undefined; + let compilationError: Error | undefined = undefined; + let compiledValidator: ValidateFunction | undefined; + if (schema["$id"]) { + compiledValidator = this.ajv.getSchema(schema["$id"]); + } try { - this.ajv.validate(schema, formData); + if (compiledValidator === undefined) { + compiledValidator = this.ajv.compile(schema); + } + compiledValidator(formData); } catch (err) { - validationError = err as Error; + compilationError = err as Error; } - if (typeof this.localizer === "function") { - this.localizer(this.ajv.errors); - } - const errors = this.ajv.errors || undefined; + let errors; + if (compiledValidator) { + if (typeof this.localizer === "function") { + this.localizer(compiledValidator.errors); + } + errors = compiledValidator.errors || undefined; - // Clear errors to prevent persistent errors, see #1104 - this.ajv.errors = null; + // Clear errors to prevent persistent errors, see #1104 + compiledValidator.errors = null; + } - return { errors: errors as unknown as Result[], validationError }; + return { + errors: errors as unknown as Result[], + validationError: compilationError, + }; } /** This function processes the `formData` with an optional user contributed `customValidate` function, which receives @@ -366,25 +379,37 @@ export default class AJV8Validator< * false otherwise. If the schema is invalid, then this function will return * false. * - * @param schema - The schema against which to validate the form data * @param schema - * @param formData- - The form data to validate + * @param schema - The schema against which to validate the form data + * @param formData - The form data to validate * @param rootSchema - The root schema used to provide $ref resolutions */ isValid(schema: S, formData: T, rootSchema: S) { + const rootSchemaId = rootSchema["$id"] ?? ROOT_SCHEMA_PREFIX; try { // add the rootSchema ROOT_SCHEMA_PREFIX as id. // 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(this.withIdRefPrefix(schema), formData); + if (this.ajv.getSchema(rootSchemaId) === undefined) { + this.ajv.addSchema(rootSchema, rootSchemaId); + } + const schemaWithIdRefPrefix = this.withIdRefPrefix(schema) as S; + let compiledValidator: ValidateFunction | undefined; + if (schemaWithIdRefPrefix["$id"]) { + compiledValidator = this.ajv.getSchema(schemaWithIdRefPrefix["$id"]); + } + if (compiledValidator === undefined) { + compiledValidator = this.ajv.compile(schemaWithIdRefPrefix); + } + const result = compiledValidator(formData); return result as boolean; } 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(ROOT_SCHEMA_PREFIX); + this.ajv.removeSchema(rootSchemaId); } } diff --git a/packages/validator-ajv8/test/validator.test.ts b/packages/validator-ajv8/test/validator.test.ts index 9a9dbf1ec5..a133d22c66 100644 --- a/packages/validator-ajv8/test/validator.test.ts +++ b/packages/validator-ajv8/test/validator.test.ts @@ -72,6 +72,34 @@ describe("AJV8Validator", () => { expect(validator.isValid(schema, formData, rootSchema)).toBe(true); }); + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "schema-id", + }; + + const rootSchema: RJSFSchema = { + $id: "root-schema-id", + type: "object", + properties: { + name: { + type: "string", + }, + }, + }; + const formData = { + name: "John Doe", + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call isValid twice with the same schema + validator.isValid(schema, formData, rootSchema); + validator.isValid(schema, formData, rootSchema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); }); describe("validator.withIdRefPrefix()", () => { it("should recursively add id prefix to all refs", () => { @@ -250,6 +278,29 @@ describe("AJV8Validator", () => { expect(errorSchema.foo!.__errors![0]).toEqual("must be string"); }); }); + describe("Doesn't recompile a schema with a specified ID", () => { + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "this-schema-has-an-id", + type: "object", + properties: { + string: { + type: "string", + }, + }, + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call validateFormData twice with the same schema + validator.validateFormData({ string: "a" }, schema); + validator.validateFormData({ string: "b" }, schema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); + }); describe("TransformErrors", () => { let errors: RJSFValidationError[]; let newErrorMessage: string; @@ -401,26 +452,19 @@ describe("AJV8Validator", () => { errorSchema = result.errorSchema; }); it("should return an error list", () => { - expect(errors).toHaveLength(2); - expect(errors[0].name).toEqual("type"); - expect(errors[0].property).toEqual(".properties.foo.required"); - // TODO: This schema path is wrong due to a bug in ajv; change this test when https://github.com/ajv-validator/ajv/issues/512 is fixed. - expect(errors[0].schemaPath).toEqual( - "#/definitions/stringArray/type" - ); - expect(errors[0].message).toEqual("must be array"); - - expect(errors[1].stack).toEqual( + expect(errors).toHaveLength(1); + expect(errors[0].stack).toEqual( "schema is invalid: data/properties/foo/required must be array" ); }); it("should return an errorSchema", () => { - expect(errorSchema.properties!.foo!.required!.__errors).toHaveLength( - 1 - ); - expect(errorSchema.properties!.foo!.required!.__errors![0]).toEqual( - "must be array" - ); + expect(errorSchema).toEqual({ + $schema: { + __errors: [ + "schema is invalid: data/properties/foo/required must be array", + ], + }, + }); }); }); }); @@ -476,6 +520,34 @@ describe("AJV8Validator", () => { expect(validator.isValid(schema, formData, rootSchema)).toBe(true); }); + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "schema-id", + }; + + const rootSchema: RJSFSchema = { + $id: "root-schema-id", + type: "object", + properties: { + name: { + type: "string", + }, + }, + }; + const formData = { + name: "John Doe", + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call isValid twice with the same schema + validator.isValid(schema, formData, rootSchema); + validator.isValid(schema, formData, rootSchema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); }); describe("validator.withIdRefPrefix()", () => { it("should recursively add id prefix to all refs", () => { @@ -654,6 +726,30 @@ describe("AJV8Validator", () => { expect(errorSchema.foo!.__errors![0]).toEqual("must be string"); }); }); + describe("Doesn't recompile a schema with a specified ID", () => { + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "this-schema-has-an-id", + type: "object", + properties: { + string: { + title: "String field", + type: "string", + }, + }, + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call validateFormData twice with the same schema + validator.validateFormData({ string: "a" }, schema); + validator.validateFormData({ string: "b" }, schema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); + }); describe("TransformErrors", () => { let errors: RJSFValidationError[]; let newErrorMessage: string; @@ -805,24 +901,19 @@ describe("AJV8Validator", () => { errorSchema = result.errorSchema; }); it("should return an error list", () => { - expect(errors).toHaveLength(2); - expect(errors[0].name).toEqual("type"); - expect(errors[0].property).toEqual(".properties.foo.required"); - // Ajv2019 uses $defs rather than definitions - expect(errors[0].schemaPath).toEqual("#/$defs/stringArray/type"); - expect(errors[0].message).toEqual("must be array"); - - expect(errors[1].stack).toEqual( + expect(errors).toHaveLength(1); + expect(errors[0].stack).toEqual( "schema is invalid: data/properties/foo/required must be array" ); }); it("should return an errorSchema", () => { - expect(errorSchema.properties!.foo!.required!.__errors).toHaveLength( - 1 - ); - expect(errorSchema.properties!.foo!.required!.__errors![0]).toEqual( - "must be array" - ); + expect(errorSchema).toEqual({ + $schema: { + __errors: [ + "schema is invalid: data/properties/foo/required must be array", + ], + }, + }); }); }); }); @@ -878,6 +969,34 @@ describe("AJV8Validator", () => { expect(validator.isValid(schema, formData, rootSchema)).toBe(true); }); + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "schema-id", + }; + + const rootSchema: RJSFSchema = { + $id: "root-schema-id", + type: "object", + properties: { + name: { + type: "string", + }, + }, + }; + const formData = { + name: "John Doe", + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call isValid twice with the same schema + validator.isValid(schema, formData, rootSchema); + validator.isValid(schema, formData, rootSchema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); }); describe("validator.withIdRefPrefix()", () => { it("should recursively add id prefix to all refs", () => { @@ -1056,6 +1175,30 @@ describe("AJV8Validator", () => { expect(errorSchema.foo!.__errors![0]).toEqual("must be string"); }); }); + describe("Doesn't recompile a schema with a specified ID", () => { + it("Only compiles the schema once", () => { + const schema: RJSFSchema = { + $id: "this-schema-has-an-id", + type: "object", + properties: { + string: { + title: "String field", + type: "string", + }, + }, + }; + + // @ts-expect-error - accessing private Ajv instance to verify compilation happens once + const compileSpy = jest.spyOn(validator.ajv, "compile"); + compileSpy.mockClear(); + + // Call validateFormData twice with the same schema + validator.validateFormData({ string: "a" }, schema); + validator.validateFormData({ string: "b" }, schema); + + expect(compileSpy).toHaveBeenCalledTimes(1); + }); + }); describe("TransformErrors", () => { let errors: RJSFValidationError[]; let newErrorMessage: string; @@ -1207,24 +1350,19 @@ describe("AJV8Validator", () => { errorSchema = result.errorSchema; }); it("should return an error list", () => { - expect(errors).toHaveLength(2); - expect(errors[0].name).toEqual("type"); - expect(errors[0].property).toEqual(".properties.foo.required"); - // Ajv2019 uses $defs rather than definitions - expect(errors[0].schemaPath).toEqual("#/$defs/stringArray/type"); - expect(errors[0].message).toEqual("must be array"); - - expect(errors[1].stack).toEqual( + expect(errors).toHaveLength(1); + expect(errors[0].stack).toEqual( "schema is invalid: data/properties/foo/required must be array" ); }); it("should return an errorSchema", () => { - expect(errorSchema.properties!.foo!.required!.__errors).toHaveLength( - 1 - ); - expect(errorSchema.properties!.foo!.required!.__errors![0]).toEqual( - "must be array" - ); + expect(errorSchema).toEqual({ + $schema: { + __errors: [ + "schema is invalid: data/properties/foo/required must be array", + ], + }, + }); }); }); }); @@ -1264,7 +1402,7 @@ describe("AJV8Validator", () => { expect(errors.errorSchema).toEqual({ $schema: { __errors: [errMessage] }, }); - expect(localizer).toHaveBeenCalledWith(undefined); + expect(localizer).not.toHaveBeenCalled(); }); describe("validating using single custom meta schema", () => { let errors: RJSFValidationError[]; @@ -1430,7 +1568,7 @@ describe("AJV8Validator", () => { expect(errors.errorSchema).toEqual({ $schema: { __errors: [errMessage] }, }); - expect(localizer).toHaveBeenCalledWith(undefined); + expect(localizer).not.toHaveBeenCalled(); }); describe("validating using single custom meta schema", () => { let errors: RJSFValidationError[]; @@ -1598,7 +1736,7 @@ describe("AJV8Validator", () => { expect(errors.errorSchema).toEqual({ $schema: { __errors: [errMessage] }, }); - expect(localizer).toHaveBeenCalledWith(undefined); + expect(localizer).not.toHaveBeenCalled(); }); describe("validating using single custom meta schema", () => { let errors: RJSFValidationError[];