Skip to content

Revert "Optimise Ajv cache usage" #1070

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
12 changes: 5 additions & 7 deletions src/middlewares/openapi.request.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
ContentType,
ajvErrorsToValidatorError,
augmentAjvErrors,
useAjvCache,
} from './util';

type OperationObject = OpenAPIV3.OperationObject;
Expand All @@ -29,6 +28,7 @@ type ReferenceObject = OpenAPIV3.ReferenceObject;
type SecurityRequirementObject = OpenAPIV3.SecurityRequirementObject;
type SecuritySchemeObject = OpenAPIV3.SecuritySchemeObject;
type ApiKeySecurityScheme = OpenAPIV3.ApiKeySecurityScheme;

export class RequestValidator {
private middlewareCache: { [key: string]: RequestHandler } = {};
private apiDoc: OpenAPIV3.DocumentV3 | OpenAPIV3.DocumentV3_1;
Expand Down Expand Up @@ -79,7 +79,7 @@ export class RequestValidator {
const key = `${req.method}-${path}-${contentTypeKey}`;

if (!this.middlewareCache[key]) {
const middleware = this.buildMiddleware(path, reqSchema, contentType, key);
const middleware = this.buildMiddleware(path, reqSchema, contentType);
this.middlewareCache[key] = middleware;
}
return this.middlewareCache[key](req, res, next);
Expand All @@ -103,7 +103,6 @@ export class RequestValidator {
path: string,
reqSchema: OperationObject,
contentType: ContentType,
ajvCacheKey: string
): RequestHandler {
const apiDoc = this.apiDoc;
const schemaParser = new ParametersSchemaParser(this.ajv, apiDoc);
Expand All @@ -113,7 +112,7 @@ export class RequestValidator {
const validator = new Validator(this.apiDoc, parameters, body, {
general: this.ajv,
body: this.ajvBody,
}, ajvCacheKey);
});

const allowUnknownQueryParameters = !!(
reqSchema['x-eov-allow-unknown-query-parameters'] ??
Expand Down Expand Up @@ -306,7 +305,6 @@ class Validator {
general: Ajv;
body: Ajv;
},
ajvCacheKey: string
) {
this.apiDoc = apiDoc;
this.schemaGeneral = this._schemaGeneral(parametersSchema);
Expand All @@ -315,8 +313,8 @@ class Validator {
...(<any>this.schemaGeneral).properties, // query, header, params props
body: (<any>this.schemaBody).properties.body, // body props
};
this.validatorGeneral = useAjvCache(ajv.general, this.schemaGeneral, ajvCacheKey);
this.validatorBody = useAjvCache(ajv.body, this.schemaBody, ajvCacheKey);
this.validatorGeneral = ajv.general.compile(this.schemaGeneral);
this.validatorBody = ajv.body.compile(this.schemaBody);
}

private _schemaGeneral(parameters: ParametersSchema): object {
Expand Down
9 changes: 4 additions & 5 deletions src/middlewares/openapi.response.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
augmentAjvErrors,
ContentType,
ajvErrorsToValidatorError,
findResponseContent, useAjvCache,
findResponseContent,
} from './util';
import {
OpenAPIV3,
Expand Down Expand Up @@ -109,7 +109,7 @@ export class ResponseValidator {

let validators = this.validatorsCache[key];
if (!validators) {
validators = this.buildValidators(responses, key);
validators = this.buildValidators(responses);
this.validatorsCache[key] = validators;
}
return validators;
Expand Down Expand Up @@ -212,7 +212,7 @@ export class ResponseValidator {
* @param responses
* @returns a map of validators
*/
private buildValidators(responses: OpenAPIV3.ResponsesObject, ajvCacheKey: string): {
private buildValidators(responses: OpenAPIV3.ResponsesObject): {
[key: string]: ValidateFunction;
} {
const validationTypes = (response) => {
Expand Down Expand Up @@ -295,10 +295,9 @@ export class ResponseValidator {
const schema = contentTypeSchemas[contentType];
schema.paths = this.spec.paths; // add paths for resolution with multi-file
schema.components = this.spec.components; // add components for resolution w/ multi-file
const validator = useAjvCache(this.ajvBody, <object>schema, `${ajvCacheKey}-${code}-${contentType}`)
validators[code] = {
...validators[code],
[contentType]: validator,
[contentType]: this.ajvBody.compile(<object>schema),
};
}
}
Expand Down
19 changes: 1 addition & 18 deletions src/middlewares/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { ErrorObject } from 'ajv-draft-04';
import { Request } from 'express';
import { AjvInstance, ValidationError } from '../framework/types';
import { ValidationError } from '../framework/types';

export class ContentType {
public readonly mediaType: string = null;
Expand Down Expand Up @@ -174,20 +174,3 @@ export const zipObject = (keys, values) =>
return acc
}, {})

/**
* Tries to fetch a schema from ajv instance by the provided key otherwise adds (and
* compiles) the schema under provided key. We provide a key to avoid ajv library
* using the whole schema as a cache key, leading to a lot of unnecessary memory
* usage - this is not recommended by the library either:
* https://ajv.js.org/guide/managing-schemas.html#cache-key-schema-vs-key-vs-id
*
* @param ajvCacheKey - Key which will be used for ajv cache
*/
export function useAjvCache(ajv: AjvInstance, schema: object, ajvCacheKey: string) {
let validator = ajv.getSchema(ajvCacheKey);
if (!validator) {
ajv.addSchema(schema, ajvCacheKey);
validator = ajv.getSchema(ajvCacheKey);
}
return validator
}