refactor validation return values

complexdatacollective · Nov 8, 2023 · ecab125 · ecab125
1 parent a9ac091
commit ecab125
Show file tree

Hide file tree

Showing 8 changed files with 158 additions and 149 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@codaco/protocol-validation",
-  "version": "3.0.0-alpha.3",
+  "version": "3.0.0-alpha.4",
   "main": "dist/index.js",
   "license": "GPL-3.0-or-later",
   "repository": {

diff --git a/src/__tests__/test-protocols.test.ts b/src/__tests__/test-protocols.test.ts
@@ -1,5 +1,5 @@
-import { beforeEach, describe, expect, it } from "bun:test";
-import { ValidationError, validateProtocol } from "../../dist";
+import { describe, expect, it } from "bun:test";
+import { validateProtocol } from "../../dist";
 import { readFile } from "fs/promises";
 import { readdirSync } from "fs";
 import { join } from "path";
@@ -35,27 +35,7 @@ const extractAndValidate = async (protocolPath: string) => {
   }
 
   // Validating protocol...
-  try {
-    await validateProtocol(protocol, schemaVersion);
-    return {
-      error: undefined,
-      errorDetails: [],
-      success: true,
-    };
-  } catch (error) {
-    console.log("validation error", error);
-    if (error instanceof ValidationError) {
-      return {
-        protocol: protocolPath,
-        schemaVersion: error.schemaVersion,
-        forced: error.schemaForced,
-        error: error.message,
-        errorDetails: [...error.logicErrors, ...error.schemaErrors],
-      };
-    }
-
-    throw error;
-  }
+  return await validateProtocol(protocol, schemaVersion);
 };
 
 const PROTOCOL_PATH = "../../test-protocols";
@@ -68,10 +48,8 @@ describe("Test protocols", () => {
     const protocolPath = join(__dirname, PROTOCOL_PATH, protocol);
     const result = await extractAndValidate(protocolPath);
 
-    expect(result).toEqual({
-      error: undefined,
-      errorDetails: [],
-      success: true,
-    });
+    expect(result.isValid).toBe(true);
+    expect(result.schemaErrors).toEqual([]);
+    expect(result.logicErrors).toEqual([]);
   });
 });
diff --git a/src/index.ts b/src/index.ts
@@ -1,28 +1,20 @@
 import { validateSchema } from "./validation/validateSchema";
 import { validateLogic } from "./validation/validateLogic";
 import { Protocol } from "@codaco/shared-consts";
