Convert validator methods to use result types

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>

server/adaptors/integrations/__test__/validators.test.ts

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { validateTemplate, validateInstance } from '../validators';
+import { validateTemplate, validateInstance, ValidationResult } from '../validators';
 
 const validTemplate: IntegrationTemplate = {
   name: 'test',
@@ -28,62 +28,57 @@ const validInstance: IntegrationInstance = {
 };
 
 describe('validateTemplate', () => {
-  it('Returns true for a valid Integration Template', () => {
-    expect(validateTemplate(validTemplate)).toBe(true);
+  it('Returns a success value for a valid Integration Template', () => {
+    const result: ValidationResult<IntegrationTemplate> = validateTemplate(validTemplate);
+    expect(result.ok).toBe(true);
+    expect((result as any).value).toBe(validTemplate);
   });
 
-  it('Returns false if a template is missing a license', () => {
+  it('Returns a failure value if a template is missing a license', () => {
     const sample: any = structuredClone(validTemplate);
     sample.license = undefined;
-    expect(validateTemplate(sample)).toBe(false);
+
+    const result: ValidationResult<IntegrationTemplate> = validateTemplate(sample);
+
+    expect(result.ok).toBe(false);
+    expect((result as any).error).toBeInstanceOf(Error);
   });
 
-  it('Returns false if a template has an invalid type', () => {
+  it('Returns a failure if a template has an invalid type', () => {
     const sample: any = structuredClone(validTemplate);
     sample.components[0].name = 'not-logs';
-    expect(validateTemplate(sample)).toBe(false);
-  });
 
-  it('Respects logErrors', () => {
-    const logValidationErrorsMock = jest.spyOn(console, 'error');
-    const sample1: any = structuredClone(validTemplate);
-    sample1.license = undefined;
-    const sample2: any = structuredClone(validTemplate);
-    sample2.components[0].name = 'not-logs';
+    const result: ValidationResult<IntegrationTemplate> = validateTemplate(sample);
 
-    expect(validateTemplate(sample1, true)).toBe(false);
-    expect(validateTemplate(sample2, true)).toBe(false);
-    expect(logValidationErrorsMock).toBeCalledTimes(2);
+    expect(result.ok).toBe(false);
+    expect((result as any).error).toBeInstanceOf(Error);
   });
 
   it("Doesn't crash if given a non-object", () => {
     // May happen in some user-provided JSON parsing scenarios.
-    expect(validateTemplate([] as any, true)).toBe(false);
+    expect(validateTemplate([] as any).ok).toBe(false);
   });
 });
 
 describe('validateInstance', () => {
   it('Returns true for a valid Integration Instance', () => {
-    expect(validateInstance(validInstance)).toBe(true);
+    const result: ValidationResult<IntegrationInstance> = validateInstance(validInstance);
+    expect(result.ok).toBe(true);
+    expect((result as any).value).toBe(validInstance);
   });
 
   it('Returns false if an instance is missing a template', () => {
     const sample: any = structuredClone(validInstance);
     sample.templateName = undefined;
-    expect(validateInstance(sample)).toBe(false);
-  });
 
-  it('Respects logErrors', () => {
-    const logValidationErrorsMock = jest.spyOn(console, 'error');
-    const sample1: any = structuredClone(validInstance);
-    sample1.templateName = undefined;
+    const result: ValidationResult<IntegrationInstance> = validateInstance(sample);
 
-    expect(validateInstance(sample1, true)).toBe(false);
-    expect(logValidationErrorsMock).toBeCalled();
+    expect(result.ok).toBe(false);
+    expect((result as any).error).toBeInstanceOf(Error);
   });
 
   it("Doesn't crash if given a non-object", () => {
     // May happen in some user-provided JSON parsing scenarios.
-    expect(validateInstance([] as any, true)).toBe(false);
+    expect(validateInstance([] as any).ok).toBe(false);
   });
 });

server/adaptors/integrations/repository/integration.ts

@@ -151,8 +151,12 @@ export class Integration {
     try {
       const config = await fs.readFile(configPath, { encoding: 'utf-8' });
       const possibleTemplate = JSON.parse(config);
-
-      return validateTemplate(possibleTemplate, true) ? possibleTemplate : null;
+      const template = validateTemplate(possibleTemplate);
+      if (template.ok) {
+        return template.value;
+      }
+      console.error(template.error);
+      return null;
     } catch (err: any) {
       if (err instanceof SyntaxError) {
         console.error(`Syntax errors in ${configFile}`, err);

server/adaptors/integrations/validators.ts

@@ -5,6 +5,8 @@
 
 import Ajv, { JSONSchemaType } from 'ajv';
 
+export type ValidationResult<T, E = Error> = { ok: true; value: T } | { ok: false; error: E };
+
 const ajv = new Ajv();
 
 const staticAsset: JSONSchemaType<StaticAsset> = {
@@ -110,37 +112,50 @@ const instanceSchema: JSONSchemaType<IntegrationInstance> = {
 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 => {
+/**
+ * Validates an integration template against a predefined schema using the AJV library.
+ * Since AJV validators use side effects for errors,
+ * this is a more conventional wrapper that simplifies calling.
+ *
+ * @param data The data to be validated as an IntegrationTemplate.
+ * @return A ValidationResult indicating whether the validation was successful or not.
+ *         If validation succeeds, returns an object with 'ok' set to true and the validated data.
+ *         If validation fails, returns an object with 'ok' set to false and an Error object describing the validation error.
+ */
+export const validateTemplate = (data: unknown): ValidationResult<IntegrationTemplate> => {
   if (!templateValidator(data)) {
-    if (logErrors) {
-      console.error(
-        `The integration config '${data.name ?? data}' is invalid:`,
-        ajv.errorsText(templateValidator.errors)
-      );
-    }
-    return false;
+    return { ok: false, error: new Error(ajv.errorsText(templateValidator.errors)) };
   }
   // 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 {
+      ok: false,
+      error: new Error(`The integration type '${data.type}' must be included as a component`),
+    };
   }
-  return true;
+  return {
+    ok: true,
+    value: data,
+  };
 };
 
-export const validateInstance = (data: { name?: unknown }, logErrors?: true): boolean => {
+/**
+ * Validates an integration instance against a predefined schema using the AJV library.
+ *
+ * @param data The data to be validated as an IntegrationInstance.
+ * @return A ValidationResult indicating whether the validation was successful or not.
+ *         If validation succeeds, returns an object with 'ok' set to true and the validated data.
+ *         If validation fails, returns an object with 'ok' set to false and an Error object describing the validation error.
+ */
+export const validateInstance = (data: unknown): ValidationResult<IntegrationInstance> => {
   if (!instanceValidator(data)) {
-    if (logErrors) {
-      console.error(
-        `The integration instance '${data.name ?? data}' is invalid:`,
-        ajv.errorsText(instanceValidator.errors)
-      );
-    }
-    return false;
+    return {
+      ok: false,
+      error: new Error(ajv.errorsText(instanceValidator.errors)),
+    };
   }
-  return true;
+  return {
+    ok: true,
+    value: data,
+  };
 };