Skip to content

Commit

Permalink
Refactor validation logic with a deeper interface
Browse files Browse the repository at this point in the history
Signed-off-by: Simeon Widdis <[email protected]>
  • Loading branch information
Swiddis committed Aug 21, 2023
1 parent c8587fd commit 12c4bcf
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ describe('Integration', () => {
name: 'sample',
version: '2.0.0',
license: 'Apache-2.0',
type: '',
components: [],
type: 'logs',
components: [
{
name: 'logs',
version: '1.0.0',
},
],
assets: {
savedObjects: {
name: 'sample',
Expand Down Expand Up @@ -105,7 +110,7 @@ describe('Integration', () => {
const result = await integration.getConfig(sampleIntegration.version);

expect(result).toBeNull();
expect(logValidationErrorsMock).toHaveBeenCalledWith(expect.any(String), expect.any(Array));
expect(logValidationErrorsMock).toHaveBeenCalled();
});

it('should return null and log syntax errors if the config file has syntax errors', async () => {
Expand Down
21 changes: 2 additions & 19 deletions server/adaptors/integrations/repository/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import * as fs from 'fs/promises';
import path from 'path';
import { ValidateFunction } from 'ajv';
import { templateValidator } from '../validators';
import { validateTemplate } from '../validators';

/**
* Helper function to compare version numbers.
Expand Down Expand Up @@ -49,18 +49,6 @@ async function isDirectory(dirPath: string): Promise<boolean> {
}
}

/**
* Helper function to log validation errors.
* Relies on the `ajv` package for validation error logs..
*
* @param integration The name of the component that failed validation.
* @param validator A failing ajv validator.
*/
function logValidationErrors(integration: string, validator: ValidateFunction<any>) {
const errors = validator.errors?.map((e) => e.message);
console.error(`Validation errors in ${integration}`, errors);
}

/**
* The Integration class represents the data for Integration Templates.
* It is backed by the repository file system.
Expand Down Expand Up @@ -165,12 +153,7 @@ export class Integration {
const config = await fs.readFile(configPath, { encoding: 'utf-8' });
const possibleTemplate = JSON.parse(config);

if (!templateValidator(possibleTemplate)) {
logValidationErrors(configFile, templateValidator);
return null;
}

return possibleTemplate;
return validateTemplate(possibleTemplate, true) ? possibleTemplate : null;
} catch (err: any) {
if (err instanceof SyntaxError) {
console.error(`Syntax errors in ${configFile}`, err);
Expand Down
39 changes: 37 additions & 2 deletions server/adaptors/integrations/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,5 +107,40 @@ const instanceSchema: JSONSchemaType<IntegrationInstance> = {
required: ['name', 'templateName', 'dataSource', 'creationDate', 'assets'],
};

export const templateValidator = ajv.compile(templateSchema);
export const instanceValidator = ajv.compile(instanceSchema);
const templateValidator = ajv.compile(templateSchema);
const instanceValidator = ajv.compile(instanceSchema);

// AJV validators use side effects for errors, so we provide a more conventional wrapper.
// The wrapper optionally handles error logging with the `logErrors` parameter.
export const validateTemplate = (data: { name?: unknown }, logErrors?: true): boolean => {
if (!templateValidator(data)) {
if (logErrors) {
console.error(
`The integration '${data.name ?? 'config'}' is invalid:`,
ajv.errorsText(templateValidator.errors)
);
}
return false;
}
// We assume an invariant that the type of an integration is connected with its component.
if (data.components.findIndex((x) => x.name === data.type) < 0) {
if (logErrors) {
console.error(`The integration type '${data.type}' must be included as a component`);
}
return false;
}
return true;
};

export const validateInstance = (data: { name?: unknown }, logErrors?: true): boolean => {
if (!instanceValidator(data)) {
if (logErrors) {
console.error(
`The integration '${data.name ?? 'instance'} is invalid:`,
ajv.errorsText(instanceValidator.errors)
);
}
return false;
}
return true;
};

0 comments on commit 12c4bcf

Please sign in to comment.