From 4ab0a028eaeef5ae73d6301424505b3de7a20e1d Mon Sep 17 00:00:00 2001
From: markford <mark@massfords.com>
Date: Fri, 24 Jun 2022 13:02:53 -0400
Subject: [PATCH 1/2] fix: detect duplicate state names refactor: moved
 existing checks to be post schema validation

---
 .../definitions/invalid-map-dupe-state.json   | 28 +++++++++++++++++++
 src/lib/state-names.js                        | 23 +++++++++++++++
 src/validator.js                              | 27 ++++++++----------
 3 files changed, 62 insertions(+), 16 deletions(-)
 create mode 100644 src/__tests__/definitions/invalid-map-dupe-state.json
 create mode 100644 src/lib/state-names.js

diff --git a/src/__tests__/definitions/invalid-map-dupe-state.json b/src/__tests__/definitions/invalid-map-dupe-state.json
new file mode 100644
index 0000000..ae04230
--- /dev/null
+++ b/src/__tests__/definitions/invalid-map-dupe-state.json
@@ -0,0 +1,28 @@
+{
+  "Comment": "Exhibits duplicate state error. State names MUST be unique within the scope of the whole state machine.",
+  "StartAt": "Map",
+  "States": {
+    "Map": {
+      "Type": "Map",
+      "Next": "Final State",
+      "InputPath": "$.input",
+      "ItemsPath": "$.items",
+      "MaxConcurrency": 0,
+      "Iterator": {
+        "StartAt": "Final State",
+        "States": {
+          "Final State": {
+            "Type": "Wait",
+            "Seconds": 20,
+            "End": true
+          }
+        }
+      },
+      "ResultPath": "$.result"
+    },
+    "Final State": {
+      "Type": "Pass",
+      "End": true
+    }
+  }
+}
diff --git a/src/lib/state-names.js b/src/lib/state-names.js
new file mode 100644
index 0000000..eeea6a7
--- /dev/null
+++ b/src/lib/state-names.js
@@ -0,0 +1,23 @@
+const jp = require('jsonpath');
+
+module.exports = (definition) => {
+  const errorMessages = [];
+  const names = new Map();
+  jp.query(definition, '$..[\'States\']')
+    .forEach((states) => {
+      Object.keys(states).forEach((stateName) => {
+        const current = names.get(stateName);
+        names.set(stateName, current ? current + 1 : 1);
+      });
+      names.forEach((value, key) => {
+        if (value > 1) {
+          errorMessages.push({
+            'Error code': 'DUPLICATE_STATE_NAMES',
+            Message: `A state with this name already exists: ${key}`,
+          });
+        }
+      });
+    });
+
+  return errorMessages;
+};
diff --git a/src/validator.js b/src/validator.js
index 0f8c956..d060f87 100644
--- a/src/validator.js
+++ b/src/validator.js
@@ -14,6 +14,7 @@ const errors = require('./schemas/errors');
 const checkJsonPath = require('./lib/json-path-errors');
 const missingTransitionTarget = require('./lib/missing-transition-target');
 const stateTransitions = require('./lib/state-transitions');
+const stateNames = require('./lib/state-names');
 
 function formatError(e) {
   const code = e.Code ? e.Code : e['Error code'];
@@ -37,32 +38,26 @@ function validator(definition) {
     ],
   });
 
-  // Validating JSON paths
-  const jsonPathErrors = checkJsonPath(definition);
-
-  // Check unreachable states
-  const missingTransitionTargetErrors = missingTransitionTarget(definition);
-
   // Validating JSON schemas
   const isJsonSchemaValid = ajv.validate('http://asl-validator.cloud/state-machine.json#', definition);
 
-  // Check for Parallel states
-  const transitionErrors = isJsonSchemaValid ? stateTransitions(definition) : [];
+  const postSchemaValidationErrors = [];
+  if (isJsonSchemaValid) {
+    postSchemaValidationErrors.push(...checkJsonPath(definition));
+    postSchemaValidationErrors.push(...missingTransitionTarget(definition));
+    postSchemaValidationErrors.push(...stateTransitions(definition));
+    postSchemaValidationErrors.push(...stateNames(definition));
+  }
 
   return {
-    isValid: isJsonSchemaValid && !jsonPathErrors.length && !missingTransitionTargetErrors.length
-        && !transitionErrors.length,
-    errors: jsonPathErrors.concat(ajv.errors || [])
-      .concat(missingTransitionTargetErrors || [])
-      .concat(transitionErrors || []),
+    isValid: isJsonSchemaValid && !postSchemaValidationErrors.length,
+    errors: (ajv.errors || []).concat(postSchemaValidationErrors || []),
     errorsText: (separator = '\n') => {
       const errorList = [];
-      errorList.push(jsonPathErrors.map(formatError).join(separator));
       if (ajv.errors) {
         errorList.push(ajv.errorsText(ajv.errors, { separator }));
       }
-      errorList.push(missingTransitionTargetErrors.map(formatError).join(separator));
-      errorList.push(transitionErrors.map(formatError).join(separator));
+      errorList.push(postSchemaValidationErrors.map(formatError).join(separator));
       return errorList.join(separator);
     },
   };

From cb564fd16fb41e9f4a6a6eb0eb2c0f130178d7db Mon Sep 17 00:00:00 2001
From: markford <mark@massfords.com>
Date: Sun, 26 Jun 2022 05:29:23 -0400
Subject: [PATCH 2/2] walk names once outside of loop

---
 src/lib/state-names.js | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/lib/state-names.js b/src/lib/state-names.js
index eeea6a7..33d567a 100644
--- a/src/lib/state-names.js
+++ b/src/lib/state-names.js
@@ -9,15 +9,15 @@ module.exports = (definition) => {
         const current = names.get(stateName);
         names.set(stateName, current ? current + 1 : 1);
       });
-      names.forEach((value, key) => {
-        if (value > 1) {
-          errorMessages.push({
-            'Error code': 'DUPLICATE_STATE_NAMES',
-            Message: `A state with this name already exists: ${key}`,
-          });
-        }
-      });
     });
+  names.forEach((value, key) => {
+    if (value > 1) {
+      errorMessages.push({
+        'Error code': 'DUPLICATE_STATE_NAMES',
+        Message: `A state with this name already exists: ${key}`,
+      });
+    }
+  });
 
   return errorMessages;
 };