From 12c4bcf8b72a2a067827b3958da4ca24595b9985 Mon Sep 17 00:00:00 2001
From: Simeon Widdis <sawiddis@amazon.com>
Date: Mon, 21 Aug 2023 11:36:02 -0700
Subject: [PATCH] Refactor validation logic with a deeper interface

Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
---
 .../repository/__test__/integration.test.ts   | 11 ++++--
 .../integrations/repository/integration.ts    | 21 +---------
 server/adaptors/integrations/validators.ts    | 39 ++++++++++++++++++-
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts
index 4474fc48f..2002ad04a 100644
--- a/server/adaptors/integrations/repository/__test__/integration.test.ts
+++ b/server/adaptors/integrations/repository/__test__/integration.test.ts
@@ -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',
@@ -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 () => {
diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts
index fe9ddd0ef..1350cb653 100644
--- a/server/adaptors/integrations/repository/integration.ts
+++ b/server/adaptors/integrations/repository/integration.ts
@@ -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.
@@ -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.
@@ -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);
diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts
index 3cb24212d..ab51c17cb 100644
--- a/server/adaptors/integrations/validators.ts
+++ b/server/adaptors/integrations/validators.ts
@@ -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;
+};