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

APP-14990 Refactor to make registering AJV schemas and formats more modular #197

Merged
merged 5 commits into from
Apr 4, 2024

Conversation

eyadmba
Copy link
Contributor

@eyadmba eyadmba commented Apr 4, 2024

Needed to run validations on entities in the persister, and that required different constructor parameters to be passed to the AJV instance, so I couldn't use the already existing one in src/IntegrationSchema.ts.

Instead, I exposed a function that takes in an AJV instance, and sets all the schemas on it. This way I can control the instance's constructor params, and just leverage the package's function to set the schemas on it.
Similar thing was done for the custom formats of AJV. There's a function that adds all the custom formats on an AJV instance.

So this is how I'd end up using those two new functions:

const myInstance = new Ajv({ whatever I want });
registerSchemas(myInstance);
registerFormats(myInstance);

@eyadmba eyadmba requested a review from a team as a code owner April 4, 2024 06:47
import WorkflowJson from './schemas/Workflow.json';
import WorkloadJson from './schemas/Workload.json';

export function registerSchemas(ajvInstance: Ajv): void {
Copy link
Contributor Author

@eyadmba eyadmba Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the PR's description, this is the function that takes in an AJV instance and adds all the data model's schemas to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to src/IntegrationSchema.ts, this file gets auto generated entirely using the script tools/generate-register-schemas-function.sh.

fn: (x) => ipv4CidrRegex.test(x) || ipv6CidrRegex.test(x),
};

export function registerFormats(ajvInstance: Ajv): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to registerSchemas, here's registerFormats().

});

// Install ajv-formats
addFormats(IntegrationSchema);
registerFormats(IntegrationSchema);

// Schema Imports : generated by tools/generate-schema-imports.sh
import WorkloadJson from './schemas/Workload.json';
Copy link
Contributor Author

@eyadmba eyadmba Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have replaced all the code from this point until the end of the file with just a simple call to the new function: registerSchemas(IntegrationSchema), but I noticed that there are exposed members from this module in those lines that I didn't know if they're being used elsewhere or not, so I had to leave them.

zemberdotnet
zemberdotnet previously approved these changes Apr 4, 2024
@eyadmba eyadmba merged commit 5803376 into main Apr 4, 2024
10 checks passed
@eyadmba eyadmba deleted the APP-14990 branch April 4, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants