feat: precompile cloudevent schema #471
Conversation
This commit modifies the build pipleline so that the cloudevent schema is precompiled for runtime validation. This eliminates the need to compile the schema at runtime, improving both performance and security. Fixes: cloudevents#423 Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
lance
force-pushed
the
lance/423-precompile-schemas
branch
from
2235811
to
9e9c0f0
Compare
| if (!isValidAgainstSchemaV1(event)) { | ||
| throw new ValidationError("invalid payload", isValidAgainstSchemaV1.errors); | ||
| if (!validate(event)) { | ||
| throw new ValidationError("invalid payload", (validate as any).errors); |
There was a problem hiding this comment.
(validate as any) seems a bit off. Why do we need to cast to any?
I'm seeing validate as a function:
function validate(data: any, { instancePath, parentData, parentDataProperty, rootData }?: {
instancePath?: string | undefined;
parentData: any;
parentDataProperty: any;
rootData?: any;
}): booleanThere was a problem hiding this comment.
The function that AJV generates has an errors object attached to it if there are errors found when validating with validate(data). It's a non-optimal API. Here you can see it in the generated validation code. I welcome suggestions on how to deal with this another way.
function validate20(data, { instancePath = "", parentData, parentDataProperty, rootData = data } = {}) {
let vErrors = null;
let errors = 0;
if (errors === 0) {
if (data && typeof data == "object" && !Array.isArray(data)) {
let missing0;
if (
(data.id === undefined && (missing0 = "id")) ||
(data.source === undefined && (missing0 = "source")) ||
(data.specversion === undefined && (missing0 = "specversion")) ||
(data.type === undefined && (missing0 = "type"))
) {
validate20.errors = [
{
instancePath,
schemaPath: "#/required",
keyword: "required",
params: { missingProperty: missing0 },
message: `must have required property '${ missing0 }'`,
},
];
return false;
}| } | ||
| }, | ||
| "required": ["id", "source", "specversion", "type"], | ||
| "definitions": { |
There was a problem hiding this comment.
I don't think the definitions need to have the suffix of def. Can you remove this suffix from the definitions?
There was a problem hiding this comment.
This is taken directly from the CloudEvent specification here: https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/formats/cloudevents.json. I would prefer it if we can use this document unchanged.
Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
|
@grant requested changes have been made - ptal |
ajv seems to have some bugs around precompiling and formats, particularly when using ESM: * ajv-validator/ajv#1837 * ajv-validator/ajv-cli#200 Workaround for non-esm works for cloudevents/sdk-javascript#471 So if we want to work with this, we should look at how that is done, but it is taking too many cycles to pursue for now.
This commit modifies the build pipleline so that the cloudevent schema is
precompiled for runtime validation. This eliminates the need to compile the
schema at runtime, improving both performance and security.
Fixes: #423
Signed-off-by: Lance Ball lball@redhat.com
Note: There are a number of changes related to semi-dead code from spec version 0.3 that did not get removed when we dropped support for that version.