-export class ValidationError extends Error {
-  public schemaErrors: string[];
-  public logicErrors: string[];
-  public schemaVersion: number;
-  public schemaForced: boolean;
+import { ensureError } from "./utils/ensureError";
 
-  constructor(
-    message: string,
-    schemaErrors: string[],
-    logicErrors: string[],
-    schemaVersion: number,
-    schemaForced: boolean,
-  ) {
-    super(message);
-    this.name = "ValidationError";
-    this.schemaErrors = schemaErrors;
-    this.logicErrors = logicErrors;
-    this.message = message;
-    this.schemaVersion = schemaVersion;
-    this.schemaForced = schemaForced;
-  }
-}
+export type ValidationError = {
+  path: string;
+  message: string;
+};
+
+export type ValidationResult = {
+  isValid: boolean;
+  schemaErrors: ValidationError[];
+  logicErrors: ValidationError[];
+  schemaVersion: number;
+  schemaForced: boolean;
+};
 
 const validateProtocol = async (
   protocol: Protocol,
@@ -32,28 +24,26 @@ const validateProtocol = async (
     throw new Error("Protocol is undefined");
   }
 
-  const schemaResult = await validateSchema(protocol, forceSchemaVersion);
-  const logicResult = validateLogic(protocol);
+  try {
+    const { hasErrors: hasSchemaErrors, errors: schemaErrors } =
+      await validateSchema(protocol, forceSchemaVersion);
+    const { hasErrors: hasLogicErrors, errors: logicErrors } =
+      validateLogic(protocol);
+
+    return {
+      isValid: !hasSchemaErrors && !hasLogicErrors,
+      schemaErrors,
+      logicErrors,
+      schemaVersion: protocol.schemaVersion,
+      schemaForced: forceSchemaVersion !== undefined,
+    } as ValidationResult;
+  } catch (e) {
+    const error = ensureError(e);
 
-  if (!(schemaResult instanceof Error) && !(logicResult instanceof Error)) {
-    const { hasErrors: hasSchemaErrors, errors: schemaErrors } = schemaResult;
-    const { hasErrors: hasLogicErrors, errors: logicErrors } = logicResult;
-    if (hasSchemaErrors || hasLogicErrors) {
-      throw new ValidationError(
-        "Protocol is invalid!",
-        schemaErrors,
-        logicErrors,
-        protocol.schemaVersion,
-        forceSchemaVersion !== undefined,
-      );
-    }
-  } else {
     throw new Error(
-      `Protocol validation failed due to an internal error: ${schemaResult}}`,
+      `Protocol validation failed due to an internal error: ${error.message}`,
     );
   }
-
-  return;
 };
 
 export { validateSchema, validateLogic, validateProtocol };
diff --git a/src/migrations/migrations/__tests__/__snapshots__/4.test.js.snap b/src/migrations/migrations/__tests__/__snapshots__/4.test.js.snap
@@ -1,51 +1,4 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`migrate v3 -> v4 additional attributes values 1`] = `
-[
-  {
-    "value": true,
-    "variable": "foobar",
-  },
-  {
-    "value": false,
-    "variable": "fizzpop",
-  },
-]
-`;
-
-exports[`migrate v3 -> v4 additional attributes values 2`] = `
-{
-  "id": "noBoolean",
-}
-`;
-
-exports[`migrate v3 -> v4 migrates additional attributes 1`] = `
-{
-  "codebook": {},
-  "stages": [
-    {
-      "prompts": [
-        {
-          "additionalAttributes": [
-            {
-              "value": true,
-              "variable": "foobar",
-            },
-            {
-              "value": false,
-              "variable": "fizzpop",
-            },
-          ],
-          "id": "someBoolean",
-        },
-        {
-          "id": "noBoolean",
-        },
-      ],
-    },
-  ],
-}
-`;
+// Bun Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`migrate v3 -> v4 migrates codebook 1`] = `
 {
@@ -112,27 +65,6 @@ exports[`migrate v3 -> v4 migrates codebook 1`] = `
 }
 `;
 
-exports[`migrate v3 -> v4 option values 1`] = `
-[
-  {
-    "label": "foo",
-    "value": "f_o_o",
-  },
-  {
-    "label": "foo2",
-    "value": "f_o_o2",
-  },
-  {
-    "label": "bar",
-    "value": "b_a-r:.",
-  },
-  {
-    "label": "bazz",
-    "value": 5,
-  },
-]
-`;
-
 exports[`migrate v3 -> v4 type names 1`] = `
 {
   "allowedTypeName": {
@@ -207,3 +139,71 @@ exports[`migrate v3 -> v4 variable names 1`] = `
   },
 }
 `;
+
+exports[`migrate v3 -> v4 option values 1`] = `
+[
+  {
+    "label": "foo",
+    "value": "f_o_o",
+  },
+  {
+    "label": "foo2",
+    "value": "f_o_o2",
+  },
+  {
+    "label": "bar",
+    "value": "b_a-r:.",
+  },
+  {
+    "label": "bazz",
+    "value": 5,
+  },
+]
+`;
+
+exports[`migrate v3 -> v4 migrates additional attributes 1`] = `
+{
+  "codebook": {},
+  "stages": [
+    {
+      "prompts": [
+        {
+          "additionalAttributes": [
+            {
+              "value": true,
+              "variable": "foobar",
+            },
+            {
+              "value": false,
+              "variable": "fizzpop",
+            },
+          ],
+          "id": "someBoolean",
+        },
+        {
+          "id": "noBoolean",
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`migrate v3 -> v4 additional attributes values 1`] = `
+[
+  {
+    "value": true,
+    "variable": "foobar",
+  },
+  {
+    "value": false,
+    "variable": "fizzpop",
+  },
+]
+`;
+
+exports[`migrate v3 -> v4 additional attributes values 2`] = `
+{
+  "id": "noBoolean",
+}
+`;
diff --git a/src/utils/ensureError.ts b/src/utils/ensureError.ts
@@ -0,0 +1,19 @@
+// Helper function that ensures that a value is an Error
+export function ensureError(value: unknown): Error {
+  if (!value) return new Error("No value was thrown");
+
+  if (value instanceof Error) return value;
+
+  // Test if value inherits from Error
+  if (value.isPrototypeOf(Error)) return value as Error & typeof value;
+
+  let stringified = "[Unable to stringify the thrown value]";
+  try {
+    stringified = JSON.stringify(value);
+  } catch {}
+
+  const error = new Error(
+    `This value was thrown as is, not through an Error: ${stringified}`,
+  );
+  return error;
+}
diff --git a/src/validation/Validator.ts b/src/validation/Validator.ts
@@ -1,5 +1,6 @@
 import { Protocol, StageSubject } from "@codaco/shared-consts";
 import { get } from "lodash-es";
+import { ValidationError } from "..";
 
 /**
  * See addValidation().
@@ -81,6 +82,8 @@ export type ValidationItemBase = {
 export type Validation<T> = ValidationItemBase &
   (ValidationItemSingle<T> | ValidationItemSequence<T>);
 
+export type LogicError = {};
+
 /**
  * @class
  * Support data validations on a protocol.
@@ -95,7 +98,7 @@ export type Validation<T> = ValidationItemBase &
  */
 class Validator {
   private protocol: Protocol;
-  errors: string[];
+  errors: ValidationError[];
   warnings: string[];
   private validations: Validation<any>[];
 
@@ -203,7 +206,12 @@ class Validator {
         this.warnings.push(errorString);
         return true;
       }
-      this.errors.push(`${keypathToString(keypath)}: ${failureMessage}`);
+
+      this.errors.push({
+        path: keypathToString(keypath),
+        message: failureMessage,
+      });
+
       return false;
     }
     return true;

diff --git a/src/validation/__tests__/__snapshots__/validateLogic.test.js.snap b/src/validation/__tests__/__snapshots__/validateLogic.test.js.snap
@@ -3,9 +3,18 @@
 exports[`validateLogic A well formed protocol will return an array of errors 1`] = `
 {
   "errors": [
-    "protocol.codebook: Duplicate entity name "du"",
-    "protocol.stages[0].form.fields[0]: Form field variable not found in codebook.",
-    "protocol.stages[5].form.fields[0]: Form field variable not found in codebook.",
+    {
+      "message": "Duplicate entity name "du"",
+      "path": "protocol.codebook",
+    },
+    {
+      "message": "Form field variable not found in codebook.",
+      "path": "protocol.stages[0].form.fields[0]",
+    },
+    {
+      "message": "Form field variable not found in codebook.",
+      "path": "protocol.stages[5].form.fields[0]",
+    },
   ],
   "hasErrors": true,
 